Spring JmsUtils.commitIfNecessary catches and ignores JMS IllegalStateException, losing message with ActiveMQ Artemis

JmsUtils.commitIfNecessary catch and ignore jakarta.jms.IllegalStateException saying it can only happens in case of JTA transaction:

    public static void commitIfNecessary(Session session) throws JMSException {
        Assert.notNull(session, "Session must not be null");
        try {
            session.commit();
        }
        catch (jakarta.jms.TransactionInProgressException | jakarta.jms.IllegalStateException ex) {
            // Ignore -> can only happen in case of a JTA transaction.
        }
    }

but there are cases where it can be raised also in local transaction, for example if a broker restart happens between send and commit:

13:16:57.143 [Thread-1] INFO org.springframework.jms.connection.CachingConnectionFactory -- Encountered a JMSException - resetting the underlying JMS Connection
jakarta.jms.JMSException: ActiveMQNotConnectedException[errorType=NOT_CONNECTED message=AMQ219006: Channel disconnected]
    at org.apache.activemq.artemis.jms.client.ActiveMQConnection$JMSFailureListener.connectionFailed(ActiveMQConnection.java:714)
    at org.apache.activemq.artemis.jms.client.ActiveMQConnection$JMSFailureListener.connectionFailed(ActiveMQConnection.java:735)
    at org.apache.activemq.artemis.core.client.impl.ClientSessionFactoryImpl.callSessionFailureListeners(ClientSessionFactoryImpl.java:868)
    at org.apache.activemq.artemis.core.client.impl.ClientSessionFactoryImpl.failoverOrReconnect(ClientSessionFactoryImpl.java:794)
    at org.apache.activemq.artemis.core.client.impl.ClientSessionFactoryImpl.handleConnectionFailure(ClientSessionFactoryImpl.java:566)
    at org.apache.activemq.artemis.core.client.impl.ClientSessionFactoryImpl.handleConnectionFailure(ClientSessionFactoryImpl.java:559)
    at org.apache.activemq.artemis.core.client.impl.ClientSessionFactoryImpl$1.run(ClientSessionFactoryImpl.java:417)
    at org.apache.activemq.artemis.utils.actors.OrderedExecutor.doTask(OrderedExecutor.java:57)
    at org.apache.activemq.artemis.utils.actors.OrderedExecutor.doTask(OrderedExecutor.java:32)
    at org.apache.activemq.artemis.utils.actors.ProcessorBase.executePendingTasks(ProcessorBase.java:68)
    at java.base/java.util.concurrent.ThreadPoolExecutor.runWorker(ThreadPoolExecutor.java:1136)
    at java.base/java.util.concurrent.ThreadPoolExecutor$Worker.run(ThreadPoolExecutor.java:635)
    at org.apache.activemq.artemis.utils.ActiveMQThreadFactory$1.run(ActiveMQThreadFactory.java:118)
Caused by: org.apache.activemq.artemis.api.core.ActiveMQNotConnectedException: AMQ219006: Channel disconnected
    at org.apache.activemq.artemis.core.client.impl.ClientSessionFactoryImpl.connectionDestroyed(ClientSessionFactoryImpl.java:410)
    at org.apache.activemq.artemis.core.remoting.impl.netty.NettyConnector$Listener$1.run(NettyConnector.java:1240)
    ... 6 common frames omitted

I created a couple of tests that reproduce the issue simulating the broker restart between send and commit:

https://github.com/luigidemasi/spring-jms-commit-issue

Comment From: bennyevangelista

Hi, I see this issue is closed but I'm having problem migrating from Spring 4.3.5 to 5.3.39 (I know community support fot these versions is ended bum I'm trying to investigate the problem). My application runs in IBM Websphere (JVM 8, so 5.3.39 is the highest version I can adopt) and uses jmstemplate to send message to an IBM MQ queue. Parameter "sessionTransacted" is set to "true" and with spring Framework 5.3.39 I'm getting the IllegalStateException with this stack:

