SharedMutex potential lost wakeup with exactly 3 or 4 contending writers
Summary:
SharedMutex used a saturating counter that records the number of
waiting lock() calls, but an ABA problem on futexWait could lead to a lost
wakeup when there was exactly 3 or 4 threads contending on the RW lock
in W mode. This diff changes the kWaitingE count to be heuristic (it is
possible that the count says 1 but there are two waiters), saturates at
2 instead of 3 (because there is no benefit from differentiating those
two), and doesn't decrement the count on a successful wakeup.
Also, I noticed while debugging this that boost::noncopyable was causing
SharedMutex to be 8 bytes when it should only be 4.
One way the wakeup could be lost in the old code:
1. A calls lock()
2. A updates state <- kHasE
3. A returns
4. B calls lock()
5. B spins
6. B updates state <- kHasE + 1 * kIncrWaitingE
7. A calls unlock()
8. A updates state <- 0
9. A calls futexWake(), which returns 0
10. A calls lock()
11. A updates state <- kHasE
12. A returns
13. C calls lock()
14. C spins
15. C updates state <- kHasE + 1 * kIncrWaitingE
16. C calls futexWait, expecting kHasE + 1 * kIncrWaitingE
17. B calls futexWait, expecting kHasE + 1 * kIncrWaitingE
18. A calls unlock()
19. A updates state <- 0
20. A calls futexWake(), which returns 1
21. C receives the wakeup
22. C updates state <- kHasE
23. C returns
24. C calls unlock()
25. C updates state <- 0
B missed the wakeup that was intended for it (sent at step 9, wait
started at step 17), but went to sleep anyway because it saw the write
state at step 17. Now there are two waiters but only 1 recorded in the
SharedMutex, at which point failure is inevitable.
Test Plan:
1. DeterministicSchedule test using uniformSubset that can repro the problem
2. Test in production scenario that produced occasional deadlocks under high stress
Reviewed By: yfeldblum@fb.com
Subscribers: folly-diffs@, yfeldblum, chalfant
FB internal diff:
D1980210
Tasks:
6720328
Signature: t1:
1980210:
1428623932:
ef1c00c3f88154578b2b253ac0cfdbadf9f31d8c