3 Replies Latest reply on Jan 23, 2014 6:04 PM by Eric van der Maarel

    Container interceptors and ApplicationDeadlockException handling.

    Michal Wozniak Newbie

      Hi,

       

        I'm migrating an application which was previously working on JBoss 4 to JBoss 7. We had an interceptor which was checking out exceptions returned by database driver and converting them to ApplicationDeadlockException when certain sqlstates were occuring. I changed it to container-level interceptor and the code looks like this:

       

      public class DeadlockInterceptor {

       

          @AroundInvoke

          public Object invoke(final InvocationContext mi) throws Exception {

              try {

                  return mi.proceed();

              }

              catch (Exception e) {

                  handleDeadlockException(e);

                  throw e;

              }

          }

       

          private void handleDeadlockException(Exception e) throws ApplicationDeadlockException {

       

              for (Throwable ex = e; ex != null; ex = ex.getCause()) {

                  if (ex instanceof ApplicationDeadlockException) {

                      // no need to keep looking if we already have a ApplicationDeadlockException

                      throw (ApplicationDeadlockException)ex;

                  }

                  if (ex instanceof SQLException) {

                      String sqlState = DeadlockUtil.extractSqlState((SQLException) ex);

                      if ("40001".equals(sqlState) || "61000".equals(sqlState/*oracle*/)) {

                          ApplicationDeadlockException deadlock = new ApplicationDeadlockException("database deadlock", true);

                          deadlock.initCause(ex);

                          Logger.warning(this, "Detected a DB serialization failure. This could be a possible SQL Deadlock. Now retry the related transaction up to 5 times.");

                          throw deadlock;

                      }

                  }

              }

          }

      }

       

      This was causing JBoss to retry transaction commit up to 5 times. This functionality is handled in org.jboss.as.ejb3.tx.CMTTxInterceptor (6.1.0.Alpha). Checking the program flow it seems container tries to retry the transaction but it is running into an exception:

       

      javax.ejb.EJBException: org.jboss.invocation.CannotProceedException: INV-00002:Invocation cannot proceed (end of interceptor chain has been hit)

      at org.jboss.as.ejb3.tx.CMTTxInterceptor.handleExceptionInOurTx(CMTTxInterceptor.java:165)

      at org.jboss.as.ejb3.tx.CMTTxInterceptor.invokeInOurTx(CMTTxInterceptor.java:250)

      at org.jboss.as.ejb3.tx.CMTTxInterceptor.required(CMTTxInterceptor.java:315)

      at org.jboss.as.ejb3.tx.CMTTxInterceptor.processInvocation(CMTTxInterceptor.java:214)

      at org.jboss.invocation.InterceptorContext.proceed(InterceptorContext.java:289)

      at org.jboss.as.ejb3.component.interceptors.CurrentInvocationContextInterceptor.processInvocation(CurrentInvocationContextInterceptor.java:41)

      at org.jboss.invocation.InterceptorContext.proceed(InterceptorContext.java:289)

      at org.jboss.invocation.InterceptorContext$Invocation.proceed(InterceptorContext.java:376)

      [my interceptor code here]

       

      So my question is is this supposed to work? Retry code is still there but maybe it is just some unmaintained legacy thing? Am i doing something wrong using ApplicationDeadlockException this way? Or maybe there is a different way to approach this problem? Thanks in advance,

       

      Michal Wozniak

        • 1. Re: Container interceptors and ApplicationDeadlockException handling.
          Eric van der Maarel Newbie

          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

          • 2. Re: Container interceptors and ApplicationDeadlockException handling.
            David Lloyd Master

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

            • 3. Re: Container interceptors and ApplicationDeadlockException handling.
              Eric van der Maarel Newbie

              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.)