WELD-2834 Optimize in which cases Weld keeps reference to dependent i…

impl/src/main/java/org/jboss/weld/contexts/unbound/DependentContextImpl.java

@@ -22,7 +22,6 @@
 import jakarta.enterprise.context.Dependent;
 import jakarta.enterprise.context.spi.Contextual;
 import jakarta.enterprise.context.spi.CreationalContext;
-import jakarta.enterprise.inject.spi.Decorator;
 import jakarta.enterprise.inject.spi.Interceptor;
 
 import org.jboss.weld.bean.AbstractProducerBean;
@@ -35,6 +34,8 @@
 import org.jboss.weld.exceptions.UnsupportedOperationException;
 import org.jboss.weld.injection.producer.AbstractMemberProducer;
 import org.jboss.weld.injection.producer.BasicInjectionTarget;
+import org.jboss.weld.interceptor.spi.model.InterceptionModel;
+import org.jboss.weld.interceptor.spi.model.InterceptionType;
 import org.jboss.weld.serialization.spi.ContextualStore;
 
 /**
@@ -74,22 +75,27 @@ public <T> T get(Contextual<T> contextual, CreationalContext<T> creationalContex
     protected <T> void addDependentInstance(T instance, Contextual<T> contextual, WeldCreationalContext<T> creationalContext) {
         // by this we are making sure that the dependent instance has no transitive dependency with @PreDestroy / disposal method
         if (creationalContext.getDependentInstances().isEmpty()) {
-            if (contextual instanceof ManagedBean<?> && !isInterceptorOrDecorator(contextual)) {
-                ManagedBean<?> managedBean = (ManagedBean<?>) contextual;
-                if (managedBean.getProducer() instanceof BasicInjectionTarget<?>) {
-                    BasicInjectionTarget<?> injectionTarget = (BasicInjectionTarget<?>) managedBean.getProducer();
-                    if (!injectionTarget.getLifecycleCallbackInvoker().hasPreDestroyMethods()
-                            && !injectionTarget.hasInterceptors()) {
+            // handle managed beans
+            if (contextual instanceof ManagedBean<?> managedBean) {
+                if (contextual instanceof Interceptor<?>) {
+                    // Interceptors cannot have pre-destroy callbacks
+                    // At this point we know the interceptor has no dependent instances, so we needn't keep its reference
+                    // NOTE: decorators CAN have @PreDestroy callbacks!
+                    return;
+                }
+                if (managedBean.getProducer() instanceof BasicInjectionTarget<?> injectionTarget) {
+                    boolean hasPreDestroyMethods = injectionTarget.getLifecycleCallbackInvoker().hasPreDestroyMethods();
+                    boolean hasPreDestroyInt = hasPreDestroyInterceptor(managedBean);
+                    if (!hasPreDestroyMethods && !hasPreDestroyInt) {
                         // there is no @PreDestroy callback to call when destroying this dependent instance
                         // therefore, we do not need to keep the reference
+                        // Note that we need to account for @PreDestroy on the bean as well as interceptors
                         return;
                     }
                 }
             }
-            if (contextual instanceof AbstractProducerBean<?, ?, ?>) {
-                AbstractProducerBean<?, ?, ?> producerBean = (AbstractProducerBean<?, ?, ?>) contextual;
-                if (producerBean.getProducer() instanceof AbstractMemberProducer<?, ?>) {
-                    AbstractMemberProducer<?, ?> producer = (AbstractMemberProducer<?, ?>) producerBean.getProducer();
+            if (contextual instanceof AbstractProducerBean<?, ?, ?> producerBean) {
+                if (producerBean.getProducer() instanceof AbstractMemberProducer<?, ?> producer) {
                     if (producer.getDisposalMethod() == null) {
                         // there is no disposal method to call when destroying this dependent instance
                         // therefore, we do not need to keep the reference
@@ -109,10 +115,6 @@ protected <T> void addDependentInstance(T instance, Contextual<T> contextual, We
         creationalContext.addDependentInstance(beanInstance);
     }
 
-    private boolean isInterceptorOrDecorator(Contextual<?> contextual) {
-        return contextual instanceof Interceptor<?> || contextual instanceof Decorator<?>;
-    }
-
     public <T> T get(Contextual<T> contextual) {
         return get(contextual, null);
     }
@@ -137,4 +139,27 @@ private boolean isOptimizableBuiltInBean(Contextual<?> contextual) {
         }
         return false;
     }
+
+    /**
+     * Checks if the managed bean has any interceptors with @PreDestroy lifecycle callbacks.
+     * Beans with only @AroundInvoke or other non-lifecycle interceptors should not be tracked.
+     *
+     * @param managedBean the managed bean to check
+     * @return true if the bean has @PreDestroy interceptors, false otherwise
+     */
+    private boolean hasPreDestroyInterceptor(ManagedBean<?> managedBean) {
+        InterceptionModel interceptionModel = managedBean.getBeanManager().getInterceptorModelRegistry()
+                .get(managedBean.getAnnotated());
+        if (interceptionModel != null) {
+            // Check if there are any external PRE_DESTROY interceptors (method is null for lifecycle interceptors)
+            if (!interceptionModel.getInterceptors(InterceptionType.PRE_DESTROY, null).isEmpty()) {
+                return true;
+            }
+            // Check if the bean class itself has @PreDestroy interceptor methods
+            if (interceptionModel.hasTargetClassInterceptors()) {
+                return interceptionModel.getTargetClassInterceptorMetadata().isEligible(InterceptionType.PRE_DESTROY);
+            }
+        }
+        return false;
+    }
 }

