I didn't write the original code, but my understanding is that suspend() does not call disassociateThread() just to avoid acquiring a lock on the TransactionImpl instance. (TransactionImpl's disassociateCurrentThread() method locks the target transaction if that transaction is active.)
Currently the only consequence of not calling disassociateThread() from suspend() is that additional threads may get interrupted if the transaction timeouts. This is probably better than lock contention on the TransactionImpl instance.
Yes, it seems like an optimization but how you can really disassociate a thead from a tx, if you don't intend to commit the tx in that thread?
Assume thread A starts a tx, thread B resumes the same tx, does its work then suspends to indicate it is done with it, finally thread A commit at a later time.
If we want to use the associated threads as the criterion for the commiting thread to know it is safe to commit (i.e. all the other theads being disassociated), then we need to keep this internal association up-to-date.
Also, having the thread (B) possibly interrupted after it is presumably done with the transaction seems a little strange.
I think it should dissassociate the thread. Since we need to know which threads
are associated with the transaction.
However, make sure you don't break the requirements from the spec,
e.g. the OTS spec:
suspend If the client thread is not associated with a transaction, a null object reference is returned. Otherwise, an object is returned that represents the transaction context currently associated with the client thread. This object can be given to the resume operation to reestablish this context in the same thread or a different thread. The scope within which this object is valid is implementation dependent; at a minimum, it must be usable by the client thread. In addition, the client thread becomes associated with no transaction. This operation is not dependent on the state of the transaction; in particular, it does not raise the TRANSACTION_ROLLEDBACK exception.
thread association/disassociation was for TransactionTimeouts. I put this code in so that when transaction timeout happened, it would interrupt the thread. I think dissociate should happen in suspend. Otherwise you could interrupt a perfectly happy transaction/thread.
Thanks for the replies. I think the JTA spec is quite clear, too:
Suspend the transaction currently associated with the calling thread and return a Transaction object the represents the transaction context being suspended. If the calling thread is not associated with a transaction, the method returns a null object reference. When this method returns, the calling thread is associated with no transaction.