[
https://issues.jboss.org/browse/JBTM-2846?page=com.atlassian.jira.plugin....
]
David Lloyd commented on JBTM-2846:
-----------------------------------
OK I have good news and bad news. Bad news first: there's a real (non-theoretical)
thread safety problem that may crop up under load which is not solved solely by setting
{{volatile}}. It happens under the described circumstances, where setRollbackOnly() can
return successfully yet prepare() or commit(true) also succeeds.
The good news is I worked up a quick patch for it:
https://github.com/jbosstm/narayana/compare/master...dmlloyd:jbtm-2846
If it looks OK I can put it in as a PR. I tried to explain the logic in the comments so
hopefully it's clear why it's needed and why it works. But I can't test it
locally through the Narayana test suite at the moment due to other things I have going on,
so it's up to you guys what you want to do.
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)