AddingPassivatingScopeTest expects DeploymentException, but Weld throws DefinitionException

[arjantijms]

While running the CDI TCK (4.0.8 or higher) using

mvn clean install -U -Dit.test=AddingPassivatingScopeTest

The following is logged using GlassFish 7.0.6-SNAPSHOT, which includes Weld 5.1.0.FINAL:

17:13:20.998156  SEVERE                 main quillian.container.glassfish.clientutils.GlassFishClientUtil.getResponseMap While Deploying Application: AddingPassivatingScopeTestb781b1478f39301846c5223fa0d65747ec1f08e --exit_code: FAILURE, message: Error occurred during deployment: Exception while loading the app : CDI definition failure:Exception List with 1 exceptions:
Exception 0 :
org.jboss.weld.exceptions.DefinitionException: WELD-000118: Only normal scopes can be passivating. Scope interface org.jboss.cdi.tck.tests.full.extensions.lifecycle.bbd.broken.passivatingScope.EpochScoped
    at org.jboss.weld.bootstrap.events.BeforeBeanDiscoveryImpl.addScope(BeforeBeanDiscoveryImpl.java:92)
    at org.jboss.cdi.tck.tests.full.extensions.lifecycle.bbd.broken.passivatingScope.BeforeBeanDiscoveryObserver.observe(BeforeBeanDiscoveryObserver.java:30)
    at java.base/java.lang.reflect.Method.invoke(Method.java:568)
    at org.jboss.weld.injection.StaticMethodInjectionPoint.invoke(StaticMethodInjectionPoint.java:95)

There is an org.jboss.weld.exceptions.DefinitionException at the core here. Yet, the test is:

    @ShouldThrowException(DeploymentException.class)
    @Deployment
    public static WebArchive createTestArchive() {
        return new WebArchiveBuilder().withTestClassPackage(AddingPassivatingScopeTest.class)
                .withBeansXml(new BeansXml(BeanDiscoveryMode.ALL))
                .withExtension(BeforeBeanDiscoveryObserver.class).build();
    }

Should this be @ShouldThrowException(DefinitionException.class)? Or is Weld wrong here? Or should we just transform a DefinitionException to a DeploymentException?

[arjantijms]

As a workaround, the exception transformer can scan for "Only normal scopes can be passivating." and return a DeploymentException for that. But it feels like an error still.

[manovotn]

@arjantijms please use latest TCK version (4.0.10 ATM), this was discussed in https://github.com/jakartaee/cdi-tck/issues/431 4.0.8 was a wrongly done TCK release that can break existing impls which is what you are seeing - the reason being that the test logic was changed but it wasn't excluded so older versions of Weld won't pass it. The relevant issue is https://github.com/jakartaee/cdi-tck/pull/444

The short of it is:

• TCK test was wrong and excepted wrong exception (different than what spec said)
• TCK test was changed but not excluded (wrongly) in 4.0.8
• Latest TCK should have the test changed and excluded as well
• Weld did what TCK expected and was therefore wrong as well
• Latest Weld (5.1.1.Final) has a fix for this as well

[arjantijms]

Interesting, with TCK 4.0.10 and Weld 5.1.1 I had even more errors:

[ERROR] Failures: 
[ERROR]   ApplicationContextSharedTest>Arquillian.run:138->testApplicationContextShared:94 expected [true] but found [false]
[ERROR]   ApplicationContextSharedTest>Arquillian.run:138->testApplicationScopeActiveDuringCallToEjbTimeoutMethod:110 expected [true] but found [false]
[ERROR]   InactiveConversationTest>Arquillian.run:138->testContextNotActiveExceptionThrown:73 expected [true] but found [false]
[ERROR]   ProcessBeanAttributesNotFiredForBuiltinBean>Arquillian.run:138->testProcessBeanAttributesNotFired:51 expected [true] but found [false]
[ERROR]   BuiltinMetadataSessionBeanTest>Arquillian.run:138->testInterceptorMetadata:71 » EJB jakarta.ejb.EJBException: jakarta.ejb.CreateException: Could not create stateless EJB
[INFO] 
[ERROR] Tests run: 1833, Failures: 5, Errors: 0, Skipped: 0

