From 13b8c6ea2c8c5208952c90de3aa2f7634526cd21 Mon Sep 17 00:00:00 2001 From: Stuart Douglas Date: Wed, 24 Jun 2020 10:18:25 +1000 Subject: [PATCH 1/2] Revert "Ensure that TestResourceManager is only closed once" This reverts commit 25c7a2a4d506ea0fbbff8e7b95c69a6f70437888. --- .../main/java/io/quarkus/test/junit/QuarkusTestExtension.java | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/test-framework/junit5/src/main/java/io/quarkus/test/junit/QuarkusTestExtension.java b/test-framework/junit5/src/main/java/io/quarkus/test/junit/QuarkusTestExtension.java index dfee10dc27506..2d22e5da6df43 100644 --- a/test-framework/junit5/src/main/java/io/quarkus/test/junit/QuarkusTestExtension.java +++ b/test-framework/junit5/src/main/java/io/quarkus/test/junit/QuarkusTestExtension.java @@ -675,7 +675,7 @@ public Object resolveParameter(ParameterContext parameterContext, ExtensionConte } } - class ExtensionState { + class ExtensionState implements ExtensionContext.Store.CloseableResource { private final Closeable testResourceManager; private final Closeable resource; @@ -685,6 +685,7 @@ class ExtensionState { this.resource = resource; } + @Override public void close() throws Throwable { try { resource.close(); From 6d1b8931b52f45bac7b3cd82073ffbf10c881b1d Mon Sep 17 00:00:00 2001 From: Stuart Douglas Date: Wed, 24 Jun 2020 10:28:44 +1000 Subject: [PATCH 2/2] Alternate fix to make sure resources are only closed once --- .../test/common/TestResourceManager.java | 6 +++ .../test/junit/QuarkusTestExtension.java | 44 ++++++++++++------- 2 files changed, 35 insertions(+), 15 deletions(-) diff --git a/test-framework/common/src/main/java/io/quarkus/test/common/TestResourceManager.java b/test-framework/common/src/main/java/io/quarkus/test/common/TestResourceManager.java index 043a030075ba6..8051002d82e80 100644 --- a/test-framework/common/src/main/java/io/quarkus/test/common/TestResourceManager.java +++ b/test-framework/common/src/main/java/io/quarkus/test/common/TestResourceManager.java @@ -24,6 +24,7 @@ public class TestResourceManager implements Closeable { private final List testResourceEntries; private Map oldSystemProps; + private boolean started = false; public TestResourceManager(Class testClass) { testResourceEntries = getTestResources(testClass); @@ -40,6 +41,7 @@ public void init() { } public Map start() { + started = true; Map ret = new HashMap<>(); for (TestResourceEntry entry : testResourceEntries) { try { @@ -70,6 +72,10 @@ public void inject(Object testInstance) { } public void close() { + if (!started) { + return; + } + started = false; if (oldSystemProps != null) { for (Map.Entry e : oldSystemProps.entrySet()) { if (e.getValue() == null) { diff --git a/test-framework/junit5/src/main/java/io/quarkus/test/junit/QuarkusTestExtension.java b/test-framework/junit5/src/main/java/io/quarkus/test/junit/QuarkusTestExtension.java index 2d22e5da6df43..3f1d334eb1099 100644 --- a/test-framework/junit5/src/main/java/io/quarkus/test/junit/QuarkusTestExtension.java +++ b/test-framework/junit5/src/main/java/io/quarkus/test/junit/QuarkusTestExtension.java @@ -18,6 +18,7 @@ import java.util.Objects; import java.util.ServiceLoader; import java.util.concurrent.LinkedBlockingDeque; +import java.util.concurrent.atomic.AtomicBoolean; import java.util.function.Consumer; import java.util.function.Function; import java.util.function.Predicate; @@ -32,6 +33,7 @@ import org.jboss.jandex.FieldInfo; import org.jboss.jandex.Index; import org.jboss.jandex.Type; +import org.jboss.logging.Logger; import org.junit.jupiter.api.extension.AfterAllCallback; import org.junit.jupiter.api.extension.AfterEachCallback; import org.junit.jupiter.api.extension.BeforeAllCallback; @@ -79,6 +81,8 @@ public class QuarkusTestExtension implements BeforeEachCallback, AfterEachCallback, BeforeAllCallback, InvocationInterceptor, AfterAllCallback, ParameterResolver { + private static final Logger log = Logger.getLogger(QuarkusTestExtension.class); + protected static final String TEST_LOCATION = "test-location"; protected static final String TEST_CLASS = "test-class"; @@ -224,19 +228,14 @@ public void close() throws IOException { } } }; + ExtensionState state = new ExtensionState(testResourceManager, shutdownTask); Runtime.getRuntime().addShutdownHook(new Thread(new Runnable() { @Override public void run() { - try { - shutdownTask.close(); - } catch (IOException e) { - e.printStackTrace(); - } finally { - curatedApplication.close(); - } + state.close(); } }, "Quarkus Test Cleanup Shutdown task")); - return new ExtensionState(testResourceManager, shutdownTask); + return state; } catch (Throwable e) { try { @@ -679,6 +678,7 @@ class ExtensionState implements ExtensionContext.Store.CloseableResource { private final Closeable testResourceManager; private final Closeable resource; + private final AtomicBoolean closed = new AtomicBoolean(); ExtensionState(Closeable testResourceManager, Closeable resource) { this.testResourceManager = testResourceManager; @@ -686,14 +686,28 @@ class ExtensionState implements ExtensionContext.Store.CloseableResource { } @Override - public void close() throws Throwable { - try { - resource.close(); - } finally { - if (QuarkusTestExtension.this.originalCl != null) { - setCCL(QuarkusTestExtension.this.originalCl); + public void close() { + if (closed.compareAndSet(false, true)) { + ClassLoader old = Thread.currentThread().getContextClassLoader(); + if (runningQuarkusApplication != null) { + Thread.currentThread().setContextClassLoader(runningQuarkusApplication.getClassLoader()); + } + try { + resource.close(); + } catch (Throwable e) { + log.error("Failed to shutdown Quarkus", e); + } finally { + try { + if (QuarkusTestExtension.this.originalCl != null) { + setCCL(QuarkusTestExtension.this.originalCl); + } + testResourceManager.close(); + } catch (IOException e) { + log.error("Failed to shutdown Quarkus test resources", e); + } finally { + Thread.currentThread().setContextClassLoader(old); + } } - testResourceManager.close(); } } }