You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@activemq.apache.org by "Mark Wuwer (JIRA)" <ji...@apache.org> on 2009/11/09 20:53:52 UTC

[jira] Issue Comment Edited: (AMQ-2028) ActiveMQSessionExecutor.taskRunner usage is very non-thread-safe

    [ 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.