This is cloned from JBAS-7922. A test need to be created in AS7 to ensure a regression does not re-appear in the new codebase - dimitris

Testing scenario:
Deployed JBoss 5.1.0.GA (with tweaks to the pom during build, see Environment) with a JCA adapter for an external JMS broker, used both ActiveMQ 5.3.0 and HornetQ 2.0.0, both clustered and standalone. Also tried an embedded HornetQ broker with the same results. Deployed an ear that has a simple MDB requiring container managed transactions listening to a queue with max sessions set to 2 and a simple jsp to publish an arbitrary number of text messages to that queue. The MDB simply prints the text information provided by the message, but also has a hard-coded 1 second pause for every even numbered message.

When I shut down JBoss while messages are still being consumed, the BlockContainerShutdownInterceptor starts throwing DispatcherConnectExceptions, as the EJB container has been stopped or is in the process of being stopped. However, it doesn't mark the transaction for rollback itself, and MessageInflowLocalProxy.delivery() only marks the transaction for rollback if the invocation chain throws an Error or RuntimeException. Thus, in this case, JBoss failed to deliver the message to the MDB, but committed the transaction. According to the spec, for an MDB with a container managed transaction, the message should be ACK'd to the broker if the transaction successfully commits, and both the ActiveMQ and HornetQ JCA implementations dutifully do this, resulting in all messages that were attempted to be delivered after the EJB container shutdown being lost.

When the BlockContainerShutdownInterceptor detects that the EJB container has been shutdown, it needs to take an action that will ensure the transaction is marked for rollback. After parsing through the code involved in the interceptor chain, I can see a few potential solutions:

• Have BlockContainerShutdownInterceptor.invoke() call TxUtil.getTransactionManager().setRollbackOnly() before it would throw a DispatcherConnectException. Probably need to consume any exception thrown from setRollbackOnly() and log it first I suppose, although it's always a bit odd when you get an exception thrown when you intend to throw an exception anyway for a different reason.
• Have BlockContainerShutdownInterceptor.invoke() put some state on the Invocation to indicate that delivery is being blocked. MessagingContainer.localInvoke() could then take some action based on this state to ensure the transaction is rolled back, either rolling it back itself or notifying its caller that the transaction should be rolled back. A bit odd here if BlockContainerShutdownInterceptor.invoke() continues to throw an exception, as there would be data about the exception being passed back outside the exception though.
• Have MessageInflowLocalProxy handle DispatcherConnectExceptions. It might be better to introduce an additional class, say InvocationInterruptedException that DispatcherConnectException could extend, to allow for the possibility that invocations may be interrupted for other reasons, allowing MessageInflowLocalProxy to not care as much about the particular reason invocation was halted, but still be able to roll back the transaction whenever it is.

I slightly favor the third approach based on what I've seen of that portion of the code so far, so I will probably work to put together a patch file along those lines. This is the first time I've delved deeply into the JBoss source code though. If I can find some examples of test cases in this part of the code base, I'll attempt to put together a verifying test case as well.

I marked the priority as Critical, based on the guidelines, as this is resulting in data loss and as far as I best understand the spec is a violation of the spec.