1
0
Fork 0
mirror of https://github.com/sockspls/badfish synced 2025-05-01 01:03:09 +00:00

Fix 'stop' flag changed out of lock protection

This is the first nice effect of previous patch !

Because thread_should_stop() should be declared 'const' we
need to remove the setting of 'stop' flag to true that
turns out to be a bug because thread_should_stop() is called
outside from lock protection while 'stop' flag is a volatile
shared variable so cannot be changed when not in lock.

Note that this bugs fires ONLY when we use more then 2 threads,
so commonly only in a QUAD or OCTAL machine.

Signed-off-by: Marco Costalba <mcostalba@gmail.com>
This commit is contained in:
Marco Costalba 2010-02-14 13:31:05 +01:00
parent 2b740f5495
commit 8eae6a95fb

View file

@ -81,7 +81,7 @@ namespace {
void get_beta_counters(Color us, int64_t& our, int64_t& their) const; void get_beta_counters(Color us, int64_t& our, int64_t& their) const;
bool idle_thread_exists(int master) const; bool idle_thread_exists(int master) const;
bool thread_is_available(int slave, int master) const; bool thread_is_available(int slave, int master) const;
bool thread_should_stop(int threadID); bool thread_should_stop(int threadID) const;
void wake_sleeping_threads(); void wake_sleeping_threads();
void put_threads_to_sleep(); void put_threads_to_sleep();
void idle_loop(int threadID, SplitPoint* waitSp); void idle_loop(int threadID, SplitPoint* waitSp);
@ -1881,10 +1881,12 @@ namespace {
/* Here we have the lock still grabbed */ /* Here we have the lock still grabbed */
// If this is the master thread and we have been asked to stop because of // If this is the master thread and we have been asked to stop because of
// a beta cutoff higher up in the tree, stop all slave threads. // a beta cutoff higher up in the tree, stop all slave threads. Note that
// thread_should_stop(threadID) does not imply that 'stop' flag is set, so
// do this explicitly now, under lock protection.
if (sp->master == threadID && TM.thread_should_stop(threadID)) if (sp->master == threadID && TM.thread_should_stop(threadID))
for (int i = 0; i < TM.active_threads(); i++) for (int i = 0; i < TM.active_threads(); i++)
if (sp->slaves[i]) if (sp->slaves[i] || i == threadID)
TM.set_stop_request(i); TM.set_stop_request(i);
sp->cpus--; sp->cpus--;
@ -2016,10 +2018,12 @@ namespace {
/* Here we have the lock still grabbed */ /* Here we have the lock still grabbed */
// If this is the master thread and we have been asked to stop because of // If this is the master thread and we have been asked to stop because of
// a beta cutoff higher up in the tree, stop all slave threads. // a beta cutoff higher up in the tree, stop all slave threads. Note that
// thread_should_stop(threadID) does not imply that 'stop' flag is set, so
// do this explicitly now, under lock protection.
if (sp->master == threadID && TM.thread_should_stop(threadID)) if (sp->master == threadID && TM.thread_should_stop(threadID))
for (int i = 0; i < TM.active_threads(); i++) for (int i = 0; i < TM.active_threads(); i++)
if (sp->slaves[i]) if (sp->slaves[i] || i == threadID)
TM.set_stop_request(i); TM.set_stop_request(i);
sp->cpus--; sp->cpus--;
@ -2764,7 +2768,7 @@ namespace {
// cutoff has occurred in the thread's currently active split point, or in // cutoff has occurred in the thread's currently active split point, or in
// some ancestor of the current split point. // some ancestor of the current split point.
bool ThreadsManager::thread_should_stop(int threadID) { bool ThreadsManager::thread_should_stop(int threadID) const {
assert(threadID >= 0 && threadID < ActiveThreads); assert(threadID >= 0 && threadID < ActiveThreads);
@ -2778,10 +2782,7 @@ namespace {
for (sp = threads[threadID].splitPoint; sp != NULL; sp = sp->parent) for (sp = threads[threadID].splitPoint; sp != NULL; sp = sp->parent)
if (sp->finished) if (sp->finished)
{
threads[threadID].stop = true;
return true; return true;
}
return false; return false;
} }