From ee13cc17005b60b4a427d3674f51577abf4f6776 Mon Sep 17 00:00:00 2001 From: Oliver Kowalke Date: Fri, 18 Oct 2013 17:41:08 +0200 Subject: [PATCH] make mutex thread-safe --- include/boost/fiber/mutex.hpp | 5 +- include/boost/fiber/recursive_mutex.hpp | 7 +- include/boost/fiber/recursive_timed_mutex.hpp | 7 +- include/boost/fiber/timed_mutex.hpp | 5 +- src/mutex.cpp | 22 +++++-- src/recursive_mutex.cpp | 41 +++++++----- src/recursive_timed_mutex.cpp | 65 ++++++++++++------- src/timed_mutex.cpp | 48 ++++++++++---- 8 files changed, 138 insertions(+), 62 deletions(-) diff --git a/include/boost/fiber/mutex.hpp b/include/boost/fiber/mutex.hpp index 6abe3ee8..7ced9d6c 100644 --- a/include/boost/fiber/mutex.hpp +++ b/include/boost/fiber/mutex.hpp @@ -9,6 +9,7 @@ #include +#include #include #include #include @@ -16,6 +17,7 @@ #include #include #include +#include #ifdef BOOST_HAS_ABI_HEADERS # include BOOST_ABI_PREFIX @@ -39,7 +41,8 @@ private: }; detail::fiber_base::id owner_; - volatile state_t state_; + atomic< state_t > state_; + detail::spinlock splk_; std::deque< detail::notify::ptr_t > waiting_; diff --git a/include/boost/fiber/recursive_mutex.hpp b/include/boost/fiber/recursive_mutex.hpp index 1f0c55b9..711722ab 100644 --- a/include/boost/fiber/recursive_mutex.hpp +++ b/include/boost/fiber/recursive_mutex.hpp @@ -12,6 +12,7 @@ #include #include +#include #include #include #include @@ -19,6 +20,7 @@ #include #include #include +#include #ifdef BOOST_HAS_ABI_HEADERS # include BOOST_ABI_PREFIX @@ -42,8 +44,9 @@ private: }; detail::fiber_base::id owner_; - std::size_t count_; - volatile state_t state_; + atomic< std::size_t > count_; + atomic< state_t > state_; + detail::spinlock splk_; std::deque< detail::notify::ptr_t > waiting_; diff --git a/include/boost/fiber/recursive_timed_mutex.hpp b/include/boost/fiber/recursive_timed_mutex.hpp index f4b6d881..05235f5c 100644 --- a/include/boost/fiber/recursive_timed_mutex.hpp +++ b/include/boost/fiber/recursive_timed_mutex.hpp @@ -12,12 +12,14 @@ #include #include +#include #include #include #include #include #include +#include #include #ifdef BOOST_HAS_ABI_HEADERS @@ -42,8 +44,9 @@ private: }; detail::fiber_base::id owner_; - std::size_t count_; - volatile state_t state_; + atomic< std::size_t > count_; + atomic< state_t > state_; + detail::spinlock splk_; std::deque< detail::notify::ptr_t > waiting_; diff --git a/include/boost/fiber/timed_mutex.hpp b/include/boost/fiber/timed_mutex.hpp index fd88751c..82642bd0 100644 --- a/include/boost/fiber/timed_mutex.hpp +++ b/include/boost/fiber/timed_mutex.hpp @@ -9,6 +9,7 @@ #include +#include #include #include #include @@ -16,6 +17,7 @@ #include #include #include +#include #ifdef BOOST_HAS_ABI_HEADERS # include BOOST_ABI_PREFIX @@ -39,7 +41,8 @@ private: }; detail::fiber_base::id owner_; - volatile state_t state_; + atomic< state_t > state_; + detail::spinlock splk_; std::deque< detail::notify::ptr_t > waiting_; diff --git a/src/mutex.cpp b/src/mutex.cpp index 5e247acd..8534ea04 100644 --- a/src/mutex.cpp +++ b/src/mutex.cpp @@ -25,6 +25,7 @@ namespace fibers { mutex::mutex() : owner_(), state_( UNLOCKED), + splk_(), waiting_() {} @@ -37,14 +38,19 @@ mutex::~mutex() void mutex::lock() { - while ( LOCKED == state_) + state_t expected = UNLOCKED; + while ( ! state_.compare_exchange_strong( expected, LOCKED) ) { + expected = UNLOCKED; detail::notify::ptr_t n( detail::scheduler::instance()->active() ); if ( n) { + unique_lock< detail::spinlock > lk( splk_); // store this fiber in order to be notified later waiting_.push_back( n); + splk_.unlock(); + // TODO: prevent notification (set_ready()) of fiber before set to waiting-state // suspend this fiber detail::scheduler::instance()->wait(); } @@ -54,8 +60,10 @@ mutex::lock() detail::main_notifier mn; n = detail::main_notifier::make_pointer( mn); + unique_lock< detail::spinlock > lk( splk_); // store this fiber in order to be notified later waiting_.push_back( n); + splk_.unlock(); // wait until main-fiber gets notified while ( ! n->is_ready() ) @@ -65,23 +73,23 @@ mutex::lock() } } } - BOOST_ASSERT( ! owner_); - state_ = LOCKED; + BOOST_ASSERT( ! owner_); owner_ = this_fiber::get_id(); } bool mutex::try_lock() { - if ( LOCKED == state_) { + state_t expected = UNLOCKED; + if ( ! state_.compare_exchange_strong( expected, LOCKED) ) { // let other fiber release the lock detail::scheduler::instance()->yield(); return false; } - state_ = LOCKED; owner_ = this_fiber::get_id(); + return true; } @@ -93,13 +101,15 @@ mutex::unlock() detail::notify::ptr_t n; + unique_lock< detail::spinlock > lk( splk_); if ( ! waiting_.empty() ) { n.swap( waiting_.front() ); waiting_.pop_front(); } + splk_.unlock(); - state_ = UNLOCKED; owner_ = detail::fiber_base::id(); + state_ = UNLOCKED; if ( n) n->set_ready(); diff --git a/src/recursive_mutex.cpp b/src/recursive_mutex.cpp index a3677166..bcc6359e 100644 --- a/src/recursive_mutex.cpp +++ b/src/recursive_mutex.cpp @@ -26,6 +26,7 @@ recursive_mutex::recursive_mutex() : owner_(), count_( 0), state_( UNLOCKED), + splk_(), waiting_() {} @@ -45,14 +46,19 @@ recursive_mutex::lock() return; } - while ( LOCKED == state_) + state_t expected = UNLOCKED; + while ( ! state_.compare_exchange_strong( expected, LOCKED) ) { + expected = UNLOCKED; detail::notify::ptr_t n( detail::scheduler::instance()->active() ); if ( n) { + unique_lock< detail::spinlock > lk( splk_); // store this fiber in order to be notified later waiting_.push_back( n); + splk_.unlock(); + // TODO: prevent notification (set_ready()) of fiber before set to waiting-state // suspend this fiber detail::scheduler::instance()->wait(); } @@ -62,8 +68,10 @@ recursive_mutex::lock() detail::main_notifier mn; n = detail::main_notifier::make_pointer( mn); + unique_lock< detail::spinlock > lk( splk_); // store this fiber in order to be notified later waiting_.push_back( n); + splk_.unlock(); // wait until main-fiber gets notified while ( ! n->is_ready() ) @@ -73,10 +81,10 @@ recursive_mutex::lock() } } } + BOOST_ASSERT( ! owner_); BOOST_ASSERT( 0 == count_); - state_ = LOCKED; owner_ = this_fiber::get_id(); ++count_; } @@ -84,24 +92,23 @@ recursive_mutex::lock() bool recursive_mutex::try_lock() { - if ( LOCKED == state_) + if ( LOCKED == state_ && this_fiber::get_id() == owner_) { - if ( this_fiber::get_id() == owner_) - { - ++count_; - return true; - } - else - { - // let other fiber release the lock - detail::scheduler::instance()->yield(); - return false; - } + ++count_; + return true; + } + + state_t expected = UNLOCKED; + if ( ! state_.compare_exchange_strong( expected, LOCKED) ) + { + // let other fiber release the lock + detail::scheduler::instance()->yield(); + return false; } - state_ = LOCKED; owner_ = this_fiber::get_id(); ++count_; + return true; } @@ -115,13 +122,15 @@ recursive_mutex::unlock() { detail::notify::ptr_t n; + unique_lock< detail::spinlock > lk( splk_); if ( ! waiting_.empty() ) { n.swap( waiting_.front() ); waiting_.pop_front(); } + splk_.unlock(); - state_ = UNLOCKED; owner_ = detail::fiber_base::id(); + state_ = UNLOCKED; if ( n) n->set_ready(); diff --git a/src/recursive_timed_mutex.cpp b/src/recursive_timed_mutex.cpp index b8d68f04..a9e834df 100644 --- a/src/recursive_timed_mutex.cpp +++ b/src/recursive_timed_mutex.cpp @@ -26,6 +26,7 @@ recursive_timed_mutex::recursive_timed_mutex() : owner_(), count_( 0), state_( UNLOCKED), + splk_(), waiting_() {} @@ -45,13 +46,17 @@ recursive_timed_mutex::lock() return; } - while ( LOCKED == state_) + state_t expected = UNLOCKED; + while ( ! state_.compare_exchange_strong( expected, LOCKED) ) { + expected = UNLOCKED; detail::notify::ptr_t n( detail::scheduler::instance()->active() ); if ( n) { + unique_lock< detail::spinlock > lk( splk_); // store this fiber in order to be notified later waiting_.push_back( n); + splk_.unlock(); // suspend this fiber detail::scheduler::instance()->wait(); @@ -62,8 +67,10 @@ recursive_timed_mutex::lock() detail::main_notifier mn; n = detail::main_notifier::make_pointer( mn); + unique_lock< detail::spinlock > lk( splk_); // store this fiber in order to be notified later waiting_.push_back( n); + splk_.unlock(); // wait until main-fiber gets notified while ( ! n->is_ready() ) @@ -73,10 +80,10 @@ recursive_timed_mutex::lock() } } } + BOOST_ASSERT( ! owner_); BOOST_ASSERT( 0 == count_); - state_ = LOCKED; owner_ = this_fiber::get_id(); ++count_; } @@ -84,24 +91,23 @@ recursive_timed_mutex::lock() bool recursive_timed_mutex::try_lock() { - if ( LOCKED == state_) + if ( LOCKED == state_ && this_fiber::get_id() == owner_) { - if ( this_fiber::get_id() == owner_) - { - ++count_; - return true; - } - else - { - // let other fiber release the lock - detail::scheduler::instance()->yield(); - return false; - } + ++count_; + return true; + } + + state_t expected = UNLOCKED; + if ( ! state_.compare_exchange_strong( expected, LOCKED) ) + { + // let other fiber release the lock + detail::scheduler::instance()->yield(); + return false; } - state_ = LOCKED; owner_ = this_fiber::get_id(); ++count_; + return true; } @@ -114,21 +120,29 @@ recursive_timed_mutex::try_lock_until( clock_type::time_point const& timeout_tim return true; } - while ( LOCKED == state_ && clock_type::now() < timeout_time) + state_t expected = UNLOCKED; + while ( clock_type::now() < timeout_time && ! state_.compare_exchange_strong( expected, LOCKED) ) { + expected = UNLOCKED; detail::notify::ptr_t n( detail::scheduler::instance()->active() ); try { if ( n) { + unique_lock< detail::spinlock > lk( splk_); // store this fiber in order to be notified later waiting_.push_back( n); + splk_.unlock(); // suspend this fiber until notified or timed-out - if ( ! detail::scheduler::instance()->wait_until( timeout_time) ) + if ( ! detail::scheduler::instance()->wait_until( timeout_time) ) { + unique_lock< detail::spinlock > lk( splk_); // remove fiber from waiting-list waiting_.erase( std::find( waiting_.begin(), waiting_.end(), n) ); + splk_.unlock(); + return false; + } } else { @@ -136,18 +150,22 @@ recursive_timed_mutex::try_lock_until( clock_type::time_point const& timeout_tim detail::main_notifier mn; n = detail::main_notifier::make_pointer( mn); + unique_lock< detail::spinlock > lk( splk_); // store this fiber in order to be notified later waiting_.push_back( n); + splk_.unlock(); // wait until main-fiber gets notified while ( ! n->is_ready() ) { if ( ! ( clock_type::now() < timeout_time) ) { + unique_lock< detail::spinlock > lk( splk_); // remove fiber from waiting-list waiting_.erase( std::find( waiting_.begin(), waiting_.end(), n) ); - break; + splk_.unlock(); + return false; } // run scheduler detail::scheduler::instance()->run(); @@ -156,19 +174,20 @@ recursive_timed_mutex::try_lock_until( clock_type::time_point const& timeout_tim } catch (...) { + unique_lock< detail::spinlock > lk( splk_); // remove fiber from waiting-list waiting_.erase( - std::find( waiting_.begin(), waiting_.end(), n) ); + std::find( waiting_.begin(), waiting_.end(), n) ); + splk_.unlock(); throw; } } - if ( LOCKED == state_) return false; + if ( LOCKED != state_) return false; BOOST_ASSERT( ! owner_); BOOST_ASSERT( 0 == count_); - state_ = LOCKED; owner_ = this_fiber::get_id(); ++count_; @@ -185,13 +204,15 @@ recursive_timed_mutex::unlock() { detail::notify::ptr_t n; + unique_lock< detail::spinlock > lk( splk_); if ( ! waiting_.empty() ) { n.swap( waiting_.front() ); waiting_.pop_front(); } + splk_.unlock(); - state_ = UNLOCKED; owner_ = detail::fiber_base::id(); + state_ = UNLOCKED; if ( n) n->set_ready(); diff --git a/src/timed_mutex.cpp b/src/timed_mutex.cpp index d8368ca3..c3f756d6 100644 --- a/src/timed_mutex.cpp +++ b/src/timed_mutex.cpp @@ -25,6 +25,7 @@ namespace fibers { timed_mutex::timed_mutex() : owner_(), state_( UNLOCKED), + splk_(), waiting_() {} @@ -37,13 +38,17 @@ timed_mutex::~timed_mutex() void timed_mutex::lock() { - while ( LOCKED == state_) + state_t expected = UNLOCKED; + while ( ! state_.compare_exchange_strong( expected, LOCKED) ) { + expected = UNLOCKED; detail::notify::ptr_t n( detail::scheduler::instance()->active() ); if ( n) { + unique_lock< detail::spinlock > lk( splk_); // store this fiber in order to be notified later waiting_.push_back( n); + splk_.unlock(); // suspend this fiber detail::scheduler::instance()->wait(); @@ -54,8 +59,10 @@ timed_mutex::lock() detail::main_notifier mn; n = detail::main_notifier::make_pointer( mn); + unique_lock< detail::spinlock > lk( splk_); // store this fiber in order to be notified later waiting_.push_back( n); + splk_.unlock(); // wait until main-fiber gets notified while ( ! n->is_ready() ) @@ -65,44 +72,53 @@ timed_mutex::lock() } } } + BOOST_ASSERT( ! owner_); - state_ = LOCKED; owner_ = this_fiber::get_id(); } bool timed_mutex::try_lock() { - if ( LOCKED == state_) { + state_t expected = UNLOCKED; + if ( ! state_.compare_exchange_strong( expected, LOCKED) ) { // let other fiber release the lock detail::scheduler::instance()->yield(); return false; } - state_ = LOCKED; owner_ = this_fiber::get_id(); + return true; } bool timed_mutex::try_lock_until( clock_type::time_point const& timeout_time) { - while ( LOCKED == state_ && clock_type::now() < timeout_time) + state_t expected = UNLOCKED; + while ( clock_type::now() < timeout_time && ! state_.compare_exchange_strong( expected, LOCKED) ) { + expected = UNLOCKED; detail::notify::ptr_t n( detail::scheduler::instance()->active() ); try { if ( n) { + unique_lock< detail::spinlock > lk( splk_); // store this fiber in order to be notified later waiting_.push_back( n); + splk_.unlock(); // suspend this fiber until notified or timed-out - if ( ! detail::scheduler::instance()->wait_until( timeout_time) ) + if ( ! detail::scheduler::instance()->wait_until( timeout_time) ) { + unique_lock< detail::spinlock > lk( splk_); // remove fiber from waiting-list waiting_.erase( std::find( waiting_.begin(), waiting_.end(), n) ); + splk_.unlock(); + return false; + } } else { @@ -110,18 +126,22 @@ timed_mutex::try_lock_until( clock_type::time_point const& timeout_time) detail::main_notifier mn; n = detail::main_notifier::make_pointer( mn); + unique_lock< detail::spinlock > lk( splk_); // store this fiber in order to be notified later waiting_.push_back( n); + splk_.unlock(); // wait until main-fiber gets notified while ( ! n->is_ready() ) { if ( ! ( clock_type::now() < timeout_time) ) { + unique_lock< detail::spinlock > lk( splk_); // remove fiber from waiting-list waiting_.erase( std::find( waiting_.begin(), waiting_.end(), n) ); - break; + splk_.unlock(); + return false; } // run scheduler detail::scheduler::instance()->run(); @@ -130,18 +150,19 @@ timed_mutex::try_lock_until( clock_type::time_point const& timeout_time) } catch (...) { + unique_lock< detail::spinlock > lk( splk_); // remove fiber from waiting-list waiting_.erase( - std::find( waiting_.begin(), waiting_.end(), n) ); + std::find( waiting_.begin(), waiting_.end(), n) ); + splk_.unlock(); throw; } } - if ( LOCKED == state_) return false; + if ( LOCKED != state_) return false; BOOST_ASSERT( ! owner_); - state_ = LOCKED; owner_ = this_fiber::get_id(); return true; @@ -155,13 +176,16 @@ timed_mutex::unlock() detail::notify::ptr_t n; - if ( ! waiting_.empty() ) { + unique_lock< detail::spinlock > lk( splk_); + if ( ! waiting_.empty() ) + { n.swap( waiting_.front() ); waiting_.pop_front(); } + splk_.unlock(); - state_ = UNLOCKED; owner_ = detail::fiber_base::id(); + state_ = UNLOCKED; if ( n) n->set_ready();