[JBoss JIRA] (JBTM-2848) Transaction .equals() methods do not comply with specification
by Anonymous (JIRA)
[ https://issues.jboss.org/browse/JBTM-2848?page=com.atlassian.jira.plugin.... ]
Issue was automatically transitioned when David M. Lloyd created pull request #1127 in GitHub
---------------------------------------------------------------------------------------------
Status: Pull Request Sent (was: Open)
> Transaction .equals() methods do not comply with specification
> --------------------------------------------------------------
>
> Key: JBTM-2848
> URL: https://issues.jboss.org/browse/JBTM-2848
> Project: JBoss Transaction Manager
> Issue Type: Bug
> Components: JTA
> Reporter: David Lloyd
> Priority: Blocker
>
> The JTA specification has this to say about Transaction.equals():
> {quote}
> The transaction manager must implement the Transaction object's {{equals}} method to allow comparison between the target object and another Transaction object. The {{equals}} method should return {{true}} if the target object and the parameter object both refer to the same global transaction.
> For example, the application server may need to compare two Transaction objects when trying to reuse a resource that is already enlisted with a transaction. This can be done using the {{equals}} method.
> {code}
> Transaction txObj = TransactionManager.getTransaction();
> Transaction someOtherTxObj = ..
> ..
> boolean isSame = txObj.equals(someOtherTxObj);
> {code}
> In addition, the transaction manager must implement the Transaction object's {{hashCode}} method so that if two Transaction objects are equal, they have the same hash code. However, the converse is not necessarily true. Two Transaction objects with the same hash code are not necessarily equal.
> {quote}
> There are several transaction implementation classes in Narayana including:
> * {{com.arjuna.ats.internal.jta.transaction.arjunacore.subordinate.jca.TransactionImple}}
> * {{com.arjuna.ats.internal.jta.transaction.arjunacore.subordinate.TransactionImple}}
> * {{com.arjuna.ats.internal.jta.transaction.arjunacore.TransactionImple}}
> Sometimes it comes to pass that, for whatever reason, importing a transaction might return a transaction instance of a different type than what was previously returned. In this case the flaw in the {{equals}} method is clear: it compares the types for effective equality before it compares the UID, causing two transactions of different types which refer to the same global transaction to be non-equal, which causes integrity checks in the remote JTA code to fail.
> I'll provide a PR that fixes the issue which you can use if you want.
--
This message was sent by Atlassian JIRA
(v7.2.3#72005)
7 years, 2 months
[JBoss JIRA] (JBTM-2848) Transaction .equals() methods do not comply with specification
by David Lloyd (JIRA)
David Lloyd created JBTM-2848:
---------------------------------
Summary: Transaction .equals() methods do not comply with specification
Key: JBTM-2848
URL: https://issues.jboss.org/browse/JBTM-2848
Project: JBoss Transaction Manager
Issue Type: Bug
Components: JTA
Reporter: David Lloyd
Priority: Blocker
The JTA specification has this to say about Transaction.equals():
{quote}
The transaction manager must implement the Transaction object's {{equals}} method to allow comparison between the target object and another Transaction object. The {{equals}} method should return {{true}} if the target object and the parameter object both refer to the same global transaction.
For example, the application server may need to compare two Transaction objects when trying to reuse a resource that is already enlisted with a transaction. This can be done using the {{equals}} method.
{code}
Transaction txObj = TransactionManager.getTransaction();
Transaction someOtherTxObj = ..
..
boolean isSame = txObj.equals(someOtherTxObj);
{code}
In addition, the transaction manager must implement the Transaction object's {{hashCode}} method so that if two Transaction objects are equal, they have the same hash code. However, the converse is not necessarily true. Two Transaction objects with the same hash code are not necessarily equal.
{quote}
There are several transaction implementation classes in Narayana including:
* {{com.arjuna.ats.internal.jta.transaction.arjunacore.subordinate.jca.TransactionImple}}
* {{com.arjuna.ats.internal.jta.transaction.arjunacore.subordinate.TransactionImple}}
* {{com.arjuna.ats.internal.jta.transaction.arjunacore.TransactionImple}}
Sometimes it comes to pass that, for whatever reason, importing a transaction might return a transaction instance of a different type than what was previously returned. In this case the flaw in the {{equals}} method is clear: it compares the types for effective equality before it compares the UID, causing two transactions of different types which refer to the same global transaction to be non-equal, which causes integrity checks in the remote JTA code to fail.
I'll provide a PR that fixes the issue which you can use if you want.
--
This message was sent by Atlassian JIRA
(v7.2.3#72005)
7 years, 2 months
[JBoss JIRA] (JBTM-2847) Static code analysis - check report and fix issues
by Ondra Chaloupka (JIRA)
[ https://issues.jboss.org/browse/JBTM-2847?page=com.atlassian.jira.plugin.... ]
Ondra Chaloupka updated JBTM-2847:
----------------------------------
Status: Resolved (was: Pull Request Sent)
Fix Version/s: 5.next
Resolution: Done
The mentioned issues taken from static code analysis report were addressed.
> Static code analysis - check report and fix issues
> --------------------------------------------------
>
> Key: JBTM-2847
> URL: https://issues.jboss.org/browse/JBTM-2847
> Project: JBoss Transaction Manager
> Issue Type: Task
> Affects Versions: 5.5.1.Final
> Reporter: Ondra Chaloupka
> Assignee: Ondra Chaloupka
> Priority: Minor
> Fix For: 5.next
>
>
> By going through static code analysis report of changes done during last a half year there are few issues that would be fine to be adjusted.
> They are following
> # at com.arjuna.ats.internal.jta.transaction.arjunacore.jca.XATerminatorImple,
> L614; com.arjuna.ats.internal.jta.transaction.jts.jca, L471
> (https://github.com/jbosstm/narayana/blob/master/ArjunaJTA/jta/classes/com...) the calling method getCurrentTransactionId() potentially could end with
> NullPointerException (caused by TransactionImple#getTransaction when `final BasicAction current = BasicAction.Current()` is null)
> # possible resource leak - connection was ommitted to be closed
> ## org.jboss.narayana.tomcat.jta.integration.TestExecutor
> ## TestCommitMarkableResourceFailAfterCommitOrphan
> # inner class SampleLockable of org.jboss.stm.STMVerticle could be static (http://station5.brq.redhat.com:8080/docs/en/findbugs/fb_checker_ref.html#...)
--
This message was sent by Atlassian JIRA
(v7.2.3#72005)
7 years, 2 months
[JBoss JIRA] (JBTM-989) Consider using a common code style throughout Narayana
by Ondra Chaloupka (JIRA)
[ https://issues.jboss.org/browse/JBTM-989?page=com.atlassian.jira.plugin.s... ]
Ondra Chaloupka commented on JBTM-989:
--------------------------------------
I do understand, I will get used to the current style of work just my humble opinion
* no coding style could be painful for newcomers who wants to contribute (like me)
* it's harder to read the code as each class is formatted differently
* it's harder to maintain code as you need to switch from style to style when switching classes
> Consider using a common code style throughout Narayana
> ------------------------------------------------------
>
> Key: JBTM-989
> URL: https://issues.jboss.org/browse/JBTM-989
> Project: JBoss Transaction Manager
> Issue Type: Task
> Affects Versions: 4.17.0.M1/5.0.0.M1
> Reporter: Paul Robinson
> Assignee: Tom Jenkinson
>
> I think we should consider using a common code style throughout the TS project. The benefits of doing this are as follows:
> # You can automate code formatting. This is a real productivity boost as you can type away, thinking about your code, rather than the style. When you hit save, or trigger it directly, the code is formatted.
> ## This is not possible without a project wide code style as you too frequently re-format code that needs to stay in someone else's personal style. You can't commit this changed code as; a) it may annoy the "owner" and b) it results in a change that can't be diffed (every line may be changed).
> # We get consistency over the whole project, making it easier to read code written by others.
> # We have to do this anyway for code we maintain inside the JBossAS project as the project refuses to build if it doesn't adhere to their style.
> Personally, I like the style I've used for the last 10 years. I find it harder to read code that is not in this style. Hence I can understand why people may object to changing their style. However, this is the very reason why a common style is beneficial. You can get used to a new style and once you do, the entire project will be styled in the way that you are used to. Providing the style is sensible, I would much rather use a style consistent across the projects I work on. I'm happy for that style to be different to my current style.
> As I stated above JBossAS mandates a style at build time. I don't particularly like the style (braces should occupy their own line, IMO) but I'm happy to go with this one if it becomes the Narayana standard style.
> I think we should also break the build for violations. This should prevent mistakes making their way into SVN.
--
This message was sent by Atlassian JIRA
(v7.2.3#72005)
7 years, 2 months
[JBoss JIRA] (JBTM-989) Consider using a common code style throughout Narayana
by Michael Musgrove (JIRA)
[ https://issues.jboss.org/browse/JBTM-989?page=com.atlassian.jira.plugin.s... ]
Michael Musgrove commented on JBTM-989:
---------------------------------------
There was a separate discussion via email amongst the members of the team. The consensus was to maintain the status quo, namely:
# We recommend the use of the AS7 style, but don't mandate it;
# Follow your preferred style (within reason) when working on your own code and respect another's style when working on theirs;
# Code style should be consistent throughout the file.
> Consider using a common code style throughout Narayana
> ------------------------------------------------------
>
> Key: JBTM-989
> URL: https://issues.jboss.org/browse/JBTM-989
> Project: JBoss Transaction Manager
> Issue Type: Task
> Affects Versions: 4.17.0.M1/5.0.0.M1
> Reporter: Paul Robinson
> Assignee: Tom Jenkinson
>
> I think we should consider using a common code style throughout the TS project. The benefits of doing this are as follows:
> # You can automate code formatting. This is a real productivity boost as you can type away, thinking about your code, rather than the style. When you hit save, or trigger it directly, the code is formatted.
> ## This is not possible without a project wide code style as you too frequently re-format code that needs to stay in someone else's personal style. You can't commit this changed code as; a) it may annoy the "owner" and b) it results in a change that can't be diffed (every line may be changed).
> # We get consistency over the whole project, making it easier to read code written by others.
> # We have to do this anyway for code we maintain inside the JBossAS project as the project refuses to build if it doesn't adhere to their style.
> Personally, I like the style I've used for the last 10 years. I find it harder to read code that is not in this style. Hence I can understand why people may object to changing their style. However, this is the very reason why a common style is beneficial. You can get used to a new style and once you do, the entire project will be styled in the way that you are used to. Providing the style is sensible, I would much rather use a style consistent across the projects I work on. I'm happy for that style to be different to my current style.
> As I stated above JBossAS mandates a style at build time. I don't particularly like the style (braces should occupy their own line, IMO) but I'm happy to go with this one if it becomes the Narayana standard style.
> I think we should also break the build for violations. This should prevent mistakes making their way into SVN.
--
This message was sent by Atlassian JIRA
(v7.2.3#72005)
7 years, 2 months
[JBoss JIRA] (JBTM-2847) Static code analysis - check report and fix issues
by Ondra Chaloupka (JIRA)
[ https://issues.jboss.org/browse/JBTM-2847?page=com.atlassian.jira.plugin.... ]
Issue was automatically transitioned when Ondra Chaloupka created pull request #1125 in GitHub
----------------------------------------------------------------------------------------------
Status: Pull Request Sent (was: Open)
> Static code analysis - check report and fix issues
> --------------------------------------------------
>
> Key: JBTM-2847
> URL: https://issues.jboss.org/browse/JBTM-2847
> Project: JBoss Transaction Manager
> Issue Type: Task
> Affects Versions: 5.5.1.Final
> Reporter: Ondra Chaloupka
> Assignee: Ondra Chaloupka
> Priority: Minor
>
> By going through static code analysis report of changes done during last a half year there are few issues that would be fine to be adjusted.
> They are following
> # at com.arjuna.ats.internal.jta.transaction.arjunacore.jca.XATerminatorImple,
> L614; com.arjuna.ats.internal.jta.transaction.jts.jca, L471
> (https://github.com/jbosstm/narayana/blob/master/ArjunaJTA/jta/classes/com...) the calling method getCurrentTransactionId() potentially could end with
> NullPointerException (caused by TransactionImple#getTransaction when `final BasicAction current = BasicAction.Current()` is null)
> # possible resource leak - connection was ommitted to be closed
> ## org.jboss.narayana.tomcat.jta.integration.TestExecutor
> ## TestCommitMarkableResourceFailAfterCommitOrphan
> # inner class SampleLockable of org.jboss.stm.STMVerticle could be static (http://station5.brq.redhat.com:8080/docs/en/findbugs/fb_checker_ref.html#...)
--
This message was sent by Atlassian JIRA
(v7.2.3#72005)
7 years, 2 months
[JBoss JIRA] (JBTM-2847) Static code analysis - check report and fix issues
by Ondra Chaloupka (JIRA)
Ondra Chaloupka created JBTM-2847:
-------------------------------------
Summary: Static code analysis - check report and fix issues
Key: JBTM-2847
URL: https://issues.jboss.org/browse/JBTM-2847
Project: JBoss Transaction Manager
Issue Type: Task
Affects Versions: 5.5.1.Final
Reporter: Ondra Chaloupka
Assignee: Ondra Chaloupka
Priority: Minor
By going through static code analysis report of changes done during last a half year there are few issues that would be fine to be adjusted.
They are following
# at com.arjuna.ats.internal.jta.transaction.arjunacore.jca.XATerminatorImple,
L614; com.arjuna.ats.internal.jta.transaction.jts.jca, L471
(https://github.com/jbosstm/narayana/blob/master/ArjunaJTA/jta/classes/com...) the calling method getCurrentTransactionId() potentially could end with
NullPointerException (caused by TransactionImple#getTransaction when `final BasicAction current = BasicAction.Current()` is null)
# possible resource leak - connection was ommitted to be closed
## org.jboss.narayana.tomcat.jta.integration.TestExecutor
## TestCommitMarkableResourceFailAfterCommitOrphan
# inner class SampleLockable of org.jboss.stm.STMVerticle could be static (http://station5.brq.redhat.com:8080/docs/en/findbugs/fb_checker_ref.html#...)
--
This message was sent by Atlassian JIRA
(v7.2.3#72005)
7 years, 2 months
[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:
-----------------------------------
Note that naively wrapping this method in a synchronized block may result in trouble as well because doBeforeCommit holds a different lock (com.arjuna.ats.arjuna.coordinator.TwoPhaseCoordinator#_syncLock) during its execution, and other threads can reasonably still be doing work while this is going on. Also that method calls preventCommit() as well, which may result in relevant effects.
> 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
> 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, 2 months
[JBoss JIRA] (JBTM-2846) Thread-safety problem in BasicAction
by David Lloyd (JIRA)
David Lloyd created JBTM-2846:
---------------------------------
Summary: 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
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, 2 months