Merge pull request #38069 from mkouba/issue-37982-36

[3.6] ArC: fix and optimize the ContextInstances abstraction
quarkusio · Jan 8, 2024 · a9532e6 · a9532e6
2 parents 9bca238 + c65bc86
commit a9532e6
Show file tree

Hide file tree

Showing 10 changed files with 361 additions and 133 deletions.
diff --git a/...jects/arc/processor/src/main/java/io/quarkus/arc/processor/ContextInstancesGenerator.java b/...jects/arc/processor/src/main/java/io/quarkus/arc/processor/ContextInstancesGenerator.java
diff --git a/...dent-projects/arc/processor/src/main/java/io/quarkus/arc/processor/MethodDescriptors.java b/...dent-projects/arc/processor/src/main/java/io/quarkus/arc/processor/MethodDescriptors.java
@@ -11,6 +11,7 @@
 import java.util.Set;
 import java.util.concurrent.locks.Lock;
 import java.util.function.BiFunction;
+import java.util.function.Consumer;
 import java.util.function.Function;
 import java.util.function.Supplier;
 
@@ -63,6 +64,9 @@ public final class MethodDescriptors {
 
     public static final MethodDescriptor SUPPLIER_GET = MethodDescriptor.ofMethod(Supplier.class, "get", Object.class);
 
+    public static final MethodDescriptor CONSUMER_ACCEPT = MethodDescriptor.ofMethod(Consumer.class, "accept",
+            void.class, Object.class);
+
     public static final MethodDescriptor CREATIONAL_CTX_CHILD = MethodDescriptor.ofMethod(CreationalContextImpl.class, "child",
             CreationalContextImpl.class,
             CreationalContext.class);

diff --git a/...endent-projects/arc/runtime/src/main/java/io/quarkus/arc/impl/AbstractInstanceHandle.java b/...endent-projects/arc/runtime/src/main/java/io/quarkus/arc/impl/AbstractInstanceHandle.java
@@ -7,17 +7,13 @@
 import jakarta.enterprise.context.Dependent;
 import jakarta.enterprise.context.spi.CreationalContext;
 
-import org.jboss.logging.Logger;
-
 import io.quarkus.arc.Arc;
 import io.quarkus.arc.InjectableBean;
 import io.quarkus.arc.InjectableContext;
 import io.quarkus.arc.InstanceHandle;
 
 abstract class AbstractInstanceHandle<T> implements InstanceHandle<T> {
 
-    private static final Logger LOGGER = Logger.getLogger(AbstractInstanceHandle.class.getName());
-
     @SuppressWarnings("rawtypes")
     private static final AtomicIntegerFieldUpdater<AbstractInstanceHandle> DESTROYED_UPDATER = AtomicIntegerFieldUpdater
             .newUpdater(AbstractInstanceHandle.class, "destroyed");

diff --git a/...pendent-projects/arc/runtime/src/main/java/io/quarkus/arc/impl/AbstractSharedContext.java b/...pendent-projects/arc/runtime/src/main/java/io/quarkus/arc/impl/AbstractSharedContext.java
@@ -1,9 +1,6 @@
 package io.quarkus.arc.impl;
 
-import java.util.Iterator;
-import java.util.Map;
-import java.util.Objects;
-import java.util.Set;
+import java.util.*;
 import java.util.function.Supplier;
 import java.util.stream.Collectors;
 
@@ -87,19 +84,24 @@ public void destroy(Contextual<?> contextual) {
 
     @Override
     public synchronized void destroy() {
+        // Note that shared contexts are usually only destroyed when the app stops
+        // I.e. we don't need to use the optimized ContextInstances methods here
         Set<ContextInstanceHandle<?>> values = instances.getAllPresent();
+        if (values.isEmpty()) {
+            return;
+        }
         // Destroy the producers first
-        for (Iterator<ContextInstanceHandle<?>> iterator = values.iterator(); iterator.hasNext();) {
-            ContextInstanceHandle<?> instanceHandle = iterator.next();
+        for (Iterator<ContextInstanceHandle<?>> it = values.iterator(); it.hasNext();) {
+            ContextInstanceHandle<?> instanceHandle = it.next();
             if (instanceHandle.getBean().getDeclaringBean() != null) {
                 instanceHandle.destroy();
-                iterator.remove();
+                it.remove();
             }
         }
         for (ContextInstanceHandle<?> instanceHandle : values) {
             instanceHandle.destroy();
         }
-        instances.clear();
+        instances.removeEach(null);
     }
 
     @Override

diff --git a/...rojects/arc/runtime/src/main/java/io/quarkus/arc/impl/ComputingCacheContextInstances.java b/...rojects/arc/runtime/src/main/java/io/quarkus/arc/impl/ComputingCacheContextInstances.java
@@ -1,6 +1,7 @@
 package io.quarkus.arc.impl;
 
 import java.util.Set;
+import java.util.function.Consumer;
 import java.util.function.Supplier;
 
 import io.quarkus.arc.ContextInstanceHandle;
@@ -34,7 +35,10 @@ public Set<ContextInstanceHandle<?>> getAllPresent() {
     }
 
     @Override
-    public void clear() {
+    public void removeEach(Consumer<? super ContextInstanceHandle<?>> action) {
+        if (action != null) {
+            instances.getPresentValues().forEach(action);
+        }
         instances.clear();
     }
 

diff --git a/...ent-projects/arc/runtime/src/main/java/io/quarkus/arc/impl/ContextInstanceHandleImpl.java b/...ent-projects/arc/runtime/src/main/java/io/quarkus/arc/impl/ContextInstanceHandleImpl.java
@@ -2,6 +2,8 @@
 
 import jakarta.enterprise.context.spi.CreationalContext;
 
+import org.jboss.logging.Logger;
+
 import io.quarkus.arc.ContextInstanceHandle;
 import io.quarkus.arc.InjectableBean;
 
@@ -12,13 +14,19 @@
  */
 public class ContextInstanceHandleImpl<T> extends EagerInstanceHandle<T> implements ContextInstanceHandle<T> {
 
+    private static final Logger LOG = Logger.getLogger(ContextInstanceHandleImpl.class);
+
     public ContextInstanceHandleImpl(InjectableBean<T> bean, T instance, CreationalContext<T> creationalContext) {
         super(bean, instance, creationalContext);
     }
 
     @Override
     public void destroy() {
-        destroyInternal();
+        try {
+            destroyInternal();
+        } catch (Exception e) {
+            LOG.error("Unable to destroy instance" + get(), e);
+        }
     }
 
 }
diff --git a/independent-projects/arc/runtime/src/main/java/io/quarkus/arc/impl/ContextInstances.java b/independent-projects/arc/runtime/src/main/java/io/quarkus/arc/impl/ContextInstances.java
@@ -1,20 +1,46 @@
 package io.quarkus.arc.impl;
 
 import java.util.Set;
+import java.util.function.Consumer;
 import java.util.function.Supplier;
 
 import io.quarkus.arc.ContextInstanceHandle;
 
 public interface ContextInstances {
 
+    /**
+     *
+     * @param id
+     * @param supplier
+     * @return the instance handle
+     */
     ContextInstanceHandle<?> computeIfAbsent(String id, Supplier<ContextInstanceHandle<?>> supplier);
 
+    /**
+     *
+     * @param id
+     * @return the instance handle if present, {@code null} otherwise
+     */
     ContextInstanceHandle<?> getIfPresent(String id);
 
+    /**
+     *
+     * @param id
+     * @return the removed instance handle, or {@code null}
+     */
     ContextInstanceHandle<?> remove(String id);
 
+    /**
+     *
+     * @return all instance handles
+     */
     Set<ContextInstanceHandle<?>> getAllPresent();
 
-    void clear();
+    /**
+     * Removes all instance handles and performs the given action (if present) for each handle.
+     *
+     * @param action
+     */
+    void removeEach(Consumer<? super ContextInstanceHandle<?>> action);
 
 }
diff --git a/independent-projects/arc/runtime/src/main/java/io/quarkus/arc/impl/RequestContext.java b/independent-projects/arc/runtime/src/main/java/io/quarkus/arc/impl/RequestContext.java
@@ -210,8 +210,7 @@ public void destroy(ContextState state) {
             if (reqState.invalidate()) {
                 // Fire an event with qualifier @BeforeDestroyed(RequestScoped.class) if there are any observers for it
                 fireIfNotEmpty(beforeDestroyedNotifier);
-                reqState.contextInstances.getAllPresent().forEach(this::destroyContextElement);
-                reqState.contextInstances.clear();
+                reqState.contextInstances.removeEach(ContextInstanceHandle::destroy);
                 // Fire an event with qualifier @Destroyed(RequestScoped.class) if there are any observers for it
                 fireIfNotEmpty(destroyedNotifier);
             }
@@ -229,14 +228,6 @@ private static void traceDestroy(ContextState state) {
         LOG.tracef("Destroy %s%s\n\t...", state != null ? Integer.toHexString(state.hashCode()) : "", stack);
     }
 
-    private void destroyContextElement(ContextInstanceHandle<?> contextInstanceHandle) {
-        try {
-            contextInstanceHandle.destroy();
-        } catch (Exception e) {
-            throw new IllegalStateException("Unable to destroy instance" + contextInstanceHandle.get(), e);
-        }
-    }
-
     private void fireIfNotEmpty(Notifier<Object> notifier) {
         if (notifier != null && !notifier.isEmpty()) {
             try {

diff --git a/...a/io/quarkus/arc/test/contexts/application/optimized/ApplicationContextInstancesTest.java b/...a/io/quarkus/arc/test/contexts/application/optimized/ApplicationContextInstancesTest.java
@@ -4,9 +4,17 @@
 import static org.junit.jupiter.api.Assertions.assertNotEquals;
 
 import java.util.UUID;
+import java.util.concurrent.ExecutionException;
+import java.util.concurrent.ExecutorService;
+import java.util.concurrent.Executors;
+import java.util.concurrent.Future;
+import java.util.concurrent.TimeUnit;
+import java.util.concurrent.TimeoutException;
 
 import jakarta.annotation.PostConstruct;
+import jakarta.annotation.PreDestroy;
 import jakarta.enterprise.context.ApplicationScoped;
+import jakarta.inject.Inject;
 
 import org.junit.jupiter.api.Test;
 import org.junit.jupiter.api.extension.RegisterExtension;
@@ -21,7 +29,7 @@ public class ApplicationContextInstancesTest {
 
     @RegisterExtension
     ArcTestContainer container = ArcTestContainer.builder()
-            .beanClasses(Boom.class)
+            .beanClasses(Boom.class, Bim.class)
             .optimizeContexts(true)
             .build();
 
@@ -30,15 +38,23 @@ public void testContext() {
         ArcContainer container = Arc.container();
         InstanceHandle<Boom> handle = container.instance(Boom.class);
         Boom boom = handle.get();
+        // ContextInstances#computeIfAbsent()
         String id1 = boom.ping();
         assertEquals(id1, boom.ping());
 
+        // ContextInstances#remove()
         handle.destroy();
+        // Bim bean is not destroyed
+        // ContextInstances#getAllPresent()
+        assertEquals(1, container.getActiveContext(ApplicationScoped.class).getState().getContextualInstances().size());
+
+        // Init a new instance of Boom
         String id2 = boom.ping();
         assertNotEquals(id1, id2);
         assertEquals(id2, boom.ping());
 
         InjectableContext appContext = container.getActiveContext(ApplicationScoped.class);
+        // ContextInstances#removeEach()
         appContext.destroy();
         assertNotEquals(id2, boom.ping());
     }
@@ -48,13 +64,41 @@ public static class Boom {
 
         private String id;
 
+        @Inject
+        Bim bim;
+
         String ping() {
             return id;
         }
 
         @PostConstruct
         void init() {
             id = UUID.randomUUID().toString();
+
+            ExecutorService executorService = Executors.newSingleThreadExecutor();
+            Future<?> f = executorService.submit(() -> {
+                // Force the init of the bean on a different thread
+                bim.bam();
+            });
+            try {
+                f.get(2, TimeUnit.SECONDS);
+            } catch (InterruptedException | ExecutionException | TimeoutException e) {
+                throw new IllegalStateException(e);
+            }
+            executorService.shutdownNow();
+        }
+
+        @PreDestroy
+        void destroy() {
+            throw new IllegalStateException("Boom");
+        }
+
+    }
+
+    @ApplicationScoped
+    public static class Bim {
+
+        public void bam() {
         }
 
     }