BTM
  1. BTM
  2. BTM-16

Transaction.commit() does not disassociate the tx from the thread

    Details

    • Type: Improvement Improvement
    • Status: Closed Closed
    • Priority: Major Major
    • Resolution: Fixed
    • Affects Version/s: 1.2
    • Fix Version/s: 1.3
    • Labels:
      None
    • Number of attachments :
      0

      Description

      Invoking commit() or rollback() on the jta Transaction object (i.e. BitronixTransaction)
      does not invoke BitronixTransactionManager.clearCurrentContext(), unlike invoking
      commit() or rollback() on the TM. This means that TM.getTransaction() returns non-null
      even after a commit or rollback.

        Activity

        Hide
        Ludovic Orban added a comment -

        This is according to the JTA spec: only the TransactionManager object has knowledge of thread context association.

        Show
        Ludovic Orban added a comment - This is according to the JTA spec: only the TransactionManager object has knowledge of thread context association.
        Hide
        Ronald Tschalär added a comment -

        I think maybe I didn't express myself clearly. The jta spec for
        TM.getTransaction() states

        The Transaction object that represents the transaction context of
        the calling thread. If the calling thread is not associated with a
        transaction, a null object reference is returned.

        And when a transaction completes it is disassociated from the thread.
        So TM.getTransaction() should return null irrespective of whether the
        transaction was completed by invoking commit (or rollback) on the
        Transaction or on the TM.

        Not disassociating a completed transaction from the thread leads to
        the problem that that thread cannot now start any new transactions:
        TM.begin() will throw NotSupportedException, and there's no way to
        disassociate other than by calling TM.commit() or TM.rollback(),
        neither of which make sense on a completed transaction (and will
        indeed fail).

        Show
        Ronald Tschalär added a comment - I think maybe I didn't express myself clearly. The jta spec for TM.getTransaction() states The Transaction object that represents the transaction context of the calling thread. If the calling thread is not associated with a transaction, a null object reference is returned. And when a transaction completes it is disassociated from the thread. So TM.getTransaction() should return null irrespective of whether the transaction was completed by invoking commit (or rollback) on the Transaction or on the TM. Not disassociating a completed transaction from the thread leads to the problem that that thread cannot now start any new transactions: TM.begin() will throw NotSupportedException, and there's no way to disassociate other than by calling TM.commit() or TM.rollback(), neither of which make sense on a completed transaction (and will indeed fail).
        Hide
        Ludovic Orban added a comment -

        I somewhat agree with you but that kind of silliness is due to yet another leak in the JTA spec.

        Geronimo (for instance) is behaving the same (see http://issues.apache.org/jira/browse/GERONIMO-594) and I suspect other TMs as well. This is no excuse unless doing something relevant breaks your compliance with the specification. We're discussing a corner case here and I don't feel like changing anything unless I'm sure there is no bad side effect which I cannot guarantee.

        On the other hand, there is no reason to call Transaction.commit() directly from within your code as this interface has not been intended to be used in that way even if the spec does not make that very clear.

        Once again I have to stay very defensive and tell you that unless you can prove me I'm wrong and provide the required evidences this will stay the way it is. If you want to discuss that further please use the forum as it is more convenient.

        Show
        Ludovic Orban added a comment - I somewhat agree with you but that kind of silliness is due to yet another leak in the JTA spec. Geronimo (for instance) is behaving the same (see http://issues.apache.org/jira/browse/GERONIMO-594 ) and I suspect other TMs as well. This is no excuse unless doing something relevant breaks your compliance with the specification. We're discussing a corner case here and I don't feel like changing anything unless I'm sure there is no bad side effect which I cannot guarantee. On the other hand, there is no reason to call Transaction.commit() directly from within your code as this interface has not been intended to be used in that way even if the spec does not make that very clear. Once again I have to stay very defensive and tell you that unless you can prove me I'm wrong and provide the required evidences this will stay the way it is. If you want to discuss that further please use the forum as it is more convenient.
        Hide
        Ronald Tschalär added a comment -

        I agree that the JTA spec leaves a lot to be desired, and I wish they'd made this case clearer. The reply in GERONIMO-594, however, is mistaken: section 3.3 in no way implies that the tx can't (or shouldn't) be disassociated from the thread - all it says is that you can't assume that it's the current thread you need to disassociate the tx from (i.e. you need to look up which thread to disassociate from).

        Regarding Transaction.commit() calling, well, my code is the equivalent of the application server - if I'm not the inteded user of it then who is? And besides, it doesn't change the fact that somebody is the intended user but that somebody effectively cannot use Transaction.commit() or Transaction.rollback(), ever.

        Ok, I'll check out the forum.

        Show
        Ronald Tschalär added a comment - I agree that the JTA spec leaves a lot to be desired, and I wish they'd made this case clearer. The reply in GERONIMO-594, however, is mistaken: section 3.3 in no way implies that the tx can't (or shouldn't) be disassociated from the thread - all it says is that you can't assume that it's the current thread you need to disassociate the tx from (i.e. you need to look up which thread to disassociate from). Regarding Transaction.commit() calling, well, my code is the equivalent of the application server - if I'm not the inteded user of it then who is? And besides, it doesn't change the fact that somebody is the intended user but that somebody effectively cannot use Transaction.commit() or Transaction.rollback(), ever. Ok, I'll check out the forum.
        Hide
        Ludovic Orban added a comment -

        This is not a bug but a nice feature to have. Will be implemented for 1.3.

        Show
        Ludovic Orban added a comment - This is not a bug but a nice feature to have. Will be implemented for 1.3.
        Hide
        Ludovic Orban added a comment -

        Implemented in trunk

        Show
        Ludovic Orban added a comment - Implemented in trunk
        Hide
        Nikita Tovstoles added a comment -

        This does not appear to be fixed in 1.3.2. BitronixTransactionManager.commit() does not call clearCurrentContext():

            public void commit() throws RollbackException, HeuristicMixedException, HeuristicRollbackException, SecurityException, IllegalStateException, SystemException {
                BitronixTransaction currentTx = getCurrentTransaction();
                if (log.isDebugEnabled()) log.debug("committing transaction " + currentTx);
                if (currentTx == null)
                    throw new IllegalStateException("no transaction started on this thread");
        
                currentTx.commit();
            }
        

        yet JTA spec section 3.2.2 clearly states that TM must dissociate tx from calling thread by the time commit() returns:

        The TransactionManager.commit method completes the transaction currently
        associated with the calling thread. After the commit method returns, the calling thread
        is not associated with a transaction.

        Show
        Nikita Tovstoles added a comment - This does not appear to be fixed in 1.3.2. BitronixTransactionManager.commit() does not call clearCurrentContext(): public void commit() throws RollbackException, HeuristicMixedException, HeuristicRollbackException, SecurityException, IllegalStateException, SystemException { BitronixTransaction currentTx = getCurrentTransaction(); if (log.isDebugEnabled()) log.debug("committing transaction " + currentTx); if (currentTx == null) throw new IllegalStateException("no transaction started on this thread"); currentTx.commit(); } yet JTA spec section 3.2.2 clearly states that TM must dissociate tx from calling thread by the time commit() returns: The TransactionManager.commit method completes the transaction currently associated with the calling thread. After the commit method returns, the calling thread is not associated with a transaction.
        Hide
        Ludovic Orban added a comment -

        What you say is nonsense. Did you actually read all the comments and test how BTM behaves ?

        This bug was about moving the TX disassociation code from BitronixTransactionManager to BitronixTransaction to have more flexibility while still sticking to the JTA standard.

        If you find that something isn't working as it should be welcome to discuss it in the mailing list.

        Show
        Ludovic Orban added a comment - What you say is nonsense. Did you actually read all the comments and test how BTM behaves ? This bug was about moving the TX disassociation code from BitronixTransactionManager to BitronixTransaction to have more flexibility while still sticking to the JTA standard. If you find that something isn't working as it should be welcome to discuss it in the mailing list.
        Hide
        Nikita Tovstoles added a comment -

        I just wrote a test and you are correct - context does get cleared. My apologies for the false alarm - though I would describe my above post as inaccurate - not nonsense. Would you kindly point me to "all the comments" - I'd like to understand whether BTM takes TX off the thread before or after firing Synchronization.afterCompletion(). Thanks

        Show
        Nikita Tovstoles added a comment - I just wrote a test and you are correct - context does get cleared. My apologies for the false alarm - though I would describe my above post as inaccurate - not nonsense. Would you kindly point me to "all the comments" - I'd like to understand whether BTM takes TX off the thread before or after firing Synchronization.afterCompletion(). Thanks
        Hide
        Ludovic Orban added a comment -

        Sorry for the wrong wording, 'inaccurate' sounds better than 'nonsense' indeed.

        I was referring to all the comments of this bug, especially the ones from Ronald which best describe this change.

        Show
        Ludovic Orban added a comment - Sorry for the wrong wording, 'inaccurate' sounds better than 'nonsense' indeed. I was referring to all the comments of this bug, especially the ones from Ronald which best describe this change.

          People

          • Assignee:
            Ludovic Orban
            Reporter:
            Ronald Tschalär
          • Votes:
            0 Vote for this issue
            Watchers:
            1 Start watching this issue

            Dates

            • Created:
              Updated:
              Resolved: