Skip to content

Commit

Permalink
Make sure the TestResourceManager is closed before the CL is closed
Browse files Browse the repository at this point in the history
- The TestResourceManager is instantiated using the runtime class loader
  so we need to make sure we close it before we close the runtime class
  loader. Otherwise we can get into trouble if we need to load some
  additional classes.
- Also make sure that it is only closed once. It used to be closed once
  when the application is stopped and then again when the
  QuarkusTestExtensionState was closed.

Related to quarkusio#41233
  • Loading branch information
gsmet committed Jun 25, 2024
1 parent c38c488 commit 313e10e
Show file tree
Hide file tree
Showing 3 changed files with 37 additions and 18 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,7 @@
import java.nio.charset.StandardCharsets;
import java.nio.file.Files;
import java.nio.file.StandardOpenOption;
import java.util.ArrayList;
import java.util.HashMap;
import java.util.List;
import java.util.Map;
Expand Down Expand Up @@ -52,6 +53,7 @@ public class StartupActionImpl implements StartupAction {
private final String applicationClassName;
private final Map<String, String> devServicesProperties;
private final List<RuntimeApplicationShutdownBuildItem> runtimeApplicationShutdownBuildItems;
private final List<Closeable> runtimeCloseTasks = new ArrayList<>();

public StartupActionImpl(CuratedApplication curatedApplication, BuildResult buildResult) {
this.curatedApplication = curatedApplication;
Expand Down Expand Up @@ -125,6 +127,13 @@ public void run() {
log.error("Failed to run close task", t);
}
}
for (var closeTask : runtimeCloseTasks) {
try {
closeTask.close();
} catch (Throwable t) {
log.error("Failed to run close task", t);
}
}
}
}
}, "Quarkus Main Thread");
Expand Down Expand Up @@ -168,6 +177,11 @@ public void run() {
}
}

@Override
public void addRuntimeCloseTask(Closeable closeTask) {
this.runtimeCloseTasks.add(closeTask);
}

private void doClose() {
try {
runtimeClassLoader.loadClass(Quarkus.class.getName()).getMethod("blockingExit").invoke(null);
Expand Down Expand Up @@ -234,6 +248,13 @@ public void run() {
}
return result.get();
} finally {
for (var closeTask : runtimeCloseTasks) {
try {
closeTask.close();
} catch (Throwable t) {
log.error("Failed to run close task", t);
}
}
runtimeClassLoader.close();
Thread.currentThread().setContextClassLoader(old);
for (var i : runtimeApplicationShutdownBuildItems) {
Expand Down Expand Up @@ -294,6 +315,13 @@ public void close() throws IOException {
// (e.g. ServiceLoader calls)
Thread.currentThread().setContextClassLoader(runtimeClassLoader);
closeTask.close();
for (var closeTask : runtimeCloseTasks) {
try {
closeTask.close();
} catch (Throwable t) {
log.error("Failed to run close task", t);
}
}
} finally {
Thread.currentThread().setContextClassLoader(original);
runtimeClassLoader.close();
Expand Down
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
package io.quarkus.bootstrap.app;

import java.io.Closeable;
import java.util.Map;
import java.util.function.Consumer;

Expand Down Expand Up @@ -31,4 +32,6 @@ public interface StartupAction {
*/
int runMainClassBlocking(String... args) throws Exception;

void addRuntimeCloseTask(Closeable closeTask);

}
Original file line number Diff line number Diff line change
Expand Up @@ -237,6 +237,7 @@ public Thread newThread(Runnable r) {
Map<String, String> properties = (Map<String, String>) testResourceManager.getClass().getMethod("start")
.invoke(testResourceManager);
startupAction.overrideConfig(properties);
startupAction.addRuntimeCloseTask(testResourceManager);
hasPerTestResources = (boolean) testResourceManager.getClass().getMethod("hasPerTestResources")
.invoke(testResourceManager);

Expand Down Expand Up @@ -276,7 +277,6 @@ public Thread newThread(Runnable r) {
RestorableSystemProperties restorableSystemProperties = RestorableSystemProperties.setProperties(
Collections.singletonMap("test.url", TestHTTPResourceManager.getUri(runningQuarkusApplication)));

Closeable tm = testResourceManager;
Closeable shutdownTask = new Closeable() {
@Override
public void close() throws IOException {
Expand All @@ -292,12 +292,8 @@ public void close() throws IOException {
shutdownTasks.pop().run();
}
} finally {
try {
tm.close();
} finally {
restorableSystemProperties.close();
shutdownHangDetection();
}
restorableSystemProperties.close();
shutdownHangDetection();
}
try {
TestClassIndexer.removeIndex(requiredTestClass);
Expand Down Expand Up @@ -1233,23 +1229,15 @@ protected void doClose() throws IOException {
Thread.currentThread().setContextClassLoader(runningQuarkusApplication.getClassLoader());
}
try {
// this will close the application, the test resources, the class loader...
resource.close();
} catch (Throwable e) {
log.error("Failed to shutdown Quarkus", e);
} finally {
runningQuarkusApplication = null;
clonePattern = null;
try {
if (QuarkusTestExtension.this.originalCl != null) {
setCCL(QuarkusTestExtension.this.originalCl);
}
testResourceManager.close();
} catch (Exception e) {
log.error("Failed to shutdown Quarkus test resources", e);
} finally {
Thread.currentThread().setContextClassLoader(old);
ConfigProviderResolver.setInstance(null);
}
Thread.currentThread().setContextClassLoader(old);
ConfigProviderResolver.setInstance(null);
}
}
}
Expand Down

0 comments on commit 313e10e

Please sign in to comment.