tests-arquillian/src/test/java/org/jboss/weld/tests/contexts/creational/AroundInvokeBinding.java

@@ -0,0 +1,11 @@
+package org.jboss.weld.tests.contexts.creational;
+
+import java.lang.annotation.Retention;
+import java.lang.annotation.RetentionPolicy;
+
+import jakarta.interceptor.InterceptorBinding;
+
+@Retention(RetentionPolicy.RUNTIME)
+@InterceptorBinding
+public @interface AroundInvokeBinding {
+}

tests-arquillian/src/test/java/org/jboss/weld/tests/contexts/creational/AroundInvokeInterceptor.java

@@ -0,0 +1,24 @@
+package org.jboss.weld.tests.contexts.creational;
+
+import jakarta.annotation.Priority;
+import jakarta.interceptor.AroundInvoke;
+import jakarta.interceptor.Interceptor;
+import jakarta.interceptor.InvocationContext;
+
+@AroundInvokeBinding
+@Priority(1)
+@Interceptor
+public class AroundInvokeInterceptor {
+
+    public static boolean aroundInvokeTriggered = false;
+
+    @AroundInvoke
+    public Object aroundInvoke(InvocationContext ctx) throws Exception {
+        aroundInvokeTriggered = true;
+        return ctx.proceed();
+    }
+
+    public static void reset() {
+        aroundInvokeTriggered = false;
+    }
+}

tests-arquillian/src/test/java/org/jboss/weld/tests/contexts/creational/BeanWithAroundInvokeInterceptor.java

@@ -0,0 +1,12 @@
+package org.jboss.weld.tests.contexts.creational;
+
+import jakarta.enterprise.context.Dependent;
+
+@AroundInvokeBinding
+@Dependent
+public class BeanWithAroundInvokeInterceptor {
+
+    public void ping() {
+
+    }
+}

tests-arquillian/src/test/java/org/jboss/weld/tests/contexts/creational/BeanWithPreDestroyInterceptor.java

@@ -0,0 +1,11 @@
+package org.jboss.weld.tests.contexts.creational;
+
+import jakarta.enterprise.context.Dependent;
+
+@Dependent
+@PreDestroyBinding
+public class BeanWithPreDestroyInterceptor {
+
+    public void ping() {
+    }
+}

tests-arquillian/src/test/java/org/jboss/weld/tests/contexts/creational/CreationalContextTest.java

