diff --git a/dbtests/threadedtests.cpp b/dbtests/threadedtests.cpp index bd9ffea4cf3..a56af7dc019 100644 --- a/dbtests/threadedtests.cpp +++ b/dbtests/threadedtests.cpp @@ -464,7 +464,7 @@ namespace ThreadedTests { } }; - class UpgradableTest : public ThreadedTest<5> { + class UpgradableTest : public ThreadedTest<7> { RWLock m; public: UpgradableTest() : m("utest") {} @@ -477,11 +477,15 @@ namespace ThreadedTests { R = get a read lock and we expect it to be fast w = write lock */ - const char *what = " RURwR"; + // /-- verify upgrade can be done instantly while in a read lock already + // | /-- verify upgrade acquisition isn't greedy + // | | /-- verify writes aren't greedy while in upgradable + // v v v + const char *what = " RURuRwR"; sleepmillis(100*x); - log() << x << " " << what[x] << endl; + log() << x << what[x] << " request" << endl; switch( what[x] ) { case 'w': { @@ -497,13 +501,20 @@ namespace ThreadedTests { { Timer t; m.lockAsUpgradable(); - log() << x << " U got\n" << endl; + log() << x << " U got" << endl; if( what[x] == 'U' ) { - ASSERT( t.millis() < 15 ); + if( t.millis() > 20 ) { + DEV { + // a _DEBUG buildbot might be slow, try to avoid false positives + log() << "warning lock upgrade was slow " << t.millis() << endl; + } + else { + ASSERT( false ); + } + } } - sleepsecs(5); - cout << endl; - log() << x << " U unlock\n" << endl; + sleepsecs(1); + log() << x << " U unlock" << endl; m.unlockFromUpgradable(); } break; @@ -518,25 +529,61 @@ namespace ThreadedTests { log() << "warning: when in upgradable write locks are still greedy on this platform" << endl; } } - sleepmillis(100); + sleepmillis(200); log() << x << " R unlock" << endl; m.unlock_shared(); } break; default: - log() << "default? " << x << endl; + ASSERT(false); } cc().shutdown(); } }; + class WriteLocksAreGreedy : public ThreadedTest<3> { + public: + WriteLocksAreGreedy() : m("gtest") {} + private: + RWLock m; + virtual void validate() { } + virtual void subthread(int x) { + Client::initThread("utest"); + if( x == 1 ) { + cout << mongo::curTimeMillis64() % 10000 << " 1" << endl; + rwlock_shared lk(m); + sleepmillis(300); + cout << mongo::curTimeMillis64() % 10000 << " 1x" << endl; + } + if( x == 2 ) { + sleepmillis(100); + cout << mongo::curTimeMillis64() % 10000 << " 2" << endl; + rwlock lk(m, true); + //m._lock(); + cout << mongo::curTimeMillis64() % 10000 << " 2x" << endl; + //m.unlock(); + } + if( x == 3 ) { + sleepmillis(200); + Timer t; + cout << mongo::curTimeMillis64() % 10000 << " 3" << endl; + rwlock_shared lk(m); + cout << mongo::curTimeMillis64() % 10000 << " 3x" << endl; + cout << t.millis() << endl; + ASSERT( t.millis() > 50 ); + } + cc().shutdown(); + } + }; + class All : public Suite { public: All() : Suite( "threading" ) { } void setupTests() { - add< UpgradableTest >(); + add< WriteLocksAreGreedy >(); + //add< UpgradableTest >(); add< List1Test >(); add< List1Test2 >(); diff --git a/util/concurrency/rwlock.h b/util/concurrency/rwlock.h index a296bbdf5c5..7a4e716f646 100644 --- a/util/concurrency/rwlock.h +++ b/util/concurrency/rwlock.h @@ -104,32 +104,34 @@ namespace mongo { #elif defined(BOOST_RWLOCK) - // Boost RWLock implementation + // Boost based RWLock implementation class RWLock : boost::noncopyable { shared_mutex _m; const int _lowPriorityWaitMS; public: -#if defined(_DEBUG) - const char *_name; + const char * const _name; + RWLock(const char *name, int lowPriorityWait=0) : _lowPriorityWaitMS(lowPriorityWait) , _name(name) { } -#else - RWLock(const char *, int lowPriorityWait=0) : _lowPriorityWaitMS(lowPriorityWait) { } -#endif const char * implType() const { return "boost"; } int lowPriorityWaitMS() const { return _lowPriorityWaitMS; } void lock() { - _m.lock(); -#if defined(_DEBUG) - mutexDebugger.entering(_name); -#endif + _m.lock(); + DEV mutexDebugger.entering(_name); } + + /*void lock() { + // This sequence gives us the lock semantics we want: specifically that write lock acquisition is + // greedy EXCEPT when someone already is in upgradable state. + lockAsUpgradable(); + upgrade(); + DEV mutexDebugger.entering(_name); + }*/ + void unlock() { -#if defined(_DEBUG) - mutexDebugger.leaving(_name); -#endif + DEV mutexDebugger.leaving(_name); _m.unlock(); } @@ -159,9 +161,7 @@ namespace mongo { bool lock_try( int millis = 0 ) { if( _m.timed_lock( boost::posix_time::milliseconds(millis) ) ) { -#if defined(_DEBUG) - mutexDebugger.entering(_name); -#endif + DEV mutexDebugger.entering(_name); return true; } return false; @@ -174,20 +174,16 @@ namespace mongo { class RWLock : boost::noncopyable { pthread_rwlock_t _lock; const int _lowPriorityWaitMS; - inline static void check( int x ) { - if( x == 0 ) + static void check( int x ) { + IF( x == 0 ) return; log() << "pthread rwlock failed: " << x << endl; assert( x == 0 ); } public: -#if defined(_DEBUG) const char *_name; RWLock(const char *name, int lowPriorityWaitMS=0) : _lowPriorityWaitMS(lowPriorityWaitMS), _name(name) -#else - RWLock(const char *, int lowPriorityWaitMS=0) : _lowPriorityWaitMS( lowPriorityWaitMS ) -#endif { check( pthread_rwlock_init( &_lock , 0 ) ); } @@ -204,14 +200,10 @@ namespace mongo { void lock() { check( pthread_rwlock_wrlock( &_lock ) ); -#if defined(_DEBUG) - mutexDebugger.entering(_name); -#endif + DEV mutexDebugger.entering(_name); } void unlock() { -#if defined(_DEBUG) mutexDebugger.leaving(_name); -#endif check( pthread_rwlock_unlock( &_lock ) ); } @@ -229,9 +221,7 @@ namespace mongo { bool lock_try( int millis = 0 ) { if( _try( millis , true ) ) { -#if defined(_DEBUG) - mutexDebugger.entering(_name); -#endif + DEV mutexDebugger.entering(_name); return true; } return false; @@ -354,6 +344,7 @@ namespace mongo { dassert( _state.get() < 0 ); } + // RWLockRecursive::Exclusive scoped lock class Exclusive : boost::noncopyable { RWLockRecursive& _r; rwlock *_scopedLock; @@ -373,6 +364,7 @@ namespace mongo { } }; + // RWLockRecursive::Shared scoped lock class Shared : boost::noncopyable { RWLockRecursive& _r; bool _alreadyExclusive;