But indeed, not the StackOverflow one/wrong deployment exception.

[manovotn]

Curious, I am not even aware of these tests being changed in any way and Weld changes were pretty small too. I OFC ran all the tests without container and with WFLY and had no failures there.

But indeed, not the StackOverflow one/wrong deployment exception.

I don't think I encountered SO at any point with Weld/WFLY either 🤷

[arjantijms]

I don't think I encountered SO at any point with Weld/WFLY either 🤷

This might have been specific to the GlassFish exception transformer. It sets the cause of the new Deployment of Definition exception to the original exception. The Arquillian loop gets the cause, transforms it again, causing an infinite loop.

[arjantijms]

Curious, I am not even aware of these tests being changed in any way and Weld changes were pretty small too.

The bean manager had a change it seems, which may have affected selection of the parent creational context.

On GF we're seeing the following exception now:

java.lang.IllegalArgumentException: 
    Unable to inject Session bean 
       [class org.jboss.cdi.tck.tests.implementation.builtin.metadata.session.Yoghurt with qualifiers [@Any @Default]; local interfaces are [Yoghurt] 
    into 
       [BackedAnnotatedField] @Inject private org.jboss.cdi.tck.tests.implementation.builtin.metadata.session.YoghurtInterceptor.interceptor

Where the injection target is:

@SuppressWarnings("serial")
@jakarta.interceptor.Interceptor
@Frozen
public class YoghurtInterceptor implements Serializable {

    @Inject
    private Bean<YoghurtInterceptor> bean;

    @Inject
    private Interceptor<YoghurtInterceptor> interceptor;

    @Inject
    @Intercepted
    private Bean<?> interceptedBean;

    // ...
}

and the yoghurt bean being:

@Stateless
public class Yoghurt {

    @Inject
    private Bean<Yoghurt> bean;

    public Bean<Yoghurt> getBeanBean() {
        return bean;
    }

    @Frozen
    public YoghurtInterceptor getInterceptorInstance() {
        return null;
    }

}

For some reason Weld thinks it must try to inject Yoghurt.

[arjantijms]

The exception is thrown in org.jboss.weld.bean.builtin.InterceptorMetadataBean in this method:

 @Override
    protected Interceptor<?> newInstance(InjectionPoint ip, CreationalContext<Interceptor<?>> creationalContext) {
        Contextual<?> bean = getParentCreationalContext(creationalContext).getContextual();
        if (bean instanceof Interceptor<?>) {
            return SerializableProxy.of(getBeanManager().getContextId(), (Interceptor<?>) bean);
        }
        throw new IllegalArgumentException("Unable to inject " + bean + " into " + ip);
    }

The creationalContext being passed in there has as contextual org.jboss.weld.bean.builtin.InterceptorMetadataBean (same instance for which the method is being called), and as parent a creationalContext with the dreaded SessionBeanImpl as contextual.

[manovotn]

The bean manager had a change it seems, which may have affected selection of the parent creational context.

Just the getReference() method which was in reaction to the TCK challenge (https://github.com/jakartaee/cdi-tck/issues/454) and it's fix (https://github.com/jakartaee/cdi-tck/pull/462) which showed that we were creating it incorrectly. IIRC, there was an edge case where the dependent object destruction would not be executed at all.

On GF we're seeing the following exception now:

Are you seeing this for org.jboss.cdi.tck.tests.full.implementation.builtin.metadata.BuiltinMetadataBeanTest as well? There is a very similar setup injecting and verifying Interceptor<T> but you're not mentioning that in your earlier comment.

BTW I am able to run BuiltinMetadataBeanTest with and without WFLY and pass that on Weld and I can OFC run BuiltinMetadataSessionBeanTest on WFLY and pass that too :confused:

[manovotn]

The creationalContext being passed in there has as contextual org.jboss.weld.bean.builtin.InterceptorMetadataBean (same instance for which the method is being called), and as parent a creationalContext with the dreaded SessionBeanImpl as contextual.

If I take a look at the debug of the same test on WFLY, the creationalContext I am seeing has this contextual:

• Implicit Bean [jakarta.enterprise.inject.spi.Interceptor] with qualifiers [@Default]

And it's parent context has following contextual:

• Interceptor [class org.jboss.cdi.tck.tests.implementation.builtin.metadata.session.YoghurtInterceptor intercepts @Frozen]

And just to be complete, parent's parent is what has the contextual of the session bean (Session bean [class org.jboss.cdi.tck.tests.implementation.builtin.metadata.session.Yoghurt with qualifiers [@Any @Default]; local interfaces are [Yoghurt]).

There should be a hierarchy of three CCs because the session bean will always have one(1). Then interceptors are always a dependent instances of the beans they intercept so that's stored in (1). Interceptor is a bean with its own CC(2) where it can store any dependent objects it manages (three in this case). And finally, the Interceptor<T> bean, injected into YoghurtInterceptor and stored in (2), is a bean as well and should automatically have it's own CC(3) - which is why Weld looks for parent of (3) to determine what bean it belongs to.

That's just what I can glimpse from looking at Weld code and the TCK test setup. You might want to look into how GF creates that CC and passes it around. Apparently, it is likely something in the integration code GF uses as I am not seeing these errors anywhere in my setup. Unless I am missing something obvious...

[arjantijms]

If I take a look at the debug of the same test on WFLY, the creationalContext I am seeing has this contextual: Implicit Bean [jakarta.enterprise.inject.spi.Interceptor] with qualifiers [@Default] And it's parent context has following contextual: Interceptor [class org.jboss.cdi.tck.tests.implementation.builtin.metadata.session.YoghurtInterceptor intercepts @Frozen]

I guess the first contextual is the same then; it's the one from the meta bean, but the parent is different:

Apparently, it is likely something in the integration code GF uses as I am not seeing these errors anywhere in my setup.

Likely, though it did change between Weld 4.1.0 and Weld 4.1.1. I'll try to see why it does work in 4.1.0.

[manovotn]

Likely, though it did change between Weld 4.1.0 and Weld 4.1.1. I'll try to see why it does work in 4.1.0.

@arjantijms since the CC creation changed only for invocations of BM#getReference(), try looking for usages of that in the integration code.

Also, since it's session beans, I wonder if you can reproduce this with standard CDI beans? There is bound to be more complexity between instantiation of EJB session beans and how you bind CDI interceptors to them (which I honestly don't recall at all). If there is some juggling with CC, this is where it might get tangled. Because looking at your snippet, the session bean is completely missing a child CC for the interceptor itself which seems to be the key problem here.

[arjantijms]

since the CC creation changed only for invocations of BM#getReference(), try looking for usages of that in the integration code.

Thanks for the hint, that seems to be indeed the issue.

For some reason a boolean was added that prevents creating the proper creational context.

In 5.1.1

    @Override
    public Object getReference(Bean<?> bean, Type requestedType, CreationalContext<?> creationalContext) {
        Preconditions.checkArgumentNotNull(bean, "bean");
        Preconditions.checkArgumentNotNull(requestedType, "requestedType");
        Preconditions.checkArgumentNotNull(creationalContext, CREATIONAL_CONTEXT);
        if (!BeanTypeAssignabilityRules.instance().matches(requestedType, bean.getTypes())) {
            throw BeanManagerLogger.LOG.specifiedTypeNotBeanType(requestedType, bean);
        }
        // Ensure that there is no injection point associated
        final ThreadLocalStackReference<InjectionPoint> stack = currentInjectionPoint.push(EmptyInjectionPoint.INSTANCE);
        try {
            return getReference(bean, requestedType, creationalContext, false, false);
        } finally {
            stack.pop();
        }
    }

private Object getReference(Bean<?> bean, Type requestedType, CreationalContext<?> creationalContext, boolean noProxy, boolean createChildCc) {
        if (creationalContext instanceof CreationalContextImpl<?> && createChildCc) { // TRUE && FALSE
            creationalContext = ((CreationalContextImpl<?>) creationalContext).getCreationalContext(bean);
        }
        if (!noProxy && isProxyRequired(bean)) {
            if (creationalContext != null || ContextualInstance.getIfExists(bean, this) != null) {
                if (requestedType == null) {
                    return clientProxyProvider.getClientProxy(bean);
                } else {
                    return clientProxyProvider.getClientProxy(bean, requestedType);
                }
            } else {
                return null;
            }
        } else {
            return ContextualInstance.get(bean, this, creationalContext);
        }
    }

And in 5.1.0:

public Object getReference(Bean<?> bean, Type requestedType, CreationalContext<?> creationalContext) {
        Preconditions.checkArgumentNotNull(bean, "bean");
        Preconditions.checkArgumentNotNull(requestedType, "requestedType");
        Preconditions.checkArgumentNotNull(creationalContext, CREATIONAL_CONTEXT);
        if (!BeanTypeAssignabilityRules.instance().matches(requestedType, bean.getTypes())) {
            throw BeanManagerLogger.LOG.specifiedTypeNotBeanType(requestedType, bean);
        }
        // Ensure that there is no injection point associated
        final ThreadLocalStackReference<InjectionPoint> stack = currentInjectionPoint.push(EmptyInjectionPoint.INSTANCE);
        try {
            return getReference(bean, requestedType, creationalContext, false);
        } finally {
            stack.pop();
        }
    }

public Object getReference(Bean<?> bean, Type requestedType, CreationalContext<?> creationalContext, boolean noProxy) {
        if (creationalContext instanceof CreationalContextImpl<?>) { // TRUE
            creationalContext = ((CreationalContextImpl<?>) creationalContext).getCreationalContext(bean);
        }
        if (!noProxy && isProxyRequired(bean)) {
            if (creationalContext != null || ContextualInstance.getIfExists(bean, this) != null) {
                if (requestedType == null) {
                    return clientProxyProvider.getClientProxy(bean);
                } else {
                    return clientProxyProvider.getClientProxy(bean, requestedType);
                }
            } else {
                return null;
            }
        } else {
            return ContextualInstance.get(bean, this, creationalContext);
        }
    }

I'm not really sure why you would purposely skip creating a required contextual context AND default to that (instead of making it an optional thing), but I'm also sure there's probably a good reason I'm not aware of.

I'll try to see how I can work around this.

Thanks again!

[manovotn]

Glad it helped you pinpoint the issue! :)