@@ -18,6 +18,7 @@
 
 import static org.junit.Assert.assertEquals;
 import static org.junit.Assert.assertNotNull;
+import static org.junit.Assert.fail;
 
 import java.util.HashSet;
 import java.util.Set;
@@ -56,7 +57,7 @@ public void testOnlyDependenciesThatRequireCleanupAreStoredInCreationalContext(B
         assertNotNull(instance.id);
 
         WeldCreationalContext<InjectedBean> wcc = (WeldCreationalContext<InjectedBean>) cc;
-        assertEquals(5, wcc.getDependentInstances().size());
+        assertEquals(6, wcc.getDependentInstances().size());
 
         @SuppressWarnings("serial")
         Set<Class<?>> expectedDependentInstanceClasses = new HashSet<Class<?>>() {
@@ -66,12 +67,24 @@ public void testOnlyDependenciesThatRequireCleanupAreStoredInCreationalContext(B
                 add(Bravo.class);
                 add(Delta.class);
                 add(String.class);
+                // Actual class will be intercepted proxy, so something like BeanWithPreDestroyInterceptor$Proxy$_$$_WeldSubclass
+                add(BeanWithPreDestroyInterceptor.class);
             }
         };
-        Set<Class<?>> actualDependentInstanceClasses = new HashSet<Class<?>>();
+        // Cannot assert equality of sets directly due to BeanWithPreDestroyInterceptor being interceptor proxy
+        // Instead, iterate and verify assignability between sets
         for (ContextualInstance<?> dependency : wcc.getDependentInstances()) {
-            actualDependentInstanceClasses.add(dependency.getInstance().getClass());
+            int found = 0;
+            for (Class<?> clazz : expectedDependentInstanceClasses) {
+                if (clazz.isAssignableFrom(dependency.getInstance().getClass())) {
+                    found++;
+                }
+            }
+            if (found != 1) {
+                fail("Failed to match actual dependent instance " + dependency.getInstance().getClass()
+                        + " to exactly one within the set of expected dependent instances: "
+                        + expectedDependentInstanceClasses);
+            }
         }
-        assertEquals(expectedDependentInstanceClasses, actualDependentInstanceClasses);
     }
 }

tests-arquillian/src/test/java/org/jboss/weld/tests/contexts/creational/InjectedBean.java

@@ -59,4 +59,12 @@ public class InjectedBean {
     @Juicy
     // retained within CreationalContext - because its dependency has @PreDestroy
     String id;
+
+    @Inject
+    // retained within creational context - it has pre-destroy interceptor
+    BeanWithPreDestroyInterceptor dependency9;
+
+    @Inject
+    // not retained, it has around invoke interceptor but no pre destroy
+    BeanWithAroundInvokeInterceptor dependency10;
 }

tests-arquillian/src/test/java/org/jboss/weld/tests/contexts/creational/MyPreDestroyInterceptor.java

@@ -0,0 +1,20 @@
+package org.jboss.weld.tests.contexts.creational;
+
+import jakarta.annotation.PreDestroy;
+import jakarta.annotation.Priority;
+import jakarta.interceptor.Interceptor;
+import jakarta.interceptor.InvocationContext;
+
+@PreDestroyBinding
+@Interceptor
+@Priority(2)
+public class MyPreDestroyInterceptor {
+
+    public static boolean preDestroyCalled = false;
+
+    @PreDestroy
+    public void preDestroy(InvocationContext invocationContext) throws Exception {
+        preDestroyCalled = true;
+        invocationContext.proceed();
+    }
+}

tests-arquillian/src/test/java/org/jboss/weld/tests/contexts/creational/PreDestroyBinding.java

@@ -0,0 +1,11 @@
+package org.jboss.weld.tests.contexts.creational;
+
+import java.lang.annotation.Retention;
+import java.lang.annotation.RetentionPolicy;
+
+import jakarta.interceptor.InterceptorBinding;
+
+@Retention(RetentionPolicy.RUNTIME)
+@InterceptorBinding
+public @interface PreDestroyBinding {
+}