Fixed a potential deadlock in folly::RWSpinLock
authorYang Ni <yangn@fb.com>
Sat, 15 Dec 2012 18:05:15 +0000 (10:05 -0800)
committerJordan DeLong <jdelong@fb.com>
Sat, 19 Jan 2013 00:37:05 +0000 (16:37 -0800)
commit3692ff51d35e5e38fde557d61750c884340ec2fe
treeb2d1631d155c71c708410ea0b169fee28f201a51
parentdca19fc4ef14f2b4869271e51d608624ca23a4b1
Fixed a potential deadlock in folly::RWSpinLock

Summary:
The current UpgradedHolder implementation may lead to a deadlock when upgrading from a reader lock, because it is blocking.

A reader that has failed to set the UPGRADED bit may block the
winner (upgrader) that has successfully set the UPGRADED bit, while
waiting to upgrade in an infinite loop without releasing its own
reader lock. The upgrader needs to wait for all reader locks to be
released before it can move forward.

This is the code pattern:

{
ReadHolder reader(lock);
UpgradedHolder upgrader(std::move(reader));
WriteHolder writer(std::move(upgrader));
}

To avoid this to happen, removed UpgradedHolder(ReadHolder&&)
constructor and the function that impelments it
unlock_shared_and_lock_upgrade. This would require a programmer explicitly
release a read lock before acquiring an upgrade lock, therefore
avoid the above mentioned deadlock.

In addition, the current folly::RWSpinLock::try_lock_shared()
implementation does not check the UPGRADED bit at all. The UPGRADED bit can be used to avoid starving writers.  This diff fixed this by correctly checking the UPGRADED bit.

Test Plan:
Passed folly unit tests.
Tested in a Facebook service that uses the
folly::RWSpinLock::UpgradedHolder.

Reviewed By: xliux@fb.com

FB internal diff: D659875
folly/RWSpinLock.h
folly/test/RWSpinLockTest.cpp