I'm not really sure why you would purposely skip creating a required contextual context AND default to that (instead of making it an optional thing), but I'm also sure there's probably a good reason I'm not aware of.

The reason is partially visible in the changed TCK test I linked earlier (https://github.com/jakartaee/cdi-tck/pull/462) and partially from the test in the Weld PR - they are similar but not identical. With previous code, there were some cases in which beans intantiated this way would not correctly destroy their dependant instances when they got destroyed.

I'll try to see how I can work around this.

I have no idea how the GF code around this looks like (feel free to share some links and bits) but if you can't find a way around this, we can also look into exposing some API in Weld that would allow you to get the bean reference similarly to what it was, it just shouldn't be default way. That being said, with direct dep on Weld internals, you should be able to achieve similar CC structure.

[arjantijms]

The workaround or the solution was to cast WeldManager (as obtained from WeldBootstrap) to BeanManagerImpl.

Even when depending directly on Weld internals, having to cast always feels like you're doing something you maybe should not be doing. It might be better if the getReference() method with the extra boolean and the previous behaviour was in WeldManager?

see: https://github.com/eclipse-ee4j/glassfish/pull/24480/files#diff-632c2418ab3227ac685a60ae02348a9683a24be6a2f15953d700367fb37ea623R387

[manovotn]

That's some good information there, thanks! I can see this is happening only in case of resolving interceptors this way - why does GF need to manually obtain their instances? Is it in some way reproducing the interceptor chain for EJBs?

To give some more context - the ability to resolve Interceptor instance via BM#getReference() itself is questionable (in pure CDI terms) as interceptors are dependent objects of the beans they intercept so resolving them separately is...weird. However, at least in Weld this somehow worked; whether by accident or intentionally, I do not know. What I do know is that even Weld didn't have any test coverage for this. Nonetheless, the difference between using BM#getReference() for standard bean(1) and for interceptor(2) lies in the creational context object you pass in. In case of (1) it is simply the CC of that given bean but for (2) you'd actually need to pass in CC with a parent (because interceptor is dependent) that's not really doable via non-Weld APIs.

It might be better if the getReference() method with the extra boolean and the previous behaviour was in WeldManager?

Instead, WeldManager could expose a method allowing you to create a child CC that you can then pass to getReference() So something like:

<T> CreationalContext<T> createChildCreationalContext(Contextual<T> contextual, CreationalContext<?> parent);

Where contextual is the Interceptor bean and parent is the CC you have.

Or, better still, I could fix the getReference() implementation to keep creating child CC in case the Bean parameter is instance of Interceptor. I think that would be the most sensible approach.

[ljnelson]

This is a CDI issue, and we're discussing Weld, but since we're discussing Weld….

I'm not sure the issue is related to some special characteristic of interceptors. I think it has to do with Dependent-scoped dependencies of Dependent-scoped objects.

I think the fix that Weld put in in getReference in 5.1.1.Final happens to cause one very specific test to pass while conveniently not ever really getting involved in other tests, so you don't necessarily see what it's doing elsewhere (particularly since getInjectableReference completely bypasses this path and continues to create (properly, IMHO) child CCs "all the way down").

The root cause of what Arjan is observing is the orphaning of a "second level" CreationalContext child that is supposed to handle the destruction of Dependent-scoped dependencies of a Dependent-scoped object. Interceptors are always Dependent-scoped, so Dependent-scoped dependencies of them (such as TransactionalInterceptorDependency in the test under discussion) happen to illustrate this problem.

In 5.1.0.Final, where CCs are properly created hierarchically "all the way down" in all cases, the CC holding TransactionalInterceptorDependency is effectively orphaned. It points to its parent, but its parent can't "see" it. So when you destroy AccountTransaction, its direct Dependent-scoped objects—just the TransactionalInterceptor—are destroyed. However, by the time you invoke destroy on the TransactionalInterceptor bean, the CreationalContext holding its dependencies is no longer reachable, because the CreationalContext being released can't "see" its child. Anything housed in that child just sits there and the test fails.

The fix (telling getReference to not create child CCs in certain cases) seems to basically result in the sharing of sorts of a CreationalContext in specific cases, so that (Dependent-scoped) TransactionalInterceptor's dependencies—namely (also Dependent-scoped) TransactionalInterceptorDependency—are housed "alongside" it. When you destroy AccountTransaction, and thus release the CC housing TransactionalInterceptor, you therefore destroy TransactionalInterceptor (of course) but also, by a sort of happenstance, TransactionalInterceptorDependency. The test passes, but something "feels" very off.

Additionally, this breaks the current (apparently incidental?!) behavior of getReference which previously worked "all the way down" and in all cases to enable this particular test to pass (which it does).

I think a better solution might be to restore the (IMHO correct) behavior of every prior version of Weld's implementation of getReference (as I think Arjan is also suggesting?) and somehow (not sure how yet) special-case the situation where you have a Dependent-scoped object with Dependent-scoped dependent objects. In this case if you could "unorphan" the CreationalContexts present at its dependents' creation I think the test would pass and this multi-level destruction that the test relies on would be carried out more idiomatically.

[ljnelson]

See also:

• https://issues.redhat.com/browse/WELD-2745
• https://github.com/weld/core/pull/2848/

[manovotn]

@arjantijms you should no longer need the workaround with Weld 5.1.1.SP1, the behavior of BM#getReference should be back to what it did before. The release will land in Central momentarily, it'd be great if you could give it a shot. See http://weld.cdi-spec.org/news/2023/07/11/weld-511SP1/

[arjantijms]

@manovotn Thanks! I'll try it out

[arjantijms]

@manovotn Results seem good. TCK passes and PR done: https://github.com/eclipse-ee4j/glassfish/pull/24498

[manovotn]

Glad to hear that, can we close this issue?

[arjantijms]

Yes, I'll close it. Thank you so much for all your kind help and patience! :)