[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=#CE}
A fair number of things were improved with this pull request:
# There are three monitors used for signaling; two of them use the same mutex
and then the workerMonitor is using its own. I made all three use the same
mutex.
# I replaced all calls to Synchronize with Guard on the mutex_ so it is a bit
clearer that there is one lock protecting everything.
# The worker run loop expires tasks as it encounters them. Now we only call
removeExpiredTasks if there's no room during add.
# I found that when worker threads are removed they are not joined; if a thread
is not detached we now join those threads.
# I added better documentation and simplified the Worker::run() a little so it
is easier to understand, but the logic is the same. idle_ was not used and
removed.
# I found that remove(Task) simply wasn't implemented at all! so I added code
for that and tests.
# Release mode tests of thread manager would not fail on error because they use
assert() to verify results! If we only do release builds in CI, these tests may
have been silently failing for a long time.
# ThreadManager::join() is unnecessary, as stop() will join worker threads (as
will removeWorker) depending on the ThreadFactory's detached disposition.
# Cleaned up some technical debt from the TServerFramework refactoring:
# Cleaned up some things in the different platform thread factories. Some
abstractions were unnecessary.
# Reduced overall time on concurrency test to about 30 seconds on my dev system
while increasing the reliability and improving test coverage significantly. I
ran it 100 times in a loop.
# Added tests for most of the ThreadManager APIs.
# Tested with posix, std, and boost threads on linux.
# Tested on Windows.
# Fixed a math error in time calculation that was present on Windows (with
VS2010) which made expiring tasks impossible (unit test proved the issue and
the fix).
# Re-enable unit tests in the appveyor build that are no longer unstable.
{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.
{panel:title=Resolution
Details|borderStyle=dashed|borderColor=#ccc|titleBGColor=#F7D6C1|bgColor=#CE}
A fair number of things were improved with this pull request:
{panel}
> C++ ThreadManager has a rare termination race
> -
>
> Key:
[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=#CE}
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=#CE}
> A fair number of things were improved with this pull request:
> {panel}
--
This message was sent by Atlassian JIRA
(v6.3.4#6332)
[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
]
Jake Farrell updated THRIFT-3932:
-
Fix Version/s: 0.11.0
> 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)
[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:
---
Component/s: C++ - Library
> 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
>
>
> {{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)
[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
]
Buğra Gedik updated THRIFT-3932:
Attachment: thrift-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
>Reporter: Buğra Gedik
> Attachments: thrift-patch
>
>
> {{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)
[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
]
Buğra Gedik updated THRIFT-3932:
Attachment: (was: thrift-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
>Reporter: Buğra Gedik
>
> {{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)
[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
]
Buğra Gedik updated THRIFT-3932:
Attachment: thrift-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
>Reporter: Buğra Gedik
> Attachments: thrift-patch
>
>
> {{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)
[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
]
Buğra Gedik updated THRIFT-3932:
Attachment: (was: thrift-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
>Reporter: Buğra Gedik
> Attachments: thrift-patch
>
>
> {{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)
[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
]
Buğra Gedik updated THRIFT-3932:
Attachment: thrift-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
>Reporter: Buğra Gedik
> Attachments: thrift-patch
>
>
> {{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)
[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
]
Buğra Gedik updated THRIFT-3932:
Attachment: patch.rtf
> C++ ThreadManager has a rare termination race
> -
>
> Key: THRIFT-3932
> URL: https://issues.apache.org/jira/browse/THRIFT-3932
> Project: Thrift
> Issue Type: Bug
>Reporter: Buğra Gedik
>
> {{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)
[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
]
Buğra Gedik updated THRIFT-3932:
Attachment: (was: patch.rtf)
> C++ ThreadManager has a rare termination race
> -
>
> Key: THRIFT-3932
> URL: https://issues.apache.org/jira/browse/THRIFT-3932
> Project: Thrift
> Issue Type: Bug
>Reporter: Buğra Gedik
>
> {{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)
[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
]
Buğra Gedik updated THRIFT-3932:
Attachment: patch.rtf
> C++ ThreadManager has a rare termination race
> -
>
> Key: THRIFT-3932
> URL: https://issues.apache.org/jira/browse/THRIFT-3932
> Project: Thrift
> Issue Type: Bug
>Reporter: Buğra Gedik
>
> {{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)
[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
]
Buğra Gedik 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.
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.
> C++ ThreadManager has a rare termination race
> -
>
> Key: THRIFT-3932
> URL: https://issues.apache.org/jira/browse/THRIFT-3932
> Project: Thrift
> Issue Type: Bug
>Reporter: Buğra Gedik
>
> {{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)
[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
]
Buğra Gedik updated THRIFT-3932:
Attachment: (was: patch.rtf)
> C++ ThreadManager has a rare termination race
> -
>
> Key: THRIFT-3932
> URL: https://issues.apache.org/jira/browse/THRIFT-3932
> Project: Thrift
> Issue Type: Bug
>Reporter: Buğra Gedik
>
> {{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)
[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
]
Buğra Gedik updated THRIFT-3932:
Attachment: (was: thrift-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
>Reporter: Buğra Gedik
>
> {{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.
--
This message was sent by Atlassian JIRA
(v6.3.4#6332)
[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
]
Buğra Gedik 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 small fix that addresses these problems.
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 {{ThreadManage}}r'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 small fix that addresses these problems.
> C++ ThreadManager has a rare termination race
> -
>
> Key: THRIFT-3932
> URL: https://issues.apache.org/jira/browse/THRIFT-3932
> Project: Thrift
> Issue Type: Bug
>Reporter: Buğra Gedik
> Attachments: thrift-patch
>
>
> {{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 small fix that addresses these problems.
--
This message was sent by Atlassian JIRA
(v6.3.4#6332)
[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
]
Buğra Gedik 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 {{ThreadManage}}r'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 small fix that addresses these problems.
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 first, and the ThreadManager's {{removeWorker}} method
unblocks and eventually the 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 small fix that addresses these problems.
> C++ ThreadManager has a rare termination race
> -
>
> Key: THRIFT-3932
> URL: https://issues.apache.org/jira/browse/THRIFT-3932
> Project: Thrift
> Issue Type: Bug
>Reporter: Buğra Gedik
> Attachments: thrift-patch
>
>
> {{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 {{ThreadManage}}r'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 small fix that addresses these problems.
--
This message was sent by Atlassian JIRA
(v6.3.4#6332)
[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
]
Buğra Gedik 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 first, and the ThreadManager's {{removeWorker}} method
unblocks and eventually the 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 small fix that solves these problems.
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 first, and the ThreadManager's {{removeWorker}} method
unblocks and eventually the 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, the ThreadManager never joins its threads.
Attached is a small fix that solves these problems.
> C++ ThreadManager has a rare termination race
> -
>
> Key: THRIFT-3932
> URL: https://issues.apache.org/jira/browse/THRIFT-3932
> Project: Thrift
> Issue Type: Bug
>Reporter: Buğra Gedik
> Attachments: thrift-patch
>
>
> {{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 first, and the ThreadManager's {{removeWorker}}
> method unblocks and eventually the 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 small fix that solves these problems.
--
This message was sent by Atlassian JIRA
(v6.3.4#6332)
[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
]
Buğra Gedik 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 first, and the ThreadManager's {{removeWorker}} method
unblocks and eventually the 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 small fix that addresses these problems.
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 first, and the ThreadManager's {{removeWorker}} method
unblocks and eventually the 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 small fix that solves these problems.
> C++ ThreadManager has a rare termination race
> -
>
> Key: THRIFT-3932
> URL: https://issues.apache.org/jira/browse/THRIFT-3932
> Project: Thrift
> Issue Type: Bug
>Reporter: Buğra Gedik
> Attachments: thrift-patch
>
>
> {{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 first, and the ThreadManager's {{removeWorker}}
> method unblocks and eventually the 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 small fix that addresses these problems.
--
This message was sent by Atlassian JIRA
(v6.3.4#6332)
[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
]
Buğra Gedik updated THRIFT-3932:
Description:
{{ThreadManger::join}} calls {{stopImpl(true)}}, which in term 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 first, and the ThreadManager's {{removeWorker}} method
unblocks and eventually the 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, the ThreadManager never joins its threads.
Attached is a small fix that solves these problems.
was:
{{ThreadManger::join}} calls {{stopImpl(true)}}, which in term 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 first, and the ThreadManager's {{removeWorker}} method
unblocks and eventually the 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, the ThreadManager never joins its threads.
Attached is a small fix that alleviates these problems.
> C++ ThreadManager has a rare termination race
> -
>
> Key: THRIFT-3932
> URL: https://issues.apache.org/jira/browse/THRIFT-3932
> Project: Thrift
> Issue Type: Bug
>Reporter: Buğra Gedik
> Attachments: thrift-patch
>
>
> {{ThreadManger::join}} calls {{stopImpl(true)}}, which in term 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 first, and the ThreadManager's {{removeWorker}}
> method unblocks and eventually the 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, the ThreadManager never joins its threads.
> Attached is a small fix that solves these problems.
--
This message was sent by Atlassian JIRA
(v6.3.4#6332)
[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
]
Buğra Gedik updated THRIFT-3932:
Description:
{{ThreadManger::join}} calls {{stopImpl(true)}}, which in term 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 first, and the ThreadManager's {{removeWorker}} method
unblocks and eventually the 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, the ThreadManager never joins its threads.
Attached is a small fix that alleviates these problems.
was:
{{ThreadManger::join}} calls {{stopImpl(true)}}, which in term calls
{{removeWorker(workerCount_);}}. The latter waits until {{while (workerCount_
!= workerMaxCount_)}}. In the run method, the last thread that detects
{{workerCount_ == workerMaxCount_}} notifies the {{removeWorker}} method.
However, 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 first, and the ThreadManager's {{removeWorker}} method
unblocks and eventually the ThreadManager's {{join}} returns and the object
destructed. When the other thread reaches the synchronized block above, it will
crash, as the manager is not around anymore.
Besides, the ThreadManager never joins its threads.
Attached is a small fix that alleviates the problem.
> C++ ThreadManager has a rare termination race
> -
>
> Key: THRIFT-3932
> URL: https://issues.apache.org/jira/browse/THRIFT-3932
> Project: Thrift
> Issue Type: Bug
>Reporter: Buğra Gedik
> Attachments: thrift-patch
>
>
> {{ThreadManger::join}} calls {{stopImpl(true)}}, which in term 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 first, and the ThreadManager's {{removeWorker}}
> method unblocks and eventually the 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, the ThreadManager never joins its threads.
> Attached is a small fix that alleviates these problems.
--
This message was sent by Atlassian JIRA
(v6.3.4#6332)
[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
]
Buğra Gedik updated THRIFT-3932:
Attachment: thrift-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
>Reporter: Buğra Gedik
> Attachments: thrift-patch
>
>
> {{ThreadManger::join}} calls {{stopImpl(true)}}, which in term calls
> {{removeWorker(workerCount_);}}. The latter waits until {{while (workerCount_
> != workerMaxCount_)}}. In the run method, the last thread that detects
> {{workerCount_ == workerMaxCount_}} notifies the {{removeWorker}} method.
> However, 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 first, and the ThreadManager's {{removeWorker}}
> method unblocks and eventually the ThreadManager's {{join}} returns and the
> object destructed. When the other thread reaches the synchronized block
> above, it will crash, as the manager is not around anymore.
> Besides, the ThreadManager never joins its threads.
> Attached is a small fix that alleviates the problem.
--
This message was sent by Atlassian JIRA
(v6.3.4#6332)
