Careful SMP locking - Fix very occasional hangs
authorJoona Kiiski <joona.kiiski@gmail.com>
Sun, 6 Sep 2015 20:36:59 +0000 (21:36 +0100)
committerJoona Kiiski <joona.kiiski@gmail.com>
Thu, 10 Sep 2015 18:15:43 +0000 (19:15 +0100)
commit613dc66c12b9bf357ae38129fb745dd8c2c035b1
tree8682cba3c2ea67545d49aa2cbace44c515a33d8a
parent3e2591d83c285597a7400c79b61ab9dc38875163
Careful SMP locking - Fix very occasional hangs

Louis Zulli reported that Stockfish suffers from very occasional hangs with his 20 cores machine.

Careful SMP debugging revealed that this was caused by "a ghost split point slave", where thread
was marked as a split point slave, but wasn't actually working on it.

The only logical explanation for this was double booking, where due to SMP race, the same thread
is booked for two different split points simultaneously.

Due to very intermittent nature of the problem, we can't say exactly how this happens.

The current handling of Thread specific variables is risky though. Volatile variables are in some
cases changed without spinlock being hold. In this case standard doesn't give us any kind of
guarantees about how the updated values are propagated to other threads.

We resolve the situation by enforcing very strict locking rules:
- Values for key thread variables (splitPointsSize, activeSplitPoint, searching)
can only be changed when the thread specific spinlock is held.
- Structural changes for splitPoints[] are only allowed when the thread specific spinlock is held.
- Thread booking decisions (per split point) can only be done when the thread specific spinlock is held.

With these changes hangs didn't occur anymore during 2 days torture testing on Zulli's machine.

We probably have a slight performance penalty in SMP mode due to more locking.

STC (7 threads):
ELO: -1.00 +-2.2 (95%) LOS: 18.4%
Total: 30000 W: 4538 L: 4624 D: 20838

However stability is worth more than 1-2 ELO points in this case.

No functional change

Resolves #422
src/search.cpp
src/thread.cpp