activemq-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From "Mark Wuwer (JIRA)" <j...@apache.org>
Subject [jira] Issue Comment Edited: (AMQ-2028) ActiveMQSessionExecutor.taskRunner usage is very non-thread-safe
Date Mon, 09 Nov 2009 19:53:52 GMT

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

Mark Wuwer edited comment on AMQ-2028 at 11/9/09 11:53 AM:
-----------------------------------------------------------

It seems to me that the ActiveMQSessionExecutor is still not very thread safe.

The problem is that in the stop() method the this.taskRunner is set to null without synchronization.

Hence in the wakeup() method within the synchronized block it could happen that this.taskRunner
is set to null between 
creating new TaskRunner and assigning it to the local variable, if the stop() is called in
parallel  - see below:

                        synchronized (this) {
                            if (this.taskRunner == null) {
                                this.taskRunner = session.connection.getSessionTaskRunner().createTaskRunner(this,
                                        "ActiveMQ Session: " + session.getSessionId());
                            }
                            ///////////////// here this.taskRunner can be set to null, if
stop() is called  in between .... //////////////////////
                            taskRunner = this.taskRunner;
                        }
The result would be a NullPointerException in taskRunner.wakeup() two lines below.


      was (Author: wum):
    It seems to me that the ActiveMQSessionExecutor is still not very thread save.

The problem is that in the stop() method the this.taskRunner is set to null without synchronization.

Hence in the wakeup() method within the synchronized block it could happen that this.taskRunner
is set to null between 
creating new TaskRunner and assigning it to the local variable, if the stop() is called in
parallel  - see below:

                        synchronized (this) {
                            if (this.taskRunner == null) {
                                this.taskRunner = session.connection.getSessionTaskRunner().createTaskRunner(this,
                                        "ActiveMQ Session: " + session.getSessionId());
                            }
                            ///////////////// here this.taskRunner can be set to null, if
stop() is called  in between .... //////////////////////
                            taskRunner = this.taskRunner;
                        }
The result would be a NullPointerException in taskRunner.wakeup() two lines below.

  
> ActiveMQSessionExecutor.taskRunner usage is very non-thread-safe
> ----------------------------------------------------------------
>
>                 Key: AMQ-2028
>                 URL: https://issues.apache.org/activemq/browse/AMQ-2028
>             Project: ActiveMQ
>          Issue Type: Bug
>    Affects Versions: 5.3.0
>            Reporter: David Jencks
>            Assignee: David Jencks
>             Fix For: 5.3.0
>
>
> cmon guys,
>                     if (taskRunnerCreated.compareAndSet(false, true)) {
>                         if (taskRunner == null) {
>                             taskRunner = session.connection.getSessionTaskRunner().createTaskRunner(this,
>                                     "ActiveMQ Session: " + session.getSessionId());
>                         }
>                     }
>                     taskRunner.wakeup();
> is not anywhere close to thread safe.
> I'm seeing JmsClientAckTest and JmsRedeliveredTest failing due to this.

-- 
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.


Mime
View raw message