hibernate(3)? OpenSessionInViewFilter session closing improvement [SPR-2847]

[spring-projects-issues]

Matt Benson opened SPR-2847 and commented

Current Hibernate wisdom says to close and recreate the Session on any HibernateException. When using Spring + Hibernate it is possible, having delved into Spring's inner workings, to gracefully recover from e.g. StaleObjectStateExceptions (or HibernateOptimisticLockingFailureExceptions, as it were) and install a new Session, binding into the TransactionManager. The OpenSessionInViewInterceptors are completely compatible with this approach, but the filter implementations fall down. This is because while the Interceptors implement the session management in separate methods, the Filters accomplish everything in a single method, storing the Session in a local variable. If the current Session has changed, the following happens:

Session.close() throws an Exception due to its already having been closed.
SessionFactoryUtils logs, but does not propagate, the Exception.
The active Session is not closed, thus its Connection is never released.

The simple fix is to use:

session = ((SessionHolder) TransactionSynchronizationManager.unbindResource(sessionFactory)).getSession();

instead of

TransactionSynchronizationManager.unbindResource(sessionFactory);

in the finally clause surrounding the doFilter() call.

Since my use case can be considered advanced Hibernate usage, I have classified this issue as an Improvement. However, since the Interceptors--whether by design or by luck--do not suffer from this shortcoming, I have left the priority at Major.

br,
Matt

---

Affects: 2.0 final

[spring-projects-issues]

Juergen Hoeller commented

This is not an intended use case, hence the filter implementation simply do not expect such a scenario. I am open to refining that...

Technically, closing and reopening the Session should not be necessary, though. Clearing the Session through a "session.clear()" call should be perfectly sufficient, which is what Spring automatically does in case of a transaction rollback on a pre-bound Session. For whatever reason, the Hibernate docs are a bit simplistic in their exception handling recommendations there, largely ignoring the "clear()" variant.

Note that this not available with Hibernate's native "sessionFactory.getCurrentSession()" either. To the best of my knowledge, all you can do there is call "clear()" on that single Session for the current transaction there...

Juergen

[spring-projects-issues]

Matt Benson commented

The current release of the Hibernate manual contains:

11.2.3. Exception handling

If the Session throws an exception (including any SQLException), you should immediately rollback the database transaction, call Session.close() and discard the Session instance. Certain methods of Session will not leave the session in a consistent state. No exception thrown by Hibernate can be treated as recoverable. Ensure that the Session will be closed by calling close() in a finally block.

http://forum.hibernate.org/viewtopic.php?t=967230 documents a member of the Hibernate Team reiterating this advice. I have also read such an admonition from Gavin King, though I can't find it ATM. The Gavin quote, as I recall, says that while it may currently be safe not to use a new Session, the Hibernate team makes no guarantees about the safety of continuing to use an errored Session in future versions of the library. The requested change won't impact current users, so from my perspective it seems harmless to allow Hibernate users--if they so choose--to use Spring out of the box with Hibernate while complying with Hibernate's prescribed development pattern.

br,
Matt

[spring-projects-issues]

Juergen Hoeller commented

OK, I've changed the filter code to do a fresh lookup for the SessionHolder in the close section, for both Hibernate 2.1 and Hibernate 3.x. It doesn't really hurt.

That said, I do not consider such replacement of the Session necessary either. I'm of course perfectly aware of what the Hibernate team has to say on this, but as I already wrote above, they do not take the "Session.clear()" operation into account in those simplistic recommendations. "clear()" is essentially a more efficient form of close-and-reopen, preserving the Session reference and its config state (including reuse of a JDBC Connection that it might hold on to, even if that JDBC Connection has been rolled back). There is no way that invalid state could remain in the Session's persistent object management after "clear()", with the semantics that are defined for that operation. This definitely applies at present, but of course those "clear()" semantics have to be preserved in future Hibernate versions as well, for backwards compatibility...

If there was an actual problem with Session cleanup, we'd close and reopen the Session ourselves there. However, we found that "clear()" works perfectly fine in theory and in practice, and is pretty efficient. FWIW, a similar "clear()" operation is also defined by JPA in the EntityManager interface, with no recommendation to throw away the EntityManager after an exception. So "clear()" is a perfectly fine way to reset an EntityManager after a transaction there as well, even in case of a Hibernate EntityManager.

Juergen

[spring-projects-issues]

Dennis Doubleday commented

Did this fix really get in to 2.0.2. I looked at the source that comes with the 2.0.2 distribution and the finally block still has the same code described in the original bug report.

[spring-projects-issues]

Dennis Doubleday commented

Sorry, ignore my comment--I was looking at 2.0, not 2.0.2.