[JBoss JIRA] (JBTM-2846) Thread-safety problem in BasicAction
by David Lloyd (JIRA)
[ https://issues.jboss.org/browse/JBTM-2846?page=com.atlassian.jira.plugin.... ]
David Lloyd commented on JBTM-2846:
-----------------------------------
I observed one possibly sporadic test failure that could possibly have been explained by this or a different problem.
As for 20+ years, well, the memory model itself is 12 years old (before then the rules were quite different), and every Java release is a bit more aggressively efficient than the last in terms of exploiting the properties of the JMM for performance gains.
But, I think the fix could probably be as simple as throwing a "volatile" on there.
> Thread-safety problem in BasicAction
> ------------------------------------
>
> Key: JBTM-2846
> URL: https://issues.jboss.org/browse/JBTM-2846
> Project: JBoss Transaction Manager
> Issue Type: Bug
> Components: Transaction Core
> Affects Versions: 5.4.0.Final
> Reporter: David Lloyd
> Assignee: Michael Musgrove
> Priority: Critical
>
> It is possible for multiple threads to have a given transaction association at the same time ({{com.arjuna.ats.arjuna.coordinator.BasicAction#_childThreads}}). However, only inconsistent protection is provided for {{com.arjuna.ats.arjuna.coordinator.BasicAction#actionStatus}}. In particular, the {{com.arjuna.ats.internal.jta.transaction.arjunacore.TransactionImple#setRollbackOnly}} method directly calls {{com.arjuna.ats.arjuna.coordinator.BasicAction#preventCommit}}, which, without lock or atomicity, both reads and modifies {{actionStatus}}. This will result in a poorly-defined outcome when threads concurrently call {{setRollbackOnly()}} and, say, {{rollback()}}.
> Other methods access {{actionStatus}} without regard to concurrently protection, including:
> * {{com.arjuna.ats.arjuna.coordinator.BasicAction#save_state}}
> * {{com.arjuna.ats.arjuna.coordinator.BasicAction#toString}}
> * {{com.arjuna.ats.arjuna.coordinator.BasicAction#restore_state}}
> Some methods appear to be assuming that other methods will acquire the instance monitor before calling them; as a matter of best practice, such methods should have {{assert Thread.holdsLock(this);}} at their beginning point.
> Other fields in the class should also be analyzed for unsafe access.
--
This message was sent by Atlassian JIRA
(v7.2.3#72005)
7 years, 1 month
[JBoss JIRA] (JBTM-2846) Thread-safety problem in BasicAction
by Tom Jenkinson (JIRA)
[ https://issues.jboss.org/browse/JBTM-2846?page=com.atlassian.jira.plugin.... ]
Tom Jenkinson commented on JBTM-2846:
-------------------------------------
save_state - it seems susceptible to the same issue with preventCommit (assuming a very long delay between check and set)
restore_state - OK as its just called by recovery manager on new instances
toString - syncing for that operation seems wasteful
> Thread-safety problem in BasicAction
> ------------------------------------
>
> Key: JBTM-2846
> URL: https://issues.jboss.org/browse/JBTM-2846
> Project: JBoss Transaction Manager
> Issue Type: Bug
> Components: Transaction Core
> Affects Versions: 5.4.0.Final
> Reporter: David Lloyd
> Assignee: Michael Musgrove
> Priority: Critical
>
> It is possible for multiple threads to have a given transaction association at the same time ({{com.arjuna.ats.arjuna.coordinator.BasicAction#_childThreads}}). However, only inconsistent protection is provided for {{com.arjuna.ats.arjuna.coordinator.BasicAction#actionStatus}}. In particular, the {{com.arjuna.ats.internal.jta.transaction.arjunacore.TransactionImple#setRollbackOnly}} method directly calls {{com.arjuna.ats.arjuna.coordinator.BasicAction#preventCommit}}, which, without lock or atomicity, both reads and modifies {{actionStatus}}. This will result in a poorly-defined outcome when threads concurrently call {{setRollbackOnly()}} and, say, {{rollback()}}.
> Other methods access {{actionStatus}} without regard to concurrently protection, including:
> * {{com.arjuna.ats.arjuna.coordinator.BasicAction#save_state}}
> * {{com.arjuna.ats.arjuna.coordinator.BasicAction#toString}}
> * {{com.arjuna.ats.arjuna.coordinator.BasicAction#restore_state}}
> Some methods appear to be assuming that other methods will acquire the instance monitor before calling them; as a matter of best practice, such methods should have {{assert Thread.holdsLock(this);}} at their beginning point.
> Other fields in the class should also be analyzed for unsafe access.
--
This message was sent by Atlassian JIRA
(v7.2.3#72005)
7 years, 1 month
[JBoss JIRA] (JBTM-2854) XATerminatorImple.importTransaction can produce ArrayIndexOutOfBoundsException
by Tom Jenkinson (JIRA)
[ https://issues.jboss.org/browse/JBTM-2854?page=com.atlassian.jira.plugin.... ]
Tom Jenkinson commented on JBTM-2854:
-------------------------------------
There is a critical issue here but it's rather in the way WFTC is not attempting to pass in the full Xid.
The reason I downgraded the JBTM to Major is that you have to pass us more data in the Xid than the gtrid so even making a better error message it would be for something that should not happen.
You have to pass us the Xid of the parent SubordinateTransaction for importing of the propagated transaction so that when you call doRecover with the parent node name we can give you the right set of transactions back.
You could think of the interface for importing transactions as:
importTransaction(byte[] gtrid, String parent)
The parent name for importing is part of the Xid in the propagator node and can be determined before propagation using:
((com.arjuna.ats.jta.transaction.Transactio) transactionManagerService.getTransactionManager().getTransaction()).getTxId(); // The Javadoc is not clear, for subordinate transactions data is included in the Xid beyond the gtrid
Note that passing a dummy bqual with zerod data would not fully work because of the reason that we wouldn't be able to record the correct parent node name. This would mean in a recovery situation the parent would need to be sure to directly scan all known servers - reducing the opportunity to clean up partially completed transactions to when the root coordinator was online. That might well work but it wasn't designed that way and so lots of tests will fail (and the slower recovery I just mentioned). Recovery for EJB remoting was designed to go A -> B -> C rather than A -> C.
> XATerminatorImple.importTransaction can produce ArrayIndexOutOfBoundsException
> ------------------------------------------------------------------------------
>
> Key: JBTM-2854
> URL: https://issues.jboss.org/browse/JBTM-2854
> Project: JBoss Transaction Manager
> Issue Type: Bug
> Components: JTA
> Reporter: David Lloyd
> Assignee: Michael Musgrove
>
> Here is what the stack trace looks like:
> {noformat}
> Caused by: Remote exception java.lang.ArrayIndexOutOfBoundsException: 32
> at com.arjuna.ats.jta.xa.XATxConverter.getSubordinateNodeName(XATxConverter.java:204)
> at com.arjuna.ats.internal.jta.transaction.arjunacore.subordinate.jca.SubordinateAtomicAction.<init>(SubordinateAtomicAction.java:115)
> at com.arjuna.ats.internal.jta.transaction.arjunacore.subordinate.jca.TransactionImple.<init>(TransactionImple.java:57)
> at com.arjuna.ats.internal.jta.transaction.arjunacore.jca.TransactionImporterImple.addImportedTransaction(TransactionImporterImple.java:281)
> at com.arjuna.ats.internal.jta.transaction.arjunacore.jca.TransactionImporterImple.importRemoteTransaction(TransactionImporterImple.java:105)
> at com.arjuna.ats.internal.jta.transaction.arjunacore.jca.XATerminatorImple.importTransaction(XATerminatorImple.java:599)
> at org.wildfly.transaction.client.provider.jboss.JBossLocalTransactionProvider$XAImporterImpl.findOrImportTransaction(JBossLocalTransactionProvider.java:585)
> ... 11 more
> {noformat}
> It appears that there is no protection against an XID of inadequate length in XATxConverter#setSubordinateNodeName.
> At present the protocol is attempting to import an XID which contains only the gtid from the master node (based on what I discussed with [~tomjenkinson] some time ago at a previous meeting). This can be changed if it is wrong, but either way this method should fail with a friendlier exception if the XID is not valid for some reason.
--
This message was sent by Atlassian JIRA
(v7.2.3#72005)
7 years, 1 month
[JBoss JIRA] (JBTM-2854) XATerminatorImple.importTransaction can produce ArrayIndexOutOfBoundsException
by Tom Jenkinson (JIRA)
[ https://issues.jboss.org/browse/JBTM-2854?page=com.atlassian.jira.plugin.... ]
Tom Jenkinson updated JBTM-2854:
--------------------------------
Priority: Major (was: Critical)
> XATerminatorImple.importTransaction can produce ArrayIndexOutOfBoundsException
> ------------------------------------------------------------------------------
>
> Key: JBTM-2854
> URL: https://issues.jboss.org/browse/JBTM-2854
> Project: JBoss Transaction Manager
> Issue Type: Bug
> Components: JTA
> Reporter: David Lloyd
> Assignee: Michael Musgrove
>
> Here is what the stack trace looks like:
> {noformat}
> Caused by: Remote exception java.lang.ArrayIndexOutOfBoundsException: 32
> at com.arjuna.ats.jta.xa.XATxConverter.getSubordinateNodeName(XATxConverter.java:204)
> at com.arjuna.ats.internal.jta.transaction.arjunacore.subordinate.jca.SubordinateAtomicAction.<init>(SubordinateAtomicAction.java:115)
> at com.arjuna.ats.internal.jta.transaction.arjunacore.subordinate.jca.TransactionImple.<init>(TransactionImple.java:57)
> at com.arjuna.ats.internal.jta.transaction.arjunacore.jca.TransactionImporterImple.addImportedTransaction(TransactionImporterImple.java:281)
> at com.arjuna.ats.internal.jta.transaction.arjunacore.jca.TransactionImporterImple.importRemoteTransaction(TransactionImporterImple.java:105)
> at com.arjuna.ats.internal.jta.transaction.arjunacore.jca.XATerminatorImple.importTransaction(XATerminatorImple.java:599)
> at org.wildfly.transaction.client.provider.jboss.JBossLocalTransactionProvider$XAImporterImpl.findOrImportTransaction(JBossLocalTransactionProvider.java:585)
> ... 11 more
> {noformat}
> It appears that there is no protection against an XID of inadequate length in XATxConverter#setSubordinateNodeName.
> At present the protocol is attempting to import an XID which contains only the gtid from the master node (based on what I discussed with [~tomjenkinson] some time ago at a previous meeting). This can be changed if it is wrong, but either way this method should fail with a friendlier exception if the XID is not valid for some reason.
--
This message was sent by Atlassian JIRA
(v7.2.3#72005)
7 years, 1 month
[JBoss JIRA] (JBTM-2854) XATerminatorImple.importTransaction can produce ArrayIndexOutOfBoundsException
by Tom Jenkinson (JIRA)
[ https://issues.jboss.org/browse/JBTM-2854?page=com.atlassian.jira.plugin.... ]
Tom Jenkinson commented on JBTM-2854:
-------------------------------------
Hi [~dmlloyd] - we do need that bit of data as this is the way we can check during recovery (with shared object store) if the Xid belongs to the calling server and whether it was orphaned:
https://github.com/jbosstm/narayana/blob/master/ArjunaJTA/jta/classes/com...
https://github.com/jbosstm/narayana/blob/master/ArjunaJTA/jta/classes/com...
https://github.com/jbosstm/narayana/blob/master/ArjunaJTA/jta/classes/com...
It's encoded in the bqual:
https://github.com/jbosstm/narayana/blob/master/ArjunaJTA/jta/classes/com...
> XATerminatorImple.importTransaction can produce ArrayIndexOutOfBoundsException
> ------------------------------------------------------------------------------
>
> Key: JBTM-2854
> URL: https://issues.jboss.org/browse/JBTM-2854
> Project: JBoss Transaction Manager
> Issue Type: Bug
> Components: JTA
> Reporter: David Lloyd
> Assignee: Michael Musgrove
> Priority: Critical
>
> Here is what the stack trace looks like:
> {noformat}
> Caused by: Remote exception java.lang.ArrayIndexOutOfBoundsException: 32
> at com.arjuna.ats.jta.xa.XATxConverter.getSubordinateNodeName(XATxConverter.java:204)
> at com.arjuna.ats.internal.jta.transaction.arjunacore.subordinate.jca.SubordinateAtomicAction.<init>(SubordinateAtomicAction.java:115)
> at com.arjuna.ats.internal.jta.transaction.arjunacore.subordinate.jca.TransactionImple.<init>(TransactionImple.java:57)
> at com.arjuna.ats.internal.jta.transaction.arjunacore.jca.TransactionImporterImple.addImportedTransaction(TransactionImporterImple.java:281)
> at com.arjuna.ats.internal.jta.transaction.arjunacore.jca.TransactionImporterImple.importRemoteTransaction(TransactionImporterImple.java:105)
> at com.arjuna.ats.internal.jta.transaction.arjunacore.jca.XATerminatorImple.importTransaction(XATerminatorImple.java:599)
> at org.wildfly.transaction.client.provider.jboss.JBossLocalTransactionProvider$XAImporterImpl.findOrImportTransaction(JBossLocalTransactionProvider.java:585)
> ... 11 more
> {noformat}
> It appears that there is no protection against an XID of inadequate length in XATxConverter#setSubordinateNodeName.
> At present the protocol is attempting to import an XID which contains only the gtid from the master node (based on what I discussed with [~tomjenkinson] some time ago at a previous meeting). This can be changed if it is wrong, but either way this method should fail with a friendlier exception if the XID is not valid for some reason.
--
This message was sent by Atlassian JIRA
(v7.2.3#72005)
7 years, 1 month
[JBoss JIRA] (JBTM-2854) XATerminatorImple.importTransaction can produce ArrayIndexOutOfBoundsException
by Tom Jenkinson (JIRA)
[ https://issues.jboss.org/browse/JBTM-2854?page=com.atlassian.jira.plugin.... ]
Tom Jenkinson updated JBTM-2854:
--------------------------------
Priority: Critical (was: Blocker)
> XATerminatorImple.importTransaction can produce ArrayIndexOutOfBoundsException
> ------------------------------------------------------------------------------
>
> Key: JBTM-2854
> URL: https://issues.jboss.org/browse/JBTM-2854
> Project: JBoss Transaction Manager
> Issue Type: Bug
> Components: JTA
> Reporter: David Lloyd
> Assignee: Michael Musgrove
> Priority: Critical
>
> Here is what the stack trace looks like:
> {noformat}
> Caused by: Remote exception java.lang.ArrayIndexOutOfBoundsException: 32
> at com.arjuna.ats.jta.xa.XATxConverter.getSubordinateNodeName(XATxConverter.java:204)
> at com.arjuna.ats.internal.jta.transaction.arjunacore.subordinate.jca.SubordinateAtomicAction.<init>(SubordinateAtomicAction.java:115)
> at com.arjuna.ats.internal.jta.transaction.arjunacore.subordinate.jca.TransactionImple.<init>(TransactionImple.java:57)
> at com.arjuna.ats.internal.jta.transaction.arjunacore.jca.TransactionImporterImple.addImportedTransaction(TransactionImporterImple.java:281)
> at com.arjuna.ats.internal.jta.transaction.arjunacore.jca.TransactionImporterImple.importRemoteTransaction(TransactionImporterImple.java:105)
> at com.arjuna.ats.internal.jta.transaction.arjunacore.jca.XATerminatorImple.importTransaction(XATerminatorImple.java:599)
> at org.wildfly.transaction.client.provider.jboss.JBossLocalTransactionProvider$XAImporterImpl.findOrImportTransaction(JBossLocalTransactionProvider.java:585)
> ... 11 more
> {noformat}
> It appears that there is no protection against an XID of inadequate length in XATxConverter#setSubordinateNodeName.
> At present the protocol is attempting to import an XID which contains only the gtid from the master node (based on what I discussed with [~tomjenkinson] some time ago at a previous meeting). This can be changed if it is wrong, but either way this method should fail with a friendlier exception if the XID is not valid for some reason.
--
This message was sent by Atlassian JIRA
(v7.2.3#72005)
7 years, 1 month
[JBoss JIRA] (JBTM-2846) Thread-safety problem in BasicAction
by Michael Musgrove (JIRA)
[ https://issues.jboss.org/browse/JBTM-2846?page=com.atlassian.jira.plugin.... ]
Michael Musgrove commented on JBTM-2846:
----------------------------------------
[~dmlloyd] Do you have a test case that shows up the issue - I ask because this is code that has been running for 20+ years without issues in this area?
> Thread-safety problem in BasicAction
> ------------------------------------
>
> Key: JBTM-2846
> URL: https://issues.jboss.org/browse/JBTM-2846
> Project: JBoss Transaction Manager
> Issue Type: Bug
> Components: Transaction Core
> Affects Versions: 5.4.0.Final
> Reporter: David Lloyd
> Assignee: Michael Musgrove
> Priority: Critical
>
> It is possible for multiple threads to have a given transaction association at the same time ({{com.arjuna.ats.arjuna.coordinator.BasicAction#_childThreads}}). However, only inconsistent protection is provided for {{com.arjuna.ats.arjuna.coordinator.BasicAction#actionStatus}}. In particular, the {{com.arjuna.ats.internal.jta.transaction.arjunacore.TransactionImple#setRollbackOnly}} method directly calls {{com.arjuna.ats.arjuna.coordinator.BasicAction#preventCommit}}, which, without lock or atomicity, both reads and modifies {{actionStatus}}. This will result in a poorly-defined outcome when threads concurrently call {{setRollbackOnly()}} and, say, {{rollback()}}.
> Other methods access {{actionStatus}} without regard to concurrently protection, including:
> * {{com.arjuna.ats.arjuna.coordinator.BasicAction#save_state}}
> * {{com.arjuna.ats.arjuna.coordinator.BasicAction#toString}}
> * {{com.arjuna.ats.arjuna.coordinator.BasicAction#restore_state}}
> Some methods appear to be assuming that other methods will acquire the instance monitor before calling them; as a matter of best practice, such methods should have {{assert Thread.holdsLock(this);}} at their beginning point.
> Other fields in the class should also be analyzed for unsafe access.
--
This message was sent by Atlassian JIRA
(v7.2.3#72005)
7 years, 1 month
[JBoss JIRA] (JBTM-2846) Thread-safety problem in BasicAction
by Tom Jenkinson (JIRA)
[ https://issues.jboss.org/browse/JBTM-2846?page=com.atlassian.jira.plugin.... ]
Tom Jenkinson reassigned JBTM-2846:
-----------------------------------
Assignee: Michael Musgrove
> Thread-safety problem in BasicAction
> ------------------------------------
>
> Key: JBTM-2846
> URL: https://issues.jboss.org/browse/JBTM-2846
> Project: JBoss Transaction Manager
> Issue Type: Bug
> Components: Transaction Core
> Affects Versions: 5.4.0.Final
> Reporter: David Lloyd
> Assignee: Michael Musgrove
> Priority: Critical
>
> It is possible for multiple threads to have a given transaction association at the same time ({{com.arjuna.ats.arjuna.coordinator.BasicAction#_childThreads}}). However, only inconsistent protection is provided for {{com.arjuna.ats.arjuna.coordinator.BasicAction#actionStatus}}. In particular, the {{com.arjuna.ats.internal.jta.transaction.arjunacore.TransactionImple#setRollbackOnly}} method directly calls {{com.arjuna.ats.arjuna.coordinator.BasicAction#preventCommit}}, which, without lock or atomicity, both reads and modifies {{actionStatus}}. This will result in a poorly-defined outcome when threads concurrently call {{setRollbackOnly()}} and, say, {{rollback()}}.
> Other methods access {{actionStatus}} without regard to concurrently protection, including:
> * {{com.arjuna.ats.arjuna.coordinator.BasicAction#save_state}}
> * {{com.arjuna.ats.arjuna.coordinator.BasicAction#toString}}
> * {{com.arjuna.ats.arjuna.coordinator.BasicAction#restore_state}}
> Some methods appear to be assuming that other methods will acquire the instance monitor before calling them; as a matter of best practice, such methods should have {{assert Thread.holdsLock(this);}} at their beginning point.
> Other fields in the class should also be analyzed for unsafe access.
--
This message was sent by Atlassian JIRA
(v7.2.3#72005)
7 years, 1 month
[JBoss JIRA] (JBTM-2854) XATerminatorImple.importTransaction can produce ArrayIndexOutOfBoundsException
by Tom Jenkinson (JIRA)
[ https://issues.jboss.org/browse/JBTM-2854?page=com.atlassian.jira.plugin.... ]
Tom Jenkinson reassigned JBTM-2854:
-----------------------------------
Assignee: Michael Musgrove
> XATerminatorImple.importTransaction can produce ArrayIndexOutOfBoundsException
> ------------------------------------------------------------------------------
>
> Key: JBTM-2854
> URL: https://issues.jboss.org/browse/JBTM-2854
> Project: JBoss Transaction Manager
> Issue Type: Bug
> Components: JTA
> Reporter: David Lloyd
> Assignee: Michael Musgrove
> Priority: Blocker
>
> Here is what the stack trace looks like:
> {noformat}
> Caused by: Remote exception java.lang.ArrayIndexOutOfBoundsException: 32
> at com.arjuna.ats.jta.xa.XATxConverter.getSubordinateNodeName(XATxConverter.java:204)
> at com.arjuna.ats.internal.jta.transaction.arjunacore.subordinate.jca.SubordinateAtomicAction.<init>(SubordinateAtomicAction.java:115)
> at com.arjuna.ats.internal.jta.transaction.arjunacore.subordinate.jca.TransactionImple.<init>(TransactionImple.java:57)
> at com.arjuna.ats.internal.jta.transaction.arjunacore.jca.TransactionImporterImple.addImportedTransaction(TransactionImporterImple.java:281)
> at com.arjuna.ats.internal.jta.transaction.arjunacore.jca.TransactionImporterImple.importRemoteTransaction(TransactionImporterImple.java:105)
> at com.arjuna.ats.internal.jta.transaction.arjunacore.jca.XATerminatorImple.importTransaction(XATerminatorImple.java:599)
> at org.wildfly.transaction.client.provider.jboss.JBossLocalTransactionProvider$XAImporterImpl.findOrImportTransaction(JBossLocalTransactionProvider.java:585)
> ... 11 more
> {noformat}
> It appears that there is no protection against an XID of inadequate length in XATxConverter#setSubordinateNodeName.
> At present the protocol is attempting to import an XID which contains only the gtid from the master node (based on what I discussed with [~tomjenkinson] some time ago at a previous meeting). This can be changed if it is wrong, but either way this method should fail with a friendlier exception if the XID is not valid for some reason.
--
This message was sent by Atlassian JIRA
(v7.2.3#72005)
7 years, 1 month