Hi Michal,

It seems to me that the exception-converting interceptor you have is deployed as a plain jee interceptor. The CMTTxInterceptor we see in the stack trace, is the one that implements handling the ejb transaction configuration (e.g., invokeInCallerTx() for a MANDATORY or REQUIRED transaction attribute when a transaction is already available and invokeInOurTx() for REQUIRES_NEW or REQUIRED when no transaction is available). As such, it can be used to retry the part of the interceptor stack that throws the ApplicationDeadlockException, in a new transaction (invokeInOurTx).

However, deploying as a container interceptor (as described in https://docs.jboss.org/author/display/AS72/Container+interceptors), an interceptor that implements re-running part of the interceptor stack (by re-invoking proceed(), similar to what CMTTxInterceptor does), the result is the same: org.jboss.invocation.CannotProceedException: INV-00002:Invocation cannot proceed (end of interceptor chain has been hit).

The cause is as follows.

While travelling up and down interceptor stacks (by calling InvocationContext.proceed() in an interceptor) the pointer into the interceptor stack managed by the InterceptorContext goes astray when a ChainedInterceptor or a WeavedInterceptor implementation of Interceptor is used somewhere in the stack of interceptors. This can easily been seen from the interaction between the InterceptorContext and e.g. a ChainedInterceptor (the same would happen with a WeavedInterceptor).

(all code snippets from jboss-invocation project at https://github.com/jbossas/jboss-invocation, used by jboss-as-7 and jboss-eap-6)

InterceptorContext.proceed():

    public Object proceed() throws Exception {
        final ListIterator<Interceptor> iterator = interceptorIterator;
        if (iterator.hasNext()) {
            Interceptor next = iterator.next();
            try {
                return next.processInvocation(this);
            } finally {
                if (iterator.hasPrevious()) iterator.previous();
            }
        } else {
            throw msg.cannotProceed();
        }
    }

where we see how the context's proceed() runs an interceptor, moving the iterator back one step upon return. If the interceptor is the chained interceptor, this is what happens

ChainedInterceptor.processInvocation()

    public Object processInvocation(final InterceptorContext context) throws Exception {
        final int oldNext = context.getNextInterceptorIndex();
        final List<Interceptor> old = context.getInterceptors();
        context.setInterceptors(interceptors);
        try {
            return context.proceed();
        } finally {
            context.setInterceptors(old, oldNext);
        }
    }

We see that the ChainedInterceptor is actually an interceptor that wraps a list of interceptors. It keeps track of the current state of the context (the old variables), injects its own interceptor list into the context and has the context handle it. Upon return from calling context's proceed, it tries to restores the state of the context, using the context's setInterceptors() with the old values it kept save:

InterceptorContext.setInterceptors():

    public void setInterceptors(final List<Interceptor> interceptors, int nextIndex) {
        if (interceptors == null) {
            throw new IllegalArgumentException("interceptors is null");
        }
        this.interceptors = interceptors;
        interceptorIterator = interceptors.listIterator(nextIndex);
    }

Here we see that the context's state is set with the given list of interceptors and that a new iterator instance is created and set. So, when the context returns from the processInvocation() of the ChainedInterceptor, it contains the original list of interceptors and a new Iterator object, pointing to the right place in the list. Next the context goes on setting the iterator and takes the old iterator instance and sets the iterator to a previous index. This changes the old iterator, which is not used any more, since nobody we know holds a reference to it. Furthermore, from now on the state of the context held by the context's interceptorIterator, pointing to the new iterator, is not what it's supposed to be, it points one step too far into the underlying list.

Now, back to re-invoking the context's proceed in an interceptor.

When an interceptor calls the context's proceed, while it's the one but last in the context's list of interceptors and the last one in the list is a chained or a weaved interceptor, then upon returning from proceed, the iterator in the context points too far (beyond the end) in the context's list. If we now re-invoke proceed on the context, the context's iterator finds there is no next element in the list and throws an exception (cannotProceed).

What probably should happen, is that proceed should try and set the new iterator to the previous index instead, like this:

InterceptorContext.proceed():

    public Object proceed() throws Exception {

        ListIterator<Interceptor> iterator = interceptorIterator
        if (iterator.hasNext()) {
            Interceptor next = iterator.next();
            try {
                return next.processInvocation(this);
            } finally {

                iterator = interceptorIterator;
                if (iterator.hasPrevious()) iterator.previous();
            }
        } else {
            throw msg.cannotProceed();
        }
    }

I've tested with this change, both as an jee interceptor that throws the application deadlock exception, relying on CMTTxIntercetor to re-invoke proceed(), or as a container interceptor that does the re-invoke itself. In both cases the proposed change seems to work all right.

I think I have to submit a Jira for this somewhere with JBoss.

Regards,

Eric van der Maarel

Is there any chance you would be interested in submitting a pull request with this fix?  Thanks for the in-depth analysis!

Actually already did: I've added an issue https://issues.jboss.org/browse/WFLY-2800 and a pull request https://github.com/jbossas/jboss-invocation/pull/6. (Just forgot to add a reference here.)