mirror of
https://github.com/sockspls/badfish
synced 2025-04-30 08:43:09 +00:00
Fix race condition where idle_loop() gets called from Split()
SplitPoint member slavesMask wasn't read under lock No functional change.
This commit is contained in:
parent
3ce43c20de
commit
d165d5af91
2 changed files with 12 additions and 1 deletions
|
@ -1622,7 +1622,7 @@ void Thread::idle_loop() {
|
||||||
|
|
||||||
// Pointer 'this_sp' is not null only if we are called from split(), and not
|
// Pointer 'this_sp' is not null only if we are called from split(), and not
|
||||||
// at the thread creation. So it means we are the split point's master.
|
// at the thread creation. So it means we are the split point's master.
|
||||||
const SplitPoint* this_sp = splitPointsSize ? activeSplitPoint : NULL;
|
SplitPoint* this_sp = splitPointsSize ? activeSplitPoint : NULL;
|
||||||
|
|
||||||
assert(!this_sp || (this_sp->masterThread == this && searching));
|
assert(!this_sp || (this_sp->masterThread == this && searching));
|
||||||
|
|
||||||
|
@ -1630,6 +1630,9 @@ void Thread::idle_loop() {
|
||||||
// their work at this split point, return from the idle loop.
|
// their work at this split point, return from the idle loop.
|
||||||
while (!this_sp || this_sp->slavesMask)
|
while (!this_sp || this_sp->slavesMask)
|
||||||
{
|
{
|
||||||
|
if (this_sp)
|
||||||
|
this_sp->mutex.unlock();
|
||||||
|
|
||||||
// If we are not searching, wait for a condition to be signaled instead of
|
// If we are not searching, wait for a condition to be signaled instead of
|
||||||
// wasting CPU time polling for work.
|
// wasting CPU time polling for work.
|
||||||
while ((!searching && Threads.sleepWhileIdle) || exit)
|
while ((!searching && Threads.sleepWhileIdle) || exit)
|
||||||
|
@ -1721,6 +1724,9 @@ void Thread::idle_loop() {
|
||||||
// unsafe because if we are exiting there is a chance are already freed.
|
// unsafe because if we are exiting there is a chance are already freed.
|
||||||
sp->mutex.unlock();
|
sp->mutex.unlock();
|
||||||
}
|
}
|
||||||
|
|
||||||
|
if(this_sp)
|
||||||
|
this_sp->mutex.lock();
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
|
||||||
|
|
|
@ -312,6 +312,7 @@ void Thread::split(Position& pos, Stack* ss, Value alpha, Value beta, Value* bes
|
||||||
sp.mutex.unlock();
|
sp.mutex.unlock();
|
||||||
Threads.mutex.unlock();
|
Threads.mutex.unlock();
|
||||||
|
|
||||||
|
// Calling idle_loop with sp.mutex locked
|
||||||
Thread::idle_loop(); // Force a call to base class idle_loop()
|
Thread::idle_loop(); // Force a call to base class idle_loop()
|
||||||
|
|
||||||
// In helpful master concept a master can help only a sub-tree of its split
|
// In helpful master concept a master can help only a sub-tree of its split
|
||||||
|
@ -322,6 +323,10 @@ void Thread::split(Position& pos, Stack* ss, Value alpha, Value beta, Value* bes
|
||||||
// We have returned from the idle loop, which means that all threads are
|
// We have returned from the idle loop, which means that all threads are
|
||||||
// finished. Note that setting 'searching' and decreasing splitPointsSize is
|
// finished. Note that setting 'searching' and decreasing splitPointsSize is
|
||||||
// done under lock protection to avoid a race with Thread::is_available_to().
|
// done under lock protection to avoid a race with Thread::is_available_to().
|
||||||
|
// idle_loop returns with sp.mutex locked but we must unlock it inorder to
|
||||||
|
// lock Threads.mutex without conflicting with check_time() (threads holding
|
||||||
|
// multiple locks must always acquired them in the same order to avoid deadlocks)
|
||||||
|
sp.mutex.unlock();
|
||||||
Threads.mutex.lock();
|
Threads.mutex.lock();
|
||||||
sp.mutex.lock();
|
sp.mutex.lock();
|
||||||
}
|
}
|
||||||
|
|
Loading…
Add table
Reference in a new issue