[jbossts-issues] [JBoss JIRA] (JBTM-2846) Thread-safety problem in BasicAction

[David Lloyd (JIRA)]

    [ https://issues.jboss.org/browse/JBTM-2846?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=13368159#comment-13368159 ] 

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)