11 Replies Latest reply on Jul 29, 2015 7:43 AM by Mark Little

    commit() throws exception: do we need to call rollback()?

    Radim Vansa Master

      I haven't found information from above clearly stated in JTA specification, could you please cite any authoritative source? The issue is if the code below is correct:

       

      tx.begin();
      try {
         /* do something */
         tx.commit();
      } catch (Exception e) {
         tx.rollback();
         /* handle failure during commit */
      }
      

      I believe that it's not; if the transaction has failed to commit, throwing an exception, it should be rolled back from the commit() call itself. User should not be responsible for calling rollback() - actually at this point the transaction should be disassociated. JTA 1.1 spec says in documentation for UserTransaction and TransactionManager (but not Transaction) commit()

      Complete the transaction associated with the current thread. When this method completes, the thread becomes associated with no transaction

      So after the commit completes (with exception), it's not associated and there's nothing to be rolled back.

      The spec also says (for both UserTransaction, TransactionManager and Transaction):

      RollbackException Thrown to indicate that the transaction has been rolled back rather than committed.

      That suggests that in at least some cases (and it seems to me that it should do in all cases when commit fails before committing some of the resources) the commit() actually performs complete rollback(), and then throws the exception.

       

      On the other hand, spec for Synchronization says that:

      An unchecked exception thrown by a registered Synchronization object causes the transaction to be aborted. That is, upon encountering an unchecked exception thrown by a registered synchronization object, the transaction manager must mark the transaction for rollback.

      It does not say that it immediatelly starts rollback(), just marks for the rollback. However, I think that this failure should not stop other synchronizations and the two-phase commit process to be invoked; it just says that the transaction cannot commit after this.

       

      So, in my opinion the code should look like

       

      tx.begin();
      try {
         /* do something */
      } finally {
         try {       
             if (tx.getStatus() == Status.ACTIVE) {
                 tx.commit();      
             } else {
                 tx.rollback();       
             }
         } catch (Exception e) {
             /* handle commit/rollback failure */
         }
      }
      

       

      Thanks for the clarification.

       

      Note: In the one below I am checking the status because if tx.markRollbackOnly() was called in /* do something */, we would then call a commit() doomed to fail. But that should work, too... So, theoretically just commit() should be enough.