Ensure that SyncWaiter (base/waitable_event_posix.cc) condition variables
cannot be destroyed (pthread_cond_destroy) during a broadcast
(pthread_cond_broadcast). SyncWaiter::Fire now holds the lock until the
broadcast is complete. Given the lock ordering, this guarantees that the
condition variable cannot be destroyed until it is safe to do so. Holding
the lock through the broadcast is harmless, as pthread_cond_wait and
pthread_cond_timedwait will simply block (if needed) until able to take the
lock prior to returning.
Ownership of the lock and condition variable is moved to SyncWaiter for better
self-documentation. The only true functional change here, however, is the
duration that SyncWaiter::Fire holds the lock.
This bug caused hangs in optimized official release builds on the Mac when
DCHECKS were optimized away. The DCHECK optimization was initially covered in
bug 16512 r20497 and was backed out due to bug 16871 r20889. The optimization
changed the timings just enough that we wound up hitting this case. On the
Mac, pthread_cond_broadcast wound up blocking indefinitely (with a trashed
stack) when its condition variable was destroyed beneath it; this caused other
locks held by its thread to never be released, resulting in other threads
becoming deadlocked and made the bug particularly difficult to diagnose.
BUG=19710
TEST=Application works with no unexplained deadlocks, hanging, or crashing
base_unittests (specifically WaitableEvent*)
ipc_tests (especially IPCSyncChannelTest.ChattyServer, which would hang
previously on the Mac with DCHECKs optimized away)
Review URL: http://codereview.chromium.org/173059
git-svn-id: svn://svn.chromium.org/chrome/trunk/src@23790 0039d316-1c4b-4281-b951-d872f2087c98
CrOS-Libchrome-Original-Commit: f968e381c3fba13202b44c63abfbda0f29f1c001
1 file changed