[jira] [Comment Edited] (AMQ-5263) Method ActiveMQSession.close() is not thread safe as declared and required

["Krzysztof Mackowiak (JIRA)" <ji...@apache.org>]

    [ https://issues.apache.org/jira/browse/AMQ-5263?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=16611946#comment-16611946 ] 

Krzysztof Mackowiak edited comment on AMQ-5263 at 9/12/18 11:34 AM:
--------------------------------------------------------------------

In my team we experienced the same issue with ActiveMQSession.close() and ActiveMQSession.commit() methods running concurrently. In our case the bug manifested not by NullPointerException, but by losing messages during failover between 2 instances of ActiveMQ broker.

According to ActiveMQSession documentation calling close() method in another thread than a thread which is handling messages should be thread-safe. This method is called during failover from one instance of ActiveMQ broker to another when the master is stopped.Unfortunately, it looks that close() method is NOT safe to call from another thread. When it is called from another thread we may end up with a situation when ActiveMQSession.commit() method ends without any exception but underneath ‘rollback message’ (TransactionInfo with type ROLLBACK) is sent to a broker. So a message is lost.

I've attached a test which simulates this situation. [^AMQ-5263_test_for_ActiveMQSession_close()_thread-safety.patch]


was (Author: kmackowiak):
In my team we experienced the same issue with ActiveMQSession.close() and ActiveMQSession.commit() methods running concurrently. In our case the bug manifested not by NullPointerException, but by losing messages during failover between 2 instances of ActiveMQ broker.

According to ActiveMQSession documentation calling close() method in another thread than a thread which is handling messages should be thread-safe. This method is called during failover from one instance of ActiveMQ broker to another when the master is stopped.Unfortunately, it looks that close() method is NOT safe to call from another thread. When it is called from another thread we may end up with a situation when ActiveMQSession.commit() method ends without any exception but underneath ‘rollback message’ (TransactionInfo with type ROLLBACK) is sent to a broker. So a message is lost.

I've attached a test which simulates this situation.

> Method ActiveMQSession.close() is not thread safe as declared and required
> --------------------------------------------------------------------------
>
>                 Key: AMQ-5263
>                 URL: https://issues.apache.org/jira/browse/AMQ-5263
>             Project: ActiveMQ
>          Issue Type: Bug
>          Components: JMS client
>    Affects Versions: 5.9.1
>         Environment: any operationg system, and any platform
>            Reporter: Jiang Fuqiang
>            Priority: Major
>         Attachments: AMQ-5263_bugfix_for_ActiveMQSession_close()_thread-safety.patch, AMQ-5263_test_for_ActiveMQSession_close()_thread-safety.patch
>
>
> If ActiveMQSession.close()  and  ActiveMQSession.dispose() run concurrently,  the statements in close() method might throw NullPointerException。Just marked by the following code snippet.
> -------------------------------
>     public void ActiveMQSession.close() throws JMSException {
>         if (!closed) {
>             if (getTransactionContext().isInXATransaction()) { // it might throw NullPointerException
> 			...
> -------------------------------
> In my application, I call Session.close() when I detect JMSException, but Session.close() throws NullPointerException occasionally。I suspect that's because ActiveMQConnection called ActiveMQSession.dispose() in case of transport error, and  ActiveMQSession.dispose() nullified ActiveMQSession.transactionContext before ActiveMQSession.closed is assigned to false. Since this NullPointerException error just happened once and can not be reproduced, it's only a suspection. 
> It seems that close() and dispose() should be synchronized, and then sessions can be closed safely.
> I'm not good at English, but I hope this issue is demonstrated clearly enough.



--
This message was sent by Atlassian JIRA
(v7.6.3#76005)