[jira] [Commented] (THRIFT-3932) C++ ThreadManager has a rare termination race

["ASF GitHub Bot (JIRA)" <ji...@apache.org>]

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

ASF GitHub Bot commented on THRIFT-3932:
----------------------------------------

GitHub user jeking3 opened a pull request:

    https://github.com/apache/thrift/pull/1103

    THRIFT-3932: fixed ThreadManager concurrency issues, added more tests in that area, did a little refactoring and prettying up along the way

    See the THRIFT-3932 ticket for more details.

You can merge this pull request into a Git repository by running:

    $ git pull https://github.com/jeking3/thrift defect/THRIFT-3932

Alternatively you can review and apply these changes as the patch at:

    https://github.com/apache/thrift/pull/1103.patch

To close this pull request, make a commit to your master/trunk branch
with (at least) the following in the commit message:

    This closes #1103
    
----
commit 3691db3af28d02e348b79d2c76e527432448b3c6
Author: James E. King, III <ji...@simplivity.com>
Date:   2016-09-30T22:41:17Z

    THRIFT-3932: fixed ThreadManager concurrency issues, added more tests in that area, did a little refactoring and prettying up along the way

----


> 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
>         Attachments: thrift-patch
>
>          Time Spent: 8h
>  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)