Merge pull request #10206 from stuartwdouglas/close-resource

Alternate fix to make sure resources are only closed once
quarkusio · Jun 24, 2020 · 9222ccd · 9222ccd
2 parents a002dcf + 6d1b893
commit 9222ccd
Show file tree

Hide file tree

Showing 2 changed files with 37 additions and 16 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
@@ -24,6 +24,7 @@ public class TestResourceManager implements Closeable {
 
     private final List<TestResourceEntry> testResourceEntries;
     private Map<String, String> oldSystemProps;
+    private boolean started = false;
 
     public TestResourceManager(Class<?> testClass) {
         testResourceEntries = getTestResources(testClass);
@@ -40,6 +41,7 @@ public void init() {
     }
 
     public Map<String, String> start() {
+        started = true;
         Map<String, String> 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<String, String> 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
@@ -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 {
@@ -675,24 +674,40 @@ public Object resolveParameter(ParameterContext parameterContext, ExtensionConte
         }
     }
 
-    class ExtensionState {
+    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;
             this.resource = resource;
         }
 
-        public void close() throws Throwable {
-            try {
-                resource.close();
-            } finally {
-                if (QuarkusTestExtension.this.originalCl != null) {
-                    setCCL(QuarkusTestExtension.this.originalCl);
+        @Override
+        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();
             }
         }
     }