You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@thrift.apache.org by "James E. King, III (JIRA)" <ji...@apache.org> on 2016/11/12 20:23:58 UTC
[jira] [Updated] (THRIFT-3932) C++ ThreadManager has a rare
termination race
[ https://issues.apache.org/jira/browse/THRIFT-3932?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel ]
James E. King, III updated THRIFT-3932:
---------------------------------------
Description:
{{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.
{panel:title=Resolution Details|borderStyle=dashed|borderColor=#ccc|titleBGColor=#F7D6C1|bgColor=#FFFFCE}
A fair number of things were improved with this pull request:
{panel}
was:
{{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.
> 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.
> {panel:title=Resolution Details|borderStyle=dashed|borderColor=#ccc|titleBGColor=#F7D6C1|bgColor=#FFFFCE}
> A fair number of things were improved with this pull request:
> {panel}
--
This message was sent by Atlassian JIRA
(v6.3.4#6332)