Caused by: javax.jms.IllegalStateException: Method not permitted in global transaction
null
    at com.ibm.ejs.jms.JMSSessionHandle.checkNotInGlobalTransaction(JMSSessionHandle.java:1385)
    at com.ibm.ejs.jms.JMSSessionHandle.commit(JMSSessionHandle.java:700)
    at com.ibm.ejs.jms.JMSSessionHandle.commit(JMSSessionHandle.java:663)
    at org.springframework.jms.support.JmsUtils.commitIfNecessary(JmsUtils.java:218)
    at org.springframework.jms.core.JmsTemplate.doSend(JmsTemplate.java:1278)
    at org.springframework.jms.core.JmsTemplate.lambda$send$3(JmsTemplate.java:586)
    at org.springframework.jms.core.JmsTemplate.execute(JmsTemplate.java:504)
    at orgmsTemplate.send(JmsTemplate.java:584)
    at org.springframework.jms.core.JmsTemplate.convertAndSend(JmsTemplate.java:661)
    at it.[REDACTED].sendMessage[REDACTED].java:839)
    at it.[REDACTED].java:156)
    at it.[REDACTED].java:63)
    at it.[REDACTED].java:34)
    at sun.reflect.NativeMethodAccessorImpl.invoke0(NativeMethodAccessorImpl.java:-2)
    at sun.reflect.NativeMethodAccessorImpl.invoke(NativeMethodAccessorImpl.java:90)
    at sun.reflect.DelegatingMethodAccessorImpl.invoke(DelegatingMethodAccessorImpl.java:55)

In my scenario the transaction is manged by the application server container, and spring documentation says that in this case the "sessionTransacted" parameter should be ignored, but it's not what's happening. Debugging I found that the jmsUtils.isSessionLocallyTransacted method is returning true, even if the session is globally transacted, because of method isSessionTransactionalreturning true due to TransactionSynchronizationManager.getResource(cf) returning null. Debugging the TransactionSynchronizationManager.getResource(cf) method showed that TransactionSynchronizationManager.doGetResource is returning null when getting the map from resources:

TransactionSynchronizationManager.doGetResource
...
Map<Object, Object> map = resources.get();
if (map == null) {
    return null;
}
...

This leads to the thread not being aware of the global transaction, trying to commit and finally causing the exception.

Do you have any suggestion about this? This is the first time I dig deeply into Spring Framework so I'm surely missing something, but I'm am in the middle of the migration of hundreds of applications and this represents a major issue.

Comment From: jhoeller

@bennyevangelista this seems like an unfortunate overlap of transaction management responsibilites there. Does setting sessionTransacted to false help? That would definitely be more correct than leaving it at true which indicates local transaction management at the Session level. Common JMS conventions say that it would be ignored but that is not always the case indeed.

Comment From: bennyevangelista

@jhoeller Thank you for the suggestion. I agree that, in my case, this parameter should be set to false. However, we might also need to acknowledge that the Spring documentation is inaccurate, as it states that in global transaction scenarios the parameter is ignored, which, at least in my situation, is not true. Perhaps my case is so specific that it deserves further investigation?

Comment From: jhoeller

@bennyevangelista as far as I'm aware, we are just repeating the common Jakarta EE convention for sessionTransacted being ignored in a JTA transaction, not making strong assertions of our own about that flag being ignored - and most importantly, not recommending to set that flag for global transactions. We were just leniently tolerating it, suppressing exceptions on commit.

Unfortunately, the JMS API does not provide a clear indication for a global transaction at runtime, so we cannot make strong assumptions about it. Whereas a local sessionTransacted flag is a rather clear indication for local transaction management at the Session level, so we have to try an explicit commit there. Again, we were just leniently swallowing exceptions there; this is never ideal.

As for improving the documentation, are you specifically referring to the JmsAccessor#setSessionTransacted javadoc? Any other places where we should clarify the nuances there? I'll explicitly document leaving sessionTransacted at its default false in case of JTA transaction management, attempting local transaction management semantics when set to true (which is the actual behavior).