WELD 2745 Restore proper getReference behavior from 5.1.0.Final and repair CreationalContextImpl destruction and releasing logic

impl/src/main/java/org/jboss/weld/contexts/CreationalContextImpl.java

@@ -21,10 +21,12 @@
 import java.util.ArrayList;
 import java.util.Collections;
 import java.util.HashMap;
+import java.util.HashSet;
 import java.util.Iterator;
 import java.util.LinkedList;
 import java.util.List;
 import java.util.Map;
+import java.util.Set;
 
 import jakarta.enterprise.context.spi.Contextual;
 import jakarta.enterprise.context.spi.CreationalContext;
@@ -60,6 +62,9 @@ public class CreationalContextImpl<T> implements CreationalContext<T>, WeldCreat
 
     private final CreationalContextImpl<?> parentCreationalContext;
 
+    // Precondition for access when non-null: synchronized (dependentInstances)
+    private transient Set<ContextualInstance<?>> destroyed;
+
     private transient List<ResourceReference<?>> resourceReferences;
 
     private transient boolean constructorInterceptionSuppressed;
@@ -126,9 +131,12 @@ public void release() {
     public void release(Contextual<T> contextual, T instance) {
         synchronized (dependentInstances) {
             for (ContextualInstance<?> dependentInstance : dependentInstances) {
-                // do not destroy contextual again, since it's just being destroyed
                 if (contextual == null || !(dependentInstance.getContextual().equals(contextual))) {
                     destroy(dependentInstance);
+                } else {
+                    // do not destroy contextual again, since it's just being destroyed, but make sure its dependencies
+                    // are destroyed
+                    release(dependentInstance);
                 }
             }
         }
@@ -139,8 +147,29 @@ public void release(Contextual<T> contextual, T instance) {
         }
     }
 
-    private static <T> void destroy(ContextualInstance<T> beanInstance) {
-        beanInstance.getContextual().destroy(beanInstance.getInstance(), beanInstance.getCreationalContext());
+    private <T> void destroy(ContextualInstance<T> beanInstance) {
+        // Precondition: synchronized (dependentInstances)
+        if (this.destroyed == null) {
+            this.destroyed = new HashSet<>();
+        }
+        if (this.destroyed.add(beanInstance)) {
+            beanInstance.getContextual().destroy(beanInstance.getInstance(), beanInstance.getCreationalContext());
+        }
+    }
+
+    private <T> void release(ContextualInstance<T> beanInstance) {
+        // Precondition: synchronized (dependentInstances)
+        if (this.destroyed == null) {
+            this.destroyed = new HashSet<>();
+        }
+        if (this.destroyed.add(beanInstance)) {
+            CreationalContext<T> cc = beanInstance.getCreationalContext();
+            if (cc instanceof CreationalContextImpl<?>) {
+                ((CreationalContextImpl<T>) cc).release(beanInstance.getContextual(), beanInstance.getInstance());
+            } else {
+                cc.release();
+            }
+        }
     }
 
     /**

impl/src/main/java/org/jboss/weld/manager/BeanManagerImpl.java

@@ -663,11 +663,7 @@ private Context internalGetContext(Class<? extends Annotation> scopeType) {
     }
 
     public Object getReference(Bean<?> bean, Type requestedType, CreationalContext<?> creationalContext, boolean noProxy) {
-        return getReference(bean, requestedType, creationalContext, noProxy, true);
-    }
-
-    private Object getReference(Bean<?> bean, Type requestedType, CreationalContext<?> creationalContext, boolean noProxy, boolean createChildCc) {
-        if (creationalContext instanceof CreationalContextImpl<?> && createChildCc) {
+        if (creationalContext instanceof CreationalContextImpl<?>) {
             creationalContext = ((CreationalContextImpl<?>) creationalContext).getCreationalContext(bean);
         }
         if (!noProxy && isProxyRequired(bean)) {
@@ -704,7 +700,7 @@ public Object getReference(Bean<?> bean, Type requestedType, CreationalContext<?
         // Ensure that there is no injection point associated
         final ThreadLocalStackReference<InjectionPoint> stack = currentInjectionPoint.push(EmptyInjectionPoint.INSTANCE);
         try {
-            return getReference(bean, requestedType, creationalContext, false, false);
+            return getReference(bean, requestedType, creationalContext, false);
         } finally {
             stack.pop();
         }

tests-arquillian/src/test/java/org/jboss/weld/tests/beanManager/getReference/interceptor/ManualInterceptorInstanceRetrievalTest.java

@@ -0,0 +1,46 @@
+package org.jboss.weld.tests.beanManager.getReference.interceptor;
+
+import jakarta.enterprise.context.spi.CreationalContext;
+import jakarta.enterprise.inject.spi.BeanManager;
+import jakarta.enterprise.inject.spi.InterceptionType;
+import jakarta.enterprise.inject.spi.Interceptor;
+import jakarta.inject.Inject;
+import org.jboss.arquillian.container.test.api.Deployment;
+import org.jboss.arquillian.junit.Arquillian;
+import org.jboss.shrinkwrap.api.Archive;
+import org.jboss.shrinkwrap.api.BeanArchive;
+import org.jboss.shrinkwrap.api.ShrinkWrap;
+import org.jboss.weld.test.util.Utils;
+import org.junit.Assert;
+import org.junit.Test;
+import org.junit.runner.RunWith;
+
+import java.util.List;
+
+/**
+ * NOTE: The functionality tested here (using BM#getReference for interceptor instances) isn't rooted in CDI spec but
+ * seems to be something intergrators have relied on in the past. One such example is GF resolving interceptors as part
+ * of their EJB integration. Another case used to be MP REST using this to simulate their own interceptor chain.
+ */
+@RunWith(Arquillian.class)
+public class ManualInterceptorInstanceRetrievalTest {
+
+    @Deployment
+    public static Archive<?> getDeployment() {
+        return ShrinkWrap.create(BeanArchive.class,
+                Utils.getDeploymentNameAsHash(ManualInterceptorInstanceRetrievalTest.class)).addPackage(ManualInterceptorInstanceRetrievalTest.class.getPackage());
+    }
+
+    @Inject
+    BeanManager bm;
+
+    @Test
+    public void testGetReferenceForInterceptorInstance() {
+        List<Interceptor<?>> interceptors = bm.resolveInterceptors(InterceptionType.AROUND_INVOKE, MyBinding.Literal.INSTANCE);
+        Assert.assertTrue(interceptors.size() == 1);
+        Interceptor<?> interceptor = interceptors.get(0);
+        CreationalContext<?> creationalContext = bm.createCreationalContext(interceptor);
+        Object reference = bm.getReference(interceptor, MyInterceptor.class, creationalContext);
+        Assert.assertNotNull(reference);
+    }
+}

tests-arquillian/src/test/java/org/jboss/weld/tests/beanManager/getReference/interceptor/MyBean.java

@@ -0,0 +1,12 @@
+package org.jboss.weld.tests.beanManager.getReference.interceptor;
+
+import jakarta.enterprise.context.Dependent;
+
+@Dependent
+@MyBinding
+public class MyBean {
+
+    public void ping() {
+
+    }
+}

tests-arquillian/src/test/java/org/jboss/weld/tests/beanManager/getReference/interceptor/MyBinding.java

@@ -0,0 +1,21 @@
+package org.jboss.weld.tests.beanManager.getReference.interceptor;
+
+import jakarta.enterprise.util.AnnotationLiteral;
+import jakarta.interceptor.InterceptorBinding;
+
+import java.lang.annotation.ElementType;
+import java.lang.annotation.Retention;
+import java.lang.annotation.RetentionPolicy;
+import java.lang.annotation.Target;
+
+@InterceptorBinding
+@Retention(RetentionPolicy.RUNTIME)
+@Target({ElementType.TYPE, ElementType.METHOD, ElementType.FIELD, ElementType.PARAMETER})
+public @interface MyBinding {
+
+    public static class Literal extends AnnotationLiteral<MyBinding> implements MyBinding {
+
+        public static final Literal INSTANCE = new Literal();
+
+    }
+}

tests-arquillian/src/test/java/org/jboss/weld/tests/beanManager/getReference/interceptor/MyInterceptor.java

@@ -0,0 +1,24 @@
+package org.jboss.weld.tests.beanManager.getReference.interceptor;
+
+import jakarta.annotation.Priority;
+import jakarta.enterprise.inject.Intercepted;
+import jakarta.enterprise.inject.spi.Bean;
+import jakarta.inject.Inject;
+import jakarta.interceptor.AroundInvoke;
+import jakarta.interceptor.Interceptor;
+import jakarta.interceptor.InvocationContext;
+
+@Interceptor
+@Priority(1)
+@MyBinding
+public class MyInterceptor {
+
+    @Inject
+    @Intercepted
+    Bean<?> bean;
+
+    @AroundInvoke
+    public Object intercept(InvocationContext ic) throws Exception {
+        return ic.proceed();
+    }
+}

tests-arquillian/src/test/java/org/jboss/weld/tests/beanManager/getReference/synthBean/Child.java

@@ -0,0 +1,4 @@
+package org.jboss.weld.tests.beanManager.getReference.synthBean;
+
+public class Child {
+}

tests-arquillian/src/test/java/org/jboss/weld/tests/beanManager/getReference/synthBean/MyExtension.java

@@ -0,0 +1,52 @@
+package org.jboss.weld.tests.beanManager.getReference.synthBean;
+
+import jakarta.enterprise.context.Dependent;
+import jakarta.enterprise.context.spi.CreationalContext;
+import jakarta.enterprise.event.Observes;
+import jakarta.enterprise.inject.spi.AfterBeanDiscovery;
+import jakarta.enterprise.inject.spi.BeanManager;
+import jakarta.enterprise.inject.spi.Extension;
+import jakarta.inject.Singleton;
+
+public class MyExtension implements Extension {
+
+    public static boolean childCreated;
+    public static boolean childDestroyed;
+    public static boolean parentCreated;
+    public static boolean parentDestroyed;
+
+    final void registerBeans(@Observes final AfterBeanDiscovery event, final BeanManager bm) {
+        event.addBean()
+                .addTransitiveTypeClosure(Child.class)
+                .scope(Dependent.class)
+                .createWith((CreationalContext<Child> cc) -> createChild(cc))
+                .destroyWith((child, cc) -> destroyChild(cc));
+        event.addBean()
+                .addTransitiveTypeClosure(Parent.class)
+                .scope(Singleton.class)
+                .createWith((CreationalContext<Parent> cc) -> createParent(bm, cc))
+                .destroyWith((parent, cc) -> destroyParent(cc));
+    }
+
+    private Child createChild(final CreationalContext<Child> cc) {
+        final Child c = new Child();
+        this.childCreated = true;
+        return c;
+    }
+
+    private void destroyChild(CreationalContext<Child> cc) {
+        this.childDestroyed = true;
+        cc.release();
+    }
+
+    private Parent createParent(final BeanManager bm, final CreationalContext<Parent> cc) {
+        final Parent p = new Parent((Child)bm.getReference(bm.resolve(bm.getBeans(Child.class)), Child.class, cc));
+        this.parentCreated = true;
+        return p;
+    }
+
+    private void destroyParent(final CreationalContext<Parent> cc) {
+        this.parentDestroyed = true;
+        cc.release();
+    }
+}

tests-arquillian/src/test/java/org/jboss/weld/tests/beanManager/getReference/synthBean/Parent.java

@@ -0,0 +1,11 @@
+package org.jboss.weld.tests.beanManager.getReference.synthBean;
+
+public class Parent {
+
+    private final Child child;
+
+    Parent(final Child child) {
+        super();
+        this.child = child;
+    }
+}

tests-arquillian/src/test/java/org/jboss/weld/tests/beanManager/getReference/synthBean/SimulateSynthBeanCreationalContextHierarchyTest.java

@@ -0,0 +1,53 @@
+package org.jboss.weld.tests.beanManager.getReference.synthBean;
+
+import static org.junit.Assert.assertTrue;
+
+import jakarta.enterprise.context.spi.AlterableContext;
+import jakarta.enterprise.context.spi.CreationalContext;
+import jakarta.enterprise.inject.spi.Bean;
+import jakarta.enterprise.inject.spi.BeanManager;
+import jakarta.enterprise.inject.spi.Extension;
+import jakarta.inject.Inject;
+import jakarta.inject.Singleton;
+import org.jboss.arquillian.container.test.api.Deployment;
+import org.jboss.arquillian.junit.Arquillian;
+import org.jboss.shrinkwrap.api.Archive;
+import org.jboss.shrinkwrap.api.BeanArchive;
+import org.jboss.shrinkwrap.api.ShrinkWrap;
+import org.jboss.weld.test.util.Utils;
+import org.junit.Test;
+import org.junit.runner.RunWith;
+
+/**
+ * NOTE: The functionality this test asserts is not explicitly stated in the spec but it turned out to be relied on in
+ * some cases. We therefore want to have a test coverage for it.
+ * <p>
+ * This test aims to create two synthetic beans and use their creational context to create a link between then so that
+ * once one gets destroyed, so should the other.
+ */
+@RunWith(Arquillian.class)
+public class SimulateSynthBeanCreationalContextHierarchyTest {
+
+    @Deployment
+    public static Archive<?> getDeployment() {
+        return ShrinkWrap.create(BeanArchive.class, Utils.getDeploymentNameAsHash(SimulateSynthBeanCreationalContextHierarchyTest.class))
+                .addPackage(SimulateSynthBeanCreationalContextHierarchyTest.class.getPackage())
+                .addAsServiceProvider(Extension.class, MyExtension.class);
+    }
+
+    @Inject
+    BeanManager bm;
+
+    @Test
+    public void testSimulatingCCHierarchyOnSyntBeans() {
+        final Bean<Parent> pb = (Bean<Parent>) bm.resolve(bm.getBeans(Parent.class));
+        final CreationalContext<Parent> pcc = bm.createCreationalContext(pb);
+        final Parent p = (Parent) bm.getReference(pb, Parent.class, pcc);
+        assertTrue(MyExtension.parentCreated);
+        assertTrue(MyExtension.childCreated);
+        final AlterableContext singletonContext = (AlterableContext) bm.getContext(Singleton.class);
+        singletonContext.destroy(pb);
+        assertTrue(MyExtension.parentDestroyed);
+        assertTrue(MyExtension.childDestroyed);
+    }
+}