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

[Vladimir Nicolici (JIRA)]

    [ http://opensource.atlassian.com/projects/hibernate/browse/HHH-4518?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=34312#action_34312 ] 

Vladimir Nicolici commented on HHH-4518:
----------------------------------------

First, this problem doesn't have anything to do with Oracle, it is just a coincidence I was using Oracle when I discovered the issue.

Yes, the second scenario can be only reproduced on Oracle, but when I discovered the bug I was hitting the first scenario, as the JDBC connection was never closed, and the issue still occurred. I first suspected I was hitting the second scenario, but I didn't.

So even if you can't see how it could happen unless I use JTA, it happened, and I was using managed session context, not jta or thread.

But, to be absolutely sure Oracle is not the cause of the problem, today I tested on Postgres 8.4.1, and the same issue occurred. A thread was able to see uncommitted data left by another thread, and was able to inadvertently commit or rollback the transaction left open by the other thread.

I will attach the source files for the test, and an executable JAR including all the required libraries.

To test, you will need:
 - a Postgres SQL database running on localhost on the default port, 5432, with the password for the "postgres" user set to "change.me";
 - a "hibernatetest" table in the "public" schema of the default "postgres" database, having a single varchar(30) column named "testcolumn", containing a single record with the text 'initial state'.

You can run the test with java -jar hibernate-test.jar on the same machine running the database.

What you will see is three threads run in sequence, with 5 second delays between them:
- one thread updating the record, setting the text to 'uncommitted data';
- the second thread will be able to read 'uncommitted data';
- depending on Math.random the second thread will commit or rollback;
- the third thread will see 'initial state' if the second thread performed a rollback or 'uncommitted data' if it performed a commit;

As for your accusation that the resources were not cleaned correctly, I fully agree, BUT, I was cleaning them as recommended by the hibernate documentation at http://docs.jboss.org/hibernate/stable/core/reference/en/html/transactions.html#transactions-demarcation-nonmanaged , meaning the documentation examples show "bad programming practice".

The examples in the documentation (or as it calls them "the idioms") fail to handle the case when java.lang.Error and its children are raised during a transaction, leading to the bug.

Since you closed the bug, I can no longer upload files here, so to be able to show "evidence to the contrary" I uploaded the source code here:

http://www.megaupload.com/?d=5V40YE45

and the executable jar file here:

http://www.megaupload.com/?d=EMMI3X5H

If you need more evidence, I'll be glad to provide it.


> 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/transactions.html#transactions-demarcation-nonmanaged) 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.

-- 
This message is automatically generated by JIRA.
-
If you think it was sent incorrectly contact one of the administrators: http://opensource.atlassian.com/projects/hibernate/secure/Administrators.jspa
-
For more information on JIRA, see: http://www.atlassian.com/software/jira