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

Jake Farrell commented on THRIFT-3932:
--------------------------------------

Ran through the patch and with the size and number of files that this is 
touching in the C++ lib I would error on the side of caution and look at 
committing this right after 0.10.0 is released so it does not block the release 
and it can get proper testing and soak time in before 0.11.0 gets cut. 

[~ben.craig] do you have a second to review this patch?

> C++ ThreadManager has a rare termination race
> ---------------------------------------------
>
>                 Key: THRIFT-3932
>                 URL: https://issues.apache.org/jira/browse/THRIFT-3932
>             Project: Thrift
>          Issue Type: Bug
>          Components: C++ - Library
>            Reporter: Bu─čra Gedik
>            Assignee: James E. King, III
>             Fix For: 0.11.0
>
>         Attachments: thrift-patch
>
>          Time Spent: 17h
>  Remaining Estimate: 0h
>
> {{ThreadManger::join}} calls {{stopImpl(true)}}, which in turn calls 
> {{removeWorker(workerCount_);}}. The latter waits until {{while (workerCount_ 
> != workerMaxCount_)}}. Within the {{run}} method of the workers, the last 
> thread that detects {{workerCount_ == workerMaxCount_}} notifies 
> {{removeWorker}}. The {{run}} method has the following additional code that 
> is executed at the very end:
> {code}
>     {
>       Synchronized s(manager_->workerMonitor_);
>       manager_->deadWorkers_.insert(this->thread());
>       if (notifyManager) {
>         manager_->workerMonitor_.notify();
>       }
>     }
> {code}
> This is an independent synchronized block. Now assume 2 threads. One of them 
> has {{notifyManager=true}} as it detected the {{workerCount_ == 
> workerMaxCount_}} condition earlier. It is possible that this thread gets to 
> execute  the above code block first, {{ThreadManager}}'s {{removeWorker}} 
> method unblocks, and eventually {{ThreadManager}}'s {{join}} returns and the 
> object is destructed. When the other thread reaches the synchronized block 
> above, it will crash, as the manager is not around anymore.
> Besides, {{ThreadManager}} never joins its threads.
> Attached is a patch.



--
This message was sent by Atlassian JIRA
(v6.3.4#6332)

Reply via email to