[hibernate-issues] [Hibernate-JIRA] Commented: (HHH-4518) Undeterministic behavior on Session.close without commit or rollback

Undeterministic behavior on Session.close without commit or rollback -------------------------------------------------------------------- Key: HHH-4518 URL: http://opensource.atlassian.com/projects/hibernate/browse/HHH-4518 Project: Hibernate Core Issue Type: Bug Components: core Affects Versions: 3.3.0.SP1 Environment: Hibernate 3.3.0.SP1, Oracle 10.2.0.1.0, c3p0 pool Reporter: Vladimir Nicolici Attachments: Test.java Closing a session without rollback or commit may cause the uncommitted changes to be committed. First scenario: Step 1. Obtain a session using SessionFactory.openSession, begin a transaction, makes some changes, don't commit, don't rollback, close the session. Step 2. Obtain a new session, begin a transaction, do some unrelated changes, commit the transaction, close the session. Result: the uncommitted changes performed in the first transaction are committed when the second transaction is committed. This shouldn't happen, what happens with the second session should not affect the changes performed with the first session. However, if instead of committing, you rollback the transaction from step 2, it also rollbacks the changes from step 1. Even worse, the second session has access to the uncommitted data generated by the first transaction, violating transaction isolation. Second scenario: 0. Configure a SessionFactory for an Oracle Database, with the c3p0 pool. 1. Obtain a session using SessionFactory.openSession, begin a transaction, makes some changes, don't commit, don't rollback, close the session. 2. Close the SessionFactory. Result: the uncommitted changes performed in the first transaction are committed when the SessionFactory is closed, because all the connections in the pool are closed, and oracle connections perform an implicit commit on close. If you use a different database, the changes will be rolled back. To conclude the behavior when a session doesn't commit or rollback depends on: - what the next Session does with the Connection; - what database you are using. What makes it worse is that the idiom recommended in the documentation for a non-managed environments (http://docs.jboss.org/hibernate/stable/core/reference/en/html/transaction...) is not safe: // Non-managed environment idiom Session sess = factory.openSession(); Transaction tx = null; try { tx = sess.beginTransaction(); // do some work ... tx.commit(); } catch (RuntimeException e) { if (tx != null) tx.rollback(); throw e; // or display error message } finally { sess.close(); } If "do some work" will throw java.lang.Error, it will not be caught by the catch clause for RuntimeException, the compiler will not complain that the Error is not caught, because a java.lang.Error behaves the same as RuntimeException, not requiring the programmer to catch it. This way, both the commit and the rollback code will not be executed, which may cause the first or second scenario to occur. What developers using hibernate may do until the issue is fixed: - replace "catch (RuntimeException e)" with "catch (Throwable t)" this will catch both java.lang.RuntimeException and java.lang.Error This may not be a good idea though, because according to the java.lang.Error JavaDoc (http://java.sun.com/javase/6/docs/api/java/lang/Error.html): "a reasonable application should not try to catch" it. - a better solution might be to move the rollback code to the "finally" block, like this: // Non-managed environment idiom Session sess = factory.openSession(); Transaction tx = null; try { tx = sess.beginTransaction(); // do some work ... tx.commit(); } finally { if (tx != null && !tx.wasCommitted()) tx.rollback(); sess.close(); } What I feel the developers of Hibernate should do: - update the documentation to document the issue - modify Session.close to perform rollback if the transaction has not been explicitly committed or rollback. If for some reason this can't be the default behavior, at least provide a configuration parameter to activate such behavior. This will result in much simpler and safer code, that will look like this: // Non-managed environment idiom Session sess = factory.openSession(); try { sess.beginTransaction(); // do some work ... sess.getTransaction().commit(); } finally { sess.close(); } Which reminds me that it would be also be nice if Session would have a commitTransaction method... I attached example code that reproduces the problem.