Skip to content

Commit

Permalink
Fix for WFCORE-7097, Intermittent bootable jar test failures of Permi…
Browse files Browse the repository at this point in the history
…ssionsDeploymentTestCase.testWithConfiguredMaxBootThreads
  • Loading branch information
jfdenise committed Dec 17, 2024
1 parent b94be1e commit dba8960
Show file tree
Hide file tree
Showing 3 changed files with 40 additions and 24 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -34,6 +34,9 @@ public static void main(final String[] args) throws Exception {
final Path installDir = Paths.get(args[0]);
final int retries = Integer.parseInt(args[1]);
final Path cleanupMarker = installDir.resolve("wildfly-cleanup-marker");
if (Files.notExists(cleanupMarker)) {
return;
}
int attempts = 1;
while (attempts <= retries) {
final boolean lastAttempt = attempts == retries;
Expand All @@ -51,6 +54,9 @@ public static void main(final String[] args) throws Exception {
}

private static void cleanup(final Path installDir, final Path cleanupMarker, final boolean log) throws IOException {
if (Files.notExists(cleanupMarker)) {
return;
}
Files.walkFileTree(installDir, new SimpleFileVisitor<Path>() {
@Override
public FileVisitResult visitFile(final Path file, final BasicFileAttributes attrs) throws IOException {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -442,20 +442,22 @@ public void run() {
thread.setName("installation-cleaner");
return thread;
});
final InstallationCleaner cleaner = new InstallationCleaner(environment, log);
executor.submit(cleaner);
if (Files.exists(pidFile)) {
waitForShutdown();
}
final InstallationCleaner cleaner = new InstallationCleaner(environment, log);
executor.submit(cleaner);
executor.shutdown();
try {
if (!executor.awaitTermination(environment.getTimeout(), TimeUnit.SECONDS)) {
// For some reason we've timed out. The deletion should likely be executing.
// We can't start a new cleanup to force it. On Windows we would have the side effect to have 2 cleaner processes to
// We can't start a new full cleanup to force it. On Windows we would have the side effect to have 2 cleaner processes to
// be executed, with the risk that a new installation has been installed and the new cleaner cleaning the new installation
log.cleanupTimeout(environment.getTimeout(), environment.getJBossHome());
// This cleanup only runs if marker still exists.
cleaner.cleanupTimeout();
}
} catch (InterruptedException e) {
} catch (IOException | InterruptedException e) {
// The task has been interrupted, leaving
log.cleanupTimeout(environment.getTimeout(), environment.getJBossHome());
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -35,6 +35,7 @@ class InstallationCleaner implements Runnable {
private final BootableJarLogger logger;
private final boolean newProcess;
private final int retries;
private Process process;

InstallationCleaner(final BootableEnvironment environment, final BootableJarLogger logger) {
this.environment = environment;
Expand All @@ -45,28 +46,13 @@ class InstallationCleaner implements Runnable {
}

@Override
public void run() {
public synchronized void run() {
// Clean up is not already in progress
if (Files.notExists(cleanupMarker)) {
try {
Files.createFile(cleanupMarker);
long timeout = environment.getTimeout() * 1000;
final Path pidFile = environment.getPidFile();
final long wait = 500L;
while (Files.exists(pidFile)) {
try {
TimeUnit.MILLISECONDS.sleep(wait);
} catch (InterruptedException ignore) {
break;
}
timeout -= wait;
if (timeout <= 0) {
logger.cleanupTimeout(environment.getTimeout(), pidFile);
break;
}
}
cleanup();
} catch (IOException e) {
} catch (InterruptedException | IOException e) {
logger.failedToStartCleanupProcess(e, environment.getJBossHome());
}
}
Expand All @@ -83,7 +69,10 @@ public void run() {
*
* @throws IOException if an error occurs deleting the directory
*/
void cleanup() throws IOException {
private void cleanup() throws InterruptedException, IOException {
if (Files.notExists(cleanupMarker)) {
return;
}
if (newProcess) {
try {
newProcess();
Expand All @@ -104,6 +93,23 @@ void cleanup() throws IOException {
}
}

// In case of timeout, we attempt to do a cleanup only if the cleanupMarker exists
synchronized void cleanupTimeout() throws IOException, InterruptedException {
if (Files.notExists(cleanupMarker)) {
return;
}
// In case we have a running process, kill it to avoid it to continue any cleanup.
if (newProcess && process != null) {
process.destroyForcibly();
process.waitFor(environment.getTimeout(), TimeUnit.SECONDS);
process = null;
}
// Do a last cleanup, in case the cleanupMarker still exists (could have been deleted by running process).
if (Files.exists(cleanupMarker)) {
cleanup();
}
}

private void deleteDirectory() throws IOException {
final Path installDir = environment.getJBossHome();
Files.walkFileTree(installDir, new SimpleFileVisitor<Path>() {
Expand Down Expand Up @@ -136,7 +142,7 @@ public FileVisitResult postVisitDirectory(final Path dir, final IOException exc)
});
}

private void newProcess() throws IOException {
private void newProcess() throws IOException, InterruptedException {
// Start a new process which will clean up the install directory. This is done in a new process in cases where
// this process may hold locks on to resources that need to be cleaned up.
final String[] cmd = {
Expand All @@ -153,7 +159,9 @@ private void newProcess() throws IOException {
.redirectError(ProcessBuilder.Redirect.INHERIT)
.redirectOutput(ProcessBuilder.Redirect.INHERIT)
.directory(new File(System.getProperty("user.dir")));
builder.start();
process = builder.start();
process.waitFor(environment.getTimeout(), TimeUnit.SECONDS);
process.destroyForcibly();
}

private String getJavaCommand() {
Expand Down

0 comments on commit dba8960

Please sign in to comment.