CachingConnectionFactory should catch exceptions on logical close [SPR-12148]

[spring-projects-issues]

Gordon Daugherty opened SPR-12148 and commented

When an application uses a CachingConnectionFactory it will encounter the issue shown in this stack trace if the underlying connection is killed AND the client tries to use the JMS Session object:

Caused by: javax.jms.IllegalStateException: Session is closed
at com.tibco.tibjms.TibjmsxSessionImp.getTransacted(TibjmsxSessionImp.java:4837)
at org.springframework.jms.connection.CachingConnectionFactory$CachedSessionInvocationHandler.logicalClose(CachingConnectionFactory.java:398)
at org.springframework.jms.connection.CachingConnectionFactory$CachedSessionInvocationHandler.invoke(CachingConnectionFactory.java:298)
at $Proxy30.close(Unknown Source)
at com.mycorp.client.core.ServiceProxyImpl.handleDisconnect(ServiceProxyImpl.java:226)
at com.mycorp.client.core.ServiceProxyImpl.sendRequest(ServiceProxyImpl.java:180)
... 29 more

One of my teammates was able to reproduce this by killing the Connection at the broker (Tibco v8) after exercising the software so that the cache contained some sessions.

We're using "reconnectOnException=true" (the default for CachingConnectionFactory).

I don't see any code in the CachingConnectionFactory that would catch exceptions thrown by javax.jms.Session.getTransacted(). It seems that the client should be able to successfully return a bad proxied session to the pool by calling ProxiedSession.close() and the pool should not ever hand that session out again.

Maybe I don't properly understand the responsibilities assigned to the code that's using the CachingConnectionFactory but I don't see a good way to handle this scenario from the client code's perspective. You don't want multiple threads discovering that they're holding a bad session and each one calling resetConnection() since they'd step on each other.

The code that throws the exception shown in the stack trace above appears to exist in all spring-jms versions after v2.5.5.

The code in question was introduced as part of this enhancement: #9706

---

Affects: 3.2.10, 4.1 RC2

Issue Links:

• CachingConnectionFactory does not rollback on Session.close() [SPR-5031] #9706 CachingConnectionFactory does not rollback on Session.close()
• CachingConnectionFactory - Invalid session in session cache [SPR-16450] #20995 CachingConnectionFactory - Invalid session in session cache

Referenced from: commits 82f8b43, 4255774, 8543a55

Backported to: 4.0.7, 3.2.11

[spring-projects-issues]

Juergen Hoeller commented

Good point - however, do you see any actual misbehavior as a consequence? Session.close() may throw exceptions which the client code needs to deal with (catch and log usually)... and if our logicalClose() step fails at that point, we don't get to the step where we put the Session back into the pool.

I suppose it would be appropriate to catch exceptions from the logicalClose() step, proceeding to a physicalClose() in that case. It's just that this doesn't seem to make much difference in practice since the Session is dead at that point anyway and doesn't really have to be manually closed anymore...

Or are you concerned that such a scenario doesn't trigger a reconnect attempt? I would expect the broker to propagate such an exception through the ExceptionListener mechanism, even if happening during the logical close step, which would in turn trigger a reconnect attempt anyway.

Juergen

[spring-projects-issues]

Juergen Hoeller commented

We are catching logicalClose() exceptions now, proceeding to physicalClose() in such a case. This seems to be the best we can do.

Juergen

[spring-projects-issues]

Gordon Daugherty commented

That sounds like it will do the trick.

To be clear about the use-case we're covering here:

Software obtains a connection and multiple sessions from a CachingConnectionFactory, has a session in hand and is about to use it, the connection is killed on the broker end, gets an exception when trying to use the session or an object hung off the session (such as a JMS Producer), and decides to handle that by closing the session and obtaining a new one. The original code would throw an exception when closing the session. The new CachingConnectionFactory behavior should be to allow the $Proxy.close() to complete without an exception escaping and dispose of the cached Session object (attempt to close it AND remove it from the cache).

[spring-projects-issues]

Gordon Daugherty commented

My colleague re-ran our test scenario today using the updated spring-jms library and confirmed that this change meets our needs. Thank you!