From 467ba673d348bb51d72140ade7349c3f10d2bfad Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ion=20Gazta=C3=B1aga?= Date: Tue, 29 Oct 2013 07:39:21 +0000 Subject: [PATCH] Simplified, refactored and unified (timed_)lock code based on try_lock(). There were several bugs in when handling timeout expirations. [SVN r86511] --- doc/interprocess.qbk | 5 ++ .../interprocess/detail/os_file_functions.hpp | 2 +- .../interprocess/detail/robust_emulation.hpp | 63 +------------- .../sync/detail/common_algorithms.hpp | 73 ++++++++++++++++ .../sync/detail/condition_algorithm_8a.hpp | 8 -- .../sync/detail/condition_any_algorithm.hpp | 8 -- .../boost/interprocess/sync/detail/locks.hpp | 27 ++++++ include/boost/interprocess/sync/file_lock.hpp | 84 ++---------------- .../sync/interprocess_sharable_mutex.hpp | 14 ++- .../sync/interprocess_upgradable_mutex.hpp | 24 ++---- .../sync/named_recursive_mutex.hpp | 8 +- .../interprocess/sync/named_semaphore.hpp | 8 +- .../interprocess/sync/posix/condition.hpp | 11 +-- .../boost/interprocess/sync/posix/mutex.hpp | 24 +----- .../interprocess/sync/posix/named_mutex.hpp | 8 +- .../sync/posix/ptime_to_timespec.hpp | 4 +- .../sync/posix/recursive_mutex.hpp | 22 +---- .../sync/posix/semaphore_wrapper.hpp | 16 ++-- .../interprocess/sync/shm/named_mutex.hpp | 8 +- .../sync/shm/named_recursive_mutex.hpp | 8 +- .../interprocess/sync/shm/named_semaphore.hpp | 8 +- .../sync/shm/named_upgradable_mutex.hpp | 24 +----- .../interprocess/sync/spin/condition.hpp | 10 ++- .../boost/interprocess/sync/spin/mutex.hpp | 40 +-------- .../sync/spin/recursive_mutex.hpp | 5 +- .../interprocess/sync/spin/semaphore.hpp | 33 ++----- .../sync/windows/named_condition_any.hpp | 2 +- .../sync/windows/winapi_mutex_wrapper.hpp | 51 ++--------- .../sync/windows/winapi_semaphore_wrapper.hpp | 48 +---------- .../sync/windows/winapi_wrapper_common.hpp | 86 +++++++++++++++++++ test/file_lock_test.cpp | 4 +- test/util.hpp | 2 +- 32 files changed, 276 insertions(+), 462 deletions(-) create mode 100644 include/boost/interprocess/sync/detail/common_algorithms.hpp create mode 100644 include/boost/interprocess/sync/windows/winapi_wrapper_common.hpp diff --git a/doc/interprocess.qbk b/doc/interprocess.qbk index ff60240..5e8827f 100644 --- a/doc/interprocess.qbk +++ b/doc/interprocess.qbk @@ -6717,11 +6717,16 @@ thank them: * Fixed bugs: * [@https://svn.boost.org/trac/boost/ticket/9221 #9221 (['"message_queue deadlock on linux"])]. * [@https://svn.boost.org/trac/boost/ticket/9226 #9226 (['"On some computers, Common Appdata is empty in registry, so boost interprocess cannot work"])]. + * [@https://svn.boost.org/trac/boost/ticket/9285 #9285 (['"CreateMutex returns NULL if fails"])]. + * [@https://svn.boost.org/trac/boost/ticket/9288 #9288 (['"timed_wait does not check if it has expired"])]. * [*ABI breaking]: [@https://svn.boost.org/trac/boost/ticket/9221 #9221] showed that `BOOST_INTERPROCESS_MSG_QUEUE_CIRCULAR_INDEX` option of message queue, was completely broken so an ABI break was necessary to have a working implementation. +* Simplified, refactored and unified (timed_)lock code based on try_lock(). + There were several bugs in when handling timeout expirations. + [endsect] [section:release_notes_boost_1_55_00 Boost 1.55 Release] diff --git a/include/boost/interprocess/detail/os_file_functions.hpp b/include/boost/interprocess/detail/os_file_functions.hpp index cd4aeb4..7b7d9b6 100644 --- a/include/boost/interprocess/detail/os_file_functions.hpp +++ b/include/boost/interprocess/detail/os_file_functions.hpp @@ -136,7 +136,7 @@ inline bool truncate_file (file_handle_t hnd, std::size_t size) typedef boost::make_unsigned::type uoffset_t; const uoffset_t max_filesize = uoffset_t((std::numeric_limits::max)()); //Avoid unused variable warnings in 32 bit systems - if(size > max_filesize){ + if(uoffset_t(size) > max_filesize){ winapi::set_last_error(winapi::error_file_too_large); return false; } diff --git a/include/boost/interprocess/detail/robust_emulation.hpp b/include/boost/interprocess/detail/robust_emulation.hpp index 292d681..59bfc29 100644 --- a/include/boost/interprocess/detail/robust_emulation.hpp +++ b/include/boost/interprocess/detail/robust_emulation.hpp @@ -25,6 +25,7 @@ #include #include #include +#include #include namespace boost{ @@ -215,38 +216,7 @@ inline robust_spin_mutex::robust_spin_mutex() template inline void robust_spin_mutex::lock() -{ - //If the mutex is broken (recovery didn't call consistent()), - //then throw an exception - if(atomic_read32(&this->state) == broken_state){ - throw interprocess_exception(lock_error, "Broken id"); - } - - //This function provokes intermodule_singleton instantiation - if(!this->lock_own_unique_file()){ - throw interprocess_exception(lock_error, "Broken id"); - } - - //Now the logic. Try to lock, if successful mark the owner - //if it fails, start recovery logic - spin_wait swait; - while(1){ - if (mtx.try_lock()){ - atomic_write32(&this->owner, get_current_process_id()); - break; - } - else{ - //Do the dead owner checking each spin_threshold lock tries - swait.yield(); - if(0 == (swait.count() & 255u)){ - //Check if owner dead and take ownership if possible - if(this->robust_check()){ - break; - } - } - } - } -} +{ try_based_lock(*this); } template inline bool robust_spin_mutex::try_lock() @@ -277,34 +247,7 @@ inline bool robust_spin_mutex::try_lock() template inline bool robust_spin_mutex::timed_lock (const boost::posix_time::ptime &abs_time) -{ - //Same as lock() but with an additional timeout - if(abs_time == boost::posix_time::pos_infin){ - this->lock(); - return true; - } - //Obtain current count and target time - boost::posix_time::ptime now = microsec_clock::universal_time(); - - if(now >= abs_time) - return this->try_lock(); - - spin_wait swait; - do{ - if(this->try_lock()){ - break; - } - now = microsec_clock::universal_time(); - - if(now >= abs_time){ - return this->try_lock(); - } - // relinquish current time slice - swait.yield(); - }while (true); - - return true; -} +{ return try_based_timed_lock(*this, abs_time); } template inline void robust_spin_mutex::owner_to_filename(boost::uint32_t own, std::string &s) diff --git a/include/boost/interprocess/sync/detail/common_algorithms.hpp b/include/boost/interprocess/sync/detail/common_algorithms.hpp new file mode 100644 index 0000000..6d26db8 --- /dev/null +++ b/include/boost/interprocess/sync/detail/common_algorithms.hpp @@ -0,0 +1,73 @@ +////////////////////////////////////////////////////////////////////////////// +// +// (C) Copyright Ion Gaztanaga 2012-2013. Distributed under the Boost +// Software License, Version 1.0. (See accompanying file +// LICENSE_1_0.txt or copy at http://www.boost.org/LICENSE_1_0.txt) +// +// See http://www.boost.org/libs/interprocess for documentation. +// +////////////////////////////////////////////////////////////////////////////// + +#ifndef BOOST_INTERPROCESS_SYNC_DETAIL_COMMON_ALGORITHMS_HPP +#define BOOST_INTERPROCESS_SYNC_DETAIL_COMMON_ALGORITHMS_HPP + +#include +#include + +#include + +namespace boost { +namespace interprocess { +namespace ipcdetail { + +template +bool try_based_timed_lock(MutexType &m, const boost::posix_time::ptime &abs_time) +{ + //Same as lock() + if(abs_time == boost::posix_time::pos_infin){ + m.lock(); + return true; + } + //Always try to lock to achieve POSIX guarantees: + // "Under no circumstance shall the function fail with a timeout if the mutex + // can be locked immediately. The validity of the abs_timeout parameter need not + // be checked if the mutex can be locked immediately." + else if(m.try_lock()){ + return true; + } + else{ + spin_wait swait; + while(microsec_clock::universal_time() < abs_time){ + if(m.try_lock()){ + return true; + } + swait.yield(); + } + return false; + } +} + +template +void try_based_lock(MutexType &m) +{ + if(!m.try_lock()){ + spin_wait swait; + do{ + if(m.try_lock()){ + break; + } + else{ + swait.yield(); + } + } + while(1); + } +} + +} //namespace ipcdetail +} //namespace interprocess +} //namespace boost + +#include + +#endif //BOOST_INTERPROCESS_SYNC_DETAIL_COMMON_ALGORITHMS_HPP diff --git a/include/boost/interprocess/sync/detail/condition_algorithm_8a.hpp b/include/boost/interprocess/sync/detail/condition_algorithm_8a.hpp index 89c0c22..5d7e7fd 100644 --- a/include/boost/interprocess/sync/detail/condition_algorithm_8a.hpp +++ b/include/boost/interprocess/sync/detail/condition_algorithm_8a.hpp @@ -360,10 +360,6 @@ class condition_8a_wrapper template bool timed_wait(L& lock, const boost::posix_time::ptime &abs_time) { - if(abs_time == boost::posix_time::pos_infin){ - this->wait(lock); - return true; - } if (!lock) throw lock_exception(); return algo_type::wait(m_data, lock, true, abs_time); @@ -372,10 +368,6 @@ class condition_8a_wrapper template bool timed_wait(L& lock, const boost::posix_time::ptime &abs_time, Pr pred) { - if(abs_time == boost::posix_time::pos_infin){ - this->wait(lock, pred); - return true; - } if (!lock) throw lock_exception(); while (!pred()){ diff --git a/include/boost/interprocess/sync/detail/condition_any_algorithm.hpp b/include/boost/interprocess/sync/detail/condition_any_algorithm.hpp index a276f82..b0371b8 100644 --- a/include/boost/interprocess/sync/detail/condition_any_algorithm.hpp +++ b/include/boost/interprocess/sync/detail/condition_any_algorithm.hpp @@ -189,10 +189,6 @@ class condition_any_wrapper template bool timed_wait(L& lock, const boost::posix_time::ptime &abs_time) { - if(abs_time == boost::posix_time::pos_infin){ - this->wait(lock); - return true; - } if (!lock) throw lock_exception(); return algo_type::wait(m_data, lock, true, abs_time); @@ -201,10 +197,6 @@ class condition_any_wrapper template bool timed_wait(L& lock, const boost::posix_time::ptime &abs_time, Pr pred) { - if(abs_time == boost::posix_time::pos_infin){ - this->wait(lock, pred); - return true; - } if (!lock) throw lock_exception(); while (!pred()){ diff --git a/include/boost/interprocess/sync/detail/locks.hpp b/include/boost/interprocess/sync/detail/locks.hpp index fbb003a..de30d41 100644 --- a/include/boost/interprocess/sync/detail/locks.hpp +++ b/include/boost/interprocess/sync/detail/locks.hpp @@ -57,6 +57,33 @@ class lock_inverter void unlock() { l_.lock(); } }; +template +class lock_to_sharable +{ + Lock &l_; + + public: + explicit lock_to_sharable(Lock &l) + : l_(l) + {} + void lock() { l_.lock_sharable(); } + bool try_lock(){ return l_.try_lock_sharable(); } + void unlock() { l_.unlock_sharable(); } +}; + +template +class lock_to_wait +{ + Lock &l_; + + public: + explicit lock_to_wait(Lock &l) + : l_(l) + {} + void lock() { l_.wait(); } + bool try_lock(){ return l_.try_wait(); } +}; + } //namespace ipcdetail } //namespace interprocess } //namespace boost diff --git a/include/boost/interprocess/sync/file_lock.hpp b/include/boost/interprocess/sync/file_lock.hpp index 64a7b82..d551573 100644 --- a/include/boost/interprocess/sync/file_lock.hpp +++ b/include/boost/interprocess/sync/file_lock.hpp @@ -21,7 +21,8 @@ #include #include #include -#include +#include +#include #include //!\file @@ -142,62 +143,6 @@ class file_lock private: file_handle_t m_file_hnd; - bool timed_acquire_file_lock - (file_handle_t hnd, bool &acquired, const boost::posix_time::ptime &abs_time) - { - //Obtain current count and target time - boost::posix_time::ptime now = microsec_clock::universal_time(); - using namespace boost::detail; - - if(now >= abs_time) return false; - spin_wait swait; - do{ - if(!ipcdetail::try_acquire_file_lock(hnd, acquired)) - return false; - - if(acquired) - return true; - else{ - now = microsec_clock::universal_time(); - - if(now >= abs_time){ - acquired = false; - return true; - } - // relinquish current time slice - swait.yield(); - } - }while (true); - } - - bool timed_acquire_file_lock_sharable - (file_handle_t hnd, bool &acquired, const boost::posix_time::ptime &abs_time) - { - //Obtain current count and target time - boost::posix_time::ptime now = microsec_clock::universal_time(); - using namespace boost::detail; - - if(now >= abs_time) return false; - - spin_wait swait; - do{ - if(!ipcdetail::try_acquire_file_lock_sharable(hnd, acquired)) - return false; - - if(acquired) - return true; - else{ - now = microsec_clock::universal_time(); - - if(now >= abs_time){ - acquired = false; - return true; - } - // relinquish current time slice - swait.yield(); - } - }while (true); - } /// @endcond }; @@ -238,18 +183,7 @@ inline bool file_lock::try_lock() } inline bool file_lock::timed_lock(const boost::posix_time::ptime &abs_time) -{ - if(abs_time == boost::posix_time::pos_infin){ - this->lock(); - return true; - } - bool result; - if(!this->timed_acquire_file_lock(m_file_hnd, result, abs_time)){ - error_info err(system_error_code()); - throw interprocess_exception(err); - } - return result; -} +{ return ipcdetail::try_based_timed_lock(*this, abs_time); } inline void file_lock::unlock() { @@ -279,16 +213,8 @@ inline bool file_lock::try_lock_sharable() inline bool file_lock::timed_lock_sharable(const boost::posix_time::ptime &abs_time) { - if(abs_time == boost::posix_time::pos_infin){ - this->lock_sharable(); - return true; - } - bool result; - if(!this->timed_acquire_file_lock_sharable(m_file_hnd, result, abs_time)){ - error_info err(system_error_code()); - throw interprocess_exception(err); - } - return result; + ipcdetail::lock_to_sharable lsh(*this); + return ipcdetail::try_based_timed_lock(lsh, abs_time); } inline void file_lock::unlock_sharable() diff --git a/include/boost/interprocess/sync/interprocess_sharable_mutex.hpp b/include/boost/interprocess/sync/interprocess_sharable_mutex.hpp index 8eddca5..00816ad 100644 --- a/include/boost/interprocess/sync/interprocess_sharable_mutex.hpp +++ b/include/boost/interprocess/sync/interprocess_sharable_mutex.hpp @@ -213,16 +213,14 @@ inline bool interprocess_sharable_mutex::try_lock() inline bool interprocess_sharable_mutex::timed_lock (const boost::posix_time::ptime &abs_time) { - if(abs_time == boost::posix_time::pos_infin){ - this->lock(); - return true; - } scoped_lock_t lck(m_mut, abs_time); if(!lck.owns()) return false; //The exclusive lock must block in the first gate //if an exclusive lock has been acquired while (this->m_ctrl.exclusive_in){ + //Mutexes and condvars handle just fine infinite abs_times + //so avoid checking it here if(!this->m_first_gate.timed_wait(lck, abs_time)){ if(this->m_ctrl.exclusive_in){ return false; @@ -239,6 +237,8 @@ inline bool interprocess_sharable_mutex::timed_lock //Now wait until all readers are gone while (this->m_ctrl.num_shared){ + //Mutexes and condvars handle just fine infinite abs_times + //so avoid checking it here if(!this->m_second_gate.timed_wait(lck, abs_time)){ if(this->m_ctrl.num_shared){ return false; @@ -296,10 +296,6 @@ inline bool interprocess_sharable_mutex::try_lock_sharable() inline bool interprocess_sharable_mutex::timed_lock_sharable (const boost::posix_time::ptime &abs_time) { - if(abs_time == boost::posix_time::pos_infin){ - this->lock_sharable(); - return true; - } scoped_lock_t lck(m_mut, abs_time); if(!lck.owns()) return false; @@ -308,6 +304,8 @@ inline bool interprocess_sharable_mutex::timed_lock_sharable //or there are too many sharable locks while (this->m_ctrl.exclusive_in || this->m_ctrl.num_shared == constants::max_readers){ + //Mutexes and condvars handle just fine infinite abs_times + //so avoid checking it here if(!this->m_first_gate.timed_wait(lck, abs_time)){ if(this->m_ctrl.exclusive_in || this->m_ctrl.num_shared == constants::max_readers){ diff --git a/include/boost/interprocess/sync/interprocess_upgradable_mutex.hpp b/include/boost/interprocess/sync/interprocess_upgradable_mutex.hpp index 3700c09..2719212 100644 --- a/include/boost/interprocess/sync/interprocess_upgradable_mutex.hpp +++ b/include/boost/interprocess/sync/interprocess_upgradable_mutex.hpp @@ -325,10 +325,8 @@ inline bool interprocess_upgradable_mutex::try_lock() inline bool interprocess_upgradable_mutex::timed_lock (const boost::posix_time::ptime &abs_time) { - if(abs_time == boost::posix_time::pos_infin){ - this->lock(); - return true; - } + //Mutexes and condvars handle just fine infinite abs_times + //so avoid checking it here scoped_lock_t lck(m_mut, abs_time); if(!lck.owns()) return false; @@ -413,10 +411,8 @@ inline bool interprocess_upgradable_mutex::try_lock_upgradable() inline bool interprocess_upgradable_mutex::timed_lock_upgradable (const boost::posix_time::ptime &abs_time) { - if(abs_time == boost::posix_time::pos_infin){ - this->lock_upgradable(); - return true; - } + //Mutexes and condvars handle just fine infinite abs_times + //so avoid checking it here scoped_lock_t lck(m_mut, abs_time); if(!lck.owns()) return false; @@ -492,10 +488,8 @@ inline bool interprocess_upgradable_mutex::try_lock_sharable() inline bool interprocess_upgradable_mutex::timed_lock_sharable (const boost::posix_time::ptime &abs_time) { - if(abs_time == boost::posix_time::pos_infin){ - this->lock_sharable(); - return true; - } + //Mutexes and condvars handle just fine infinite abs_times + //so avoid checking it here scoped_lock_t lck(m_mut, abs_time); if(!lck.owns()) return false; @@ -607,10 +601,8 @@ inline bool interprocess_upgradable_mutex::try_unlock_upgradable_and_lock() inline bool interprocess_upgradable_mutex::timed_unlock_upgradable_and_lock (const boost::posix_time::ptime &abs_time) { - if(abs_time == boost::posix_time::pos_infin){ - this->unlock_upgradable_and_lock(); - return true; - } + //Mutexes and condvars handle just fine infinite abs_times + //so avoid checking it here scoped_lock_t lck(m_mut, abs_time); if(!lck.owns()) return false; diff --git a/include/boost/interprocess/sync/named_recursive_mutex.hpp b/include/boost/interprocess/sync/named_recursive_mutex.hpp index 01e836b..1e0d0d4 100644 --- a/include/boost/interprocess/sync/named_recursive_mutex.hpp +++ b/include/boost/interprocess/sync/named_recursive_mutex.hpp @@ -143,13 +143,7 @@ inline bool named_recursive_mutex::try_lock() { return m_mut.try_lock(); } inline bool named_recursive_mutex::timed_lock(const boost::posix_time::ptime &abs_time) -{ - if(abs_time == boost::posix_time::pos_infin){ - this->lock(); - return true; - } - return m_mut.timed_lock(abs_time); -} +{ return m_mut.timed_lock(abs_time); } inline bool named_recursive_mutex::remove(const char *name) { return impl_t::remove(name); } diff --git a/include/boost/interprocess/sync/named_semaphore.hpp b/include/boost/interprocess/sync/named_semaphore.hpp index a7d9055..05469a0 100644 --- a/include/boost/interprocess/sync/named_semaphore.hpp +++ b/include/boost/interprocess/sync/named_semaphore.hpp @@ -153,13 +153,7 @@ inline bool named_semaphore::try_wait() { return m_sem.try_wait(); } inline bool named_semaphore::timed_wait(const boost::posix_time::ptime &abs_time) -{ - if(abs_time == boost::posix_time::pos_infin){ - this->wait(); - return true; - } - return m_sem.timed_wait(abs_time); -} +{ return m_sem.timed_wait(abs_time); } inline bool named_semaphore::remove(const char *name) { return impl_t::remove(name); } diff --git a/include/boost/interprocess/sync/posix/condition.hpp b/include/boost/interprocess/sync/posix/condition.hpp index 9f3264c..e09fe4a 100644 --- a/include/boost/interprocess/sync/posix/condition.hpp +++ b/include/boost/interprocess/sync/posix/condition.hpp @@ -83,12 +83,13 @@ class posix_condition template bool timed_wait(L& lock, const boost::posix_time::ptime &abs_time) { + if (!lock) + throw lock_exception(); + //Posix does not support infinity absolute time so handle it here if(abs_time == boost::posix_time::pos_infin){ this->wait(lock); return true; } - if (!lock) - throw lock_exception(); return this->do_timed_wait(abs_time, *lock.mutex()); } @@ -98,17 +99,17 @@ class posix_condition template bool timed_wait(L& lock, const boost::posix_time::ptime &abs_time, Pr pred) { + if (!lock) + throw lock_exception(); + //Posix does not support infinity absolute time so handle it here if(abs_time == boost::posix_time::pos_infin){ this->wait(lock, pred); return true; } - if (!lock) - throw lock_exception(); while (!pred()){ if (!this->do_timed_wait(abs_time, *lock.mutex())) return pred(); } - return true; } diff --git a/include/boost/interprocess/sync/posix/mutex.hpp b/include/boost/interprocess/sync/posix/mutex.hpp index de44341..4cd4207 100644 --- a/include/boost/interprocess/sync/posix/mutex.hpp +++ b/include/boost/interprocess/sync/posix/mutex.hpp @@ -44,7 +44,7 @@ #ifndef BOOST_INTERPROCESS_POSIX_TIMEOUTS # include -# include +# include #endif #include @@ -103,12 +103,12 @@ inline bool posix_mutex::try_lock() inline bool posix_mutex::timed_lock(const boost::posix_time::ptime &abs_time) { + #ifdef BOOST_INTERPROCESS_POSIX_TIMEOUTS + //Posix does not support infinity absolute time so handle it here if(abs_time == boost::posix_time::pos_infin){ this->lock(); return true; } - #ifdef BOOST_INTERPROCESS_POSIX_TIMEOUTS - timespec ts = ptime_to_timespec(abs_time); int res = pthread_mutex_timedlock(&m_mut, &ts); if (res != 0 && res != ETIMEDOUT) @@ -117,23 +117,7 @@ inline bool posix_mutex::timed_lock(const boost::posix_time::ptime &abs_time) #else //BOOST_INTERPROCESS_POSIX_TIMEOUTS - //Obtain current count and target time - boost::posix_time::ptime now = microsec_clock::universal_time(); - - spin_wait swait; - do{ - if(this->try_lock()){ - break; - } - now = microsec_clock::universal_time(); - - if(now >= abs_time){ - return false; - } - // relinquish current time slice - swait.yield(); - }while (true); - return true; + return ipcdetail::try_based_timed_lock(*this, abs_time); #endif //BOOST_INTERPROCESS_POSIX_TIMEOUTS } diff --git a/include/boost/interprocess/sync/posix/named_mutex.hpp b/include/boost/interprocess/sync/posix/named_mutex.hpp index 4c2f8e1..b2880b9 100644 --- a/include/boost/interprocess/sync/posix/named_mutex.hpp +++ b/include/boost/interprocess/sync/posix/named_mutex.hpp @@ -94,13 +94,7 @@ inline bool posix_named_mutex::try_lock() { return m_sem.try_wait(); } inline bool posix_named_mutex::timed_lock(const boost::posix_time::ptime &abs_time) -{ - if(abs_time == boost::posix_time::pos_infin){ - this->lock(); - return true; - } - return m_sem.timed_wait(abs_time); -} +{ return m_sem.timed_wait(abs_time); } inline bool posix_named_mutex::remove(const char *name) { return posix_named_semaphore::remove(name); } diff --git a/include/boost/interprocess/sync/posix/ptime_to_timespec.hpp b/include/boost/interprocess/sync/posix/ptime_to_timespec.hpp index 3e20dc7..79eb7dc 100644 --- a/include/boost/interprocess/sync/posix/ptime_to_timespec.hpp +++ b/include/boost/interprocess/sync/posix/ptime_to_timespec.hpp @@ -22,7 +22,9 @@ namespace ipcdetail { inline timespec ptime_to_timespec (const boost::posix_time::ptime &tm) { const boost::posix_time::ptime epoch(boost::gregorian::date(1970,1,1)); - boost::posix_time::time_duration duration (tm - epoch); + //Avoid negative absolute times + boost::posix_time::time_duration duration = (tm <= epoch) ? boost::posix_time::time_duration(epoch - epoch) + : boost::posix_time::time_duration(tm - epoch); timespec ts; ts.tv_sec = duration.total_seconds(); ts.tv_nsec = duration.total_nanoseconds() % 1000000000; diff --git a/include/boost/interprocess/sync/posix/recursive_mutex.hpp b/include/boost/interprocess/sync/posix/recursive_mutex.hpp index ce2ad68..d5bda34 100644 --- a/include/boost/interprocess/sync/posix/recursive_mutex.hpp +++ b/include/boost/interprocess/sync/posix/recursive_mutex.hpp @@ -38,7 +38,7 @@ #include #ifndef BOOST_INTERPROCESS_POSIX_TIMEOUTS # include -# include +# include #endif #include @@ -93,11 +93,12 @@ inline bool posix_recursive_mutex::try_lock() inline bool posix_recursive_mutex::timed_lock(const boost::posix_time::ptime &abs_time) { + #ifdef BOOST_INTERPROCESS_POSIX_TIMEOUTS + //Posix does not support infinity absolute time so handle it here if(abs_time == boost::posix_time::pos_infin){ this->lock(); return true; } - #ifdef BOOST_INTERPROCESS_POSIX_TIMEOUTS timespec ts = ptime_to_timespec(abs_time); int res = pthread_mutex_timedlock(&m_mut, &ts); @@ -107,22 +108,7 @@ inline bool posix_recursive_mutex::timed_lock(const boost::posix_time::ptime &ab #else //BOOST_INTERPROCESS_POSIX_TIMEOUTS - //Obtain current count and target time - boost::posix_time::ptime now = microsec_clock::universal_time(); - spin_wait swait; - do{ - if(this->try_lock()){ - break; - } - now = microsec_clock::universal_time(); - - if(now >= abs_time){ - return false; - } - // relinquish current time slice - swait.yield(); - }while (true); - return true; + return ipcdetail::try_based_timed_lock(*this, abs_time); #endif //BOOST_INTERPROCESS_POSIX_TIMEOUTS } diff --git a/include/boost/interprocess/sync/posix/semaphore_wrapper.hpp b/include/boost/interprocess/sync/posix/semaphore_wrapper.hpp index 98cb128..10ea52c 100644 --- a/include/boost/interprocess/sync/posix/semaphore_wrapper.hpp +++ b/include/boost/interprocess/sync/posix/semaphore_wrapper.hpp @@ -174,11 +174,13 @@ inline bool semaphore_try_wait(sem_t *handle) inline bool semaphore_timed_wait(sem_t *handle, const boost::posix_time::ptime &abs_time) { + #ifdef BOOST_INTERPROCESS_POSIX_TIMEOUTS + //Posix does not support infinity absolute time so handle it here if(abs_time == boost::posix_time::pos_infin){ semaphore_wait(handle); return true; } - #ifdef BOOST_INTERPROCESS_POSIX_TIMEOUTS + timespec tspec = ptime_to_timespec(abs_time); for (;;){ int res = sem_timedwait(handle, &tspec); @@ -195,14 +197,10 @@ inline bool semaphore_timed_wait(sem_t *handle, const boost::posix_time::ptime & } return false; #else //#ifdef BOOST_INTERPROCESS_POSIX_TIMEOUTS - boost::posix_time::ptime now; - spin_wait swait; - do{ - if(semaphore_try_wait(handle)) - return true; - swait.yield(); - }while((now = microsec_clock::universal_time()) < abs_time); - return false; + + ipcdetail::lock_to_wait<> lw(*this); + return ipcdetail::try_based_timed_lock(lw, abs_time); + #endif //#ifdef BOOST_INTERPROCESS_POSIX_TIMEOUTS } diff --git a/include/boost/interprocess/sync/shm/named_mutex.hpp b/include/boost/interprocess/sync/shm/named_mutex.hpp index 006bf6b..11becee 100644 --- a/include/boost/interprocess/sync/shm/named_mutex.hpp +++ b/include/boost/interprocess/sync/shm/named_mutex.hpp @@ -161,13 +161,7 @@ inline bool shm_named_mutex::try_lock() { return this->internal_mutex().try_lock(); } inline bool shm_named_mutex::timed_lock(const boost::posix_time::ptime &abs_time) -{ - if(abs_time == boost::posix_time::pos_infin){ - this->lock(); - return true; - } - return this->internal_mutex().timed_lock(abs_time); -} +{ return this->internal_mutex().timed_lock(abs_time); } inline bool shm_named_mutex::remove(const char *name) { return shared_memory_object::remove(name); } diff --git a/include/boost/interprocess/sync/shm/named_recursive_mutex.hpp b/include/boost/interprocess/sync/shm/named_recursive_mutex.hpp index 5de82ce..9e10195 100644 --- a/include/boost/interprocess/sync/shm/named_recursive_mutex.hpp +++ b/include/boost/interprocess/sync/shm/named_recursive_mutex.hpp @@ -153,13 +153,7 @@ inline bool shm_named_recursive_mutex::try_lock() { return this->mutex()->try_lock(); } inline bool shm_named_recursive_mutex::timed_lock(const boost::posix_time::ptime &abs_time) -{ - if(abs_time == boost::posix_time::pos_infin){ - this->lock(); - return true; - } - return this->mutex()->timed_lock(abs_time); -} +{ return this->mutex()->timed_lock(abs_time); } inline bool shm_named_recursive_mutex::remove(const char *name) { return shared_memory_object::remove(name); } diff --git a/include/boost/interprocess/sync/shm/named_semaphore.hpp b/include/boost/interprocess/sync/shm/named_semaphore.hpp index ff6f903..95e7a7f 100644 --- a/include/boost/interprocess/sync/shm/named_semaphore.hpp +++ b/include/boost/interprocess/sync/shm/named_semaphore.hpp @@ -120,13 +120,7 @@ inline bool shm_named_semaphore::try_wait() { return semaphore()->try_wait(); } inline bool shm_named_semaphore::timed_wait(const boost::posix_time::ptime &abs_time) -{ - if(abs_time == boost::posix_time::pos_infin){ - this->wait(); - return true; - } - return semaphore()->timed_wait(abs_time); -} +{ return semaphore()->timed_wait(abs_time); } inline bool shm_named_semaphore::remove(const char *name) { return shared_memory_object::remove(name); } diff --git a/include/boost/interprocess/sync/shm/named_upgradable_mutex.hpp b/include/boost/interprocess/sync/shm/named_upgradable_mutex.hpp index 84027a4..74a358e 100644 --- a/include/boost/interprocess/sync/shm/named_upgradable_mutex.hpp +++ b/include/boost/interprocess/sync/shm/named_upgradable_mutex.hpp @@ -287,13 +287,7 @@ inline bool named_upgradable_mutex::try_lock() inline bool named_upgradable_mutex::timed_lock (const boost::posix_time::ptime &abs_time) -{ - if(abs_time == boost::posix_time::pos_infin){ - this->lock(); - return true; - } - return this->mutex()->timed_lock(abs_time); -} +{ return this->mutex()->timed_lock(abs_time); } inline void named_upgradable_mutex::lock_upgradable() { this->mutex()->lock_upgradable(); } @@ -306,13 +300,7 @@ inline bool named_upgradable_mutex::try_lock_upgradable() inline bool named_upgradable_mutex::timed_lock_upgradable (const boost::posix_time::ptime &abs_time) -{ - if(abs_time == boost::posix_time::pos_infin){ - this->lock_upgradable(); - return true; - } - return this->mutex()->timed_lock_upgradable(abs_time); -} +{ return this->mutex()->timed_lock_upgradable(abs_time); } inline void named_upgradable_mutex::lock_sharable() { this->mutex()->lock_sharable(); } @@ -325,13 +313,7 @@ inline bool named_upgradable_mutex::try_lock_sharable() inline bool named_upgradable_mutex::timed_lock_sharable (const boost::posix_time::ptime &abs_time) -{ - if(abs_time == boost::posix_time::pos_infin){ - this->lock_sharable(); - return true; - } - return this->mutex()->timed_lock_sharable(abs_time); -} +{ return this->mutex()->timed_lock_sharable(abs_time); } inline void named_upgradable_mutex::unlock_and_lock_upgradable() { this->mutex()->unlock_and_lock_upgradable(); } diff --git a/include/boost/interprocess/sync/spin/condition.hpp b/include/boost/interprocess/sync/spin/condition.hpp index c98c759..0d75a94 100644 --- a/include/boost/interprocess/sync/spin/condition.hpp +++ b/include/boost/interprocess/sync/spin/condition.hpp @@ -41,24 +41,26 @@ class spin_condition template bool timed_wait(L& lock, const boost::posix_time::ptime &abs_time) { + if (!lock) + throw lock_exception(); + //Handle infinity absolute time here to avoid complications in do_timed_wait if(abs_time == boost::posix_time::pos_infin){ this->wait(lock); return true; } - if (!lock) - throw lock_exception(); return this->do_timed_wait(abs_time, *lock.mutex()); } template bool timed_wait(L& lock, const boost::posix_time::ptime &abs_time, Pr pred) { + if (!lock) + throw lock_exception(); + //Handle infinity absolute time here to avoid complications in do_timed_wait if(abs_time == boost::posix_time::pos_infin){ this->wait(lock, pred); return true; } - if (!lock) - throw lock_exception(); while (!pred()){ if (!this->do_timed_wait(abs_time, *lock.mutex())) return pred(); diff --git a/include/boost/interprocess/sync/spin/mutex.hpp b/include/boost/interprocess/sync/spin/mutex.hpp index 9f9b0f7..154dc6d 100644 --- a/include/boost/interprocess/sync/spin/mutex.hpp +++ b/include/boost/interprocess/sync/spin/mutex.hpp @@ -22,7 +22,7 @@ #include #include #include -#include +#include namespace boost { namespace interprocess { @@ -60,18 +60,7 @@ inline spin_mutex::~spin_mutex() } inline void spin_mutex::lock(void) -{ - spin_wait swait; - do{ - boost::uint32_t prev_s = ipcdetail::atomic_cas32(const_cast(&m_s), 1, 0); - - if (m_s == 1 && prev_s == 0){ - break; - } - // relinquish current timeslice - swait.yield(); - }while (true); -} +{ return ipcdetail::try_based_lock(*this); } inline bool spin_mutex::try_lock(void) { @@ -80,30 +69,7 @@ inline bool spin_mutex::try_lock(void) } inline bool spin_mutex::timed_lock(const boost::posix_time::ptime &abs_time) -{ - if(abs_time == boost::posix_time::pos_infin){ - this->lock(); - return true; - } - //Obtain current count and target time - boost::posix_time::ptime now = microsec_clock::universal_time(); - - spin_wait swait; - do{ - if(this->try_lock()){ - break; - } - now = microsec_clock::universal_time(); - - if(now >= abs_time){ - return false; - } - // relinquish current time slice - swait.yield(); - }while (true); - - return true; -} +{ return ipcdetail::try_based_timed_lock(*this, abs_time); } inline void spin_mutex::unlock(void) { ipcdetail::atomic_cas32(const_cast(&m_s), 0, 1); } diff --git a/include/boost/interprocess/sync/spin/recursive_mutex.hpp b/include/boost/interprocess/sync/spin/recursive_mutex.hpp index 4170542..ce6b0d1 100644 --- a/include/boost/interprocess/sync/spin/recursive_mutex.hpp +++ b/include/boost/interprocess/sync/spin/recursive_mutex.hpp @@ -118,10 +118,6 @@ inline bool spin_recursive_mutex::try_lock() inline bool spin_recursive_mutex::timed_lock(const boost::posix_time::ptime &abs_time) { typedef ipcdetail::OS_systemwide_thread_id_t handle_t; - if(abs_time == boost::posix_time::pos_infin){ - this->lock(); - return true; - } const handle_t thr_id(ipcdetail::get_current_systemwide_thread_id()); handle_t old_id; ipcdetail::systemwide_thread_id_copy(m_nOwner, old_id); @@ -133,6 +129,7 @@ inline bool spin_recursive_mutex::timed_lock(const boost::posix_time::ptime &abs ++m_nLockCount; return true; } + //m_mutex supports abs_time so no need to check it if(m_mutex.timed_lock(abs_time)){ ipcdetail::systemwide_thread_id_copy(thr_id, m_nOwner); m_nLockCount = 1; diff --git a/include/boost/interprocess/sync/spin/semaphore.hpp b/include/boost/interprocess/sync/spin/semaphore.hpp index 7882288..94922af 100644 --- a/include/boost/interprocess/sync/spin/semaphore.hpp +++ b/include/boost/interprocess/sync/spin/semaphore.hpp @@ -20,7 +20,8 @@ #include #include #include -#include +#include +#include #include namespace boost { @@ -60,10 +61,8 @@ inline void spin_semaphore::post() inline void spin_semaphore::wait() { - spin_wait swait; - while(!ipcdetail::atomic_add_unless32(&m_count, boost::uint32_t(-1), boost::uint32_t(0))){ - swait.yield(); - } + ipcdetail::lock_to_wait lw(*this); + return ipcdetail::try_based_lock(lw); } inline bool spin_semaphore::try_wait() @@ -73,30 +72,10 @@ inline bool spin_semaphore::try_wait() inline bool spin_semaphore::timed_wait(const boost::posix_time::ptime &abs_time) { - if(abs_time == boost::posix_time::pos_infin){ - this->wait(); - return true; - } - //Obtain current count and target time - boost::posix_time::ptime now(microsec_clock::universal_time()); - - spin_wait swait; - do{ - if(this->try_wait()){ - break; - } - now = microsec_clock::universal_time(); - - if(now >= abs_time){ - return this->try_wait(); - } - // relinquish current time slice - swait.yield(); - }while (true); - return true; + ipcdetail::lock_to_wait lw(*this); + return ipcdetail::try_based_timed_lock(lw, abs_time); } - //inline int spin_semaphore::get_count() const //{ //return (int)ipcdetail::atomic_read32(&m_count); diff --git a/include/boost/interprocess/sync/windows/named_condition_any.hpp b/include/boost/interprocess/sync/windows/named_condition_any.hpp index 111b954..e5481d1 100644 --- a/include/boost/interprocess/sync/windows/named_condition_any.hpp +++ b/include/boost/interprocess/sync/windows/named_condition_any.hpp @@ -97,7 +97,7 @@ class windows_named_condition_any /// @cond private: - void windows_named_condition_any::dont_close_on_destruction() + void dont_close_on_destruction() {} friend class interprocess_tester; diff --git a/include/boost/interprocess/sync/windows/winapi_mutex_wrapper.hpp b/include/boost/interprocess/sync/windows/winapi_mutex_wrapper.hpp index 58a0fba..1b41f1b 100644 --- a/include/boost/interprocess/sync/windows/winapi_mutex_wrapper.hpp +++ b/include/boost/interprocess/sync/windows/winapi_mutex_wrapper.hpp @@ -21,6 +21,7 @@ #include #include #include +#include #include #include #include @@ -44,58 +45,16 @@ class winapi_mutex_functions {} void unlock() - { - winapi::release_mutex(m_mtx_hnd); - } + { winapi::release_mutex(m_mtx_hnd); } void lock() - { - if(winapi::wait_for_single_object(m_mtx_hnd, winapi::infinite_time) != winapi::wait_object_0){ - error_info err = system_error_code(); - throw interprocess_exception(err); - } - } + { return winapi_wrapper_wait_for_single_object(m_mtx_hnd); } bool try_lock() - { - unsigned long ret = winapi::wait_for_single_object(m_mtx_hnd, 0); - if(ret == winapi::wait_object_0){ - return true; - } - else if(ret == winapi::wait_timeout){ - return false; - } - else{ - error_info err = system_error_code(); - throw interprocess_exception(err); - } - } + { return winapi_wrapper_try_wait_for_single_object(m_mtx_hnd); } bool timed_lock(const boost::posix_time::ptime &abs_time) - { - if(abs_time == boost::posix_time::pos_infin){ - this->lock(); - return true; - } - const boost::posix_time::ptime cur_time = microsec_clock::universal_time(); - if(abs_time < cur_time){ - return false; - } - else{ - unsigned long ret = winapi::wait_for_single_object - (m_mtx_hnd, (abs_time - cur_time).total_milliseconds()); - if(ret == winapi::wait_object_0){ - return true; - } - else if(ret == winapi::wait_timeout){ - return false; - } - else{ - error_info err = system_error_code(); - throw interprocess_exception(err); - } - } - } + { return winapi_wrapper_timed_wait_for_single_object(m_mtx_hnd, abs_time); } /// @cond protected: diff --git a/include/boost/interprocess/sync/windows/winapi_semaphore_wrapper.hpp b/include/boost/interprocess/sync/windows/winapi_semaphore_wrapper.hpp index a227a22..721acf9 100644 --- a/include/boost/interprocess/sync/windows/winapi_semaphore_wrapper.hpp +++ b/include/boost/interprocess/sync/windows/winapi_semaphore_wrapper.hpp @@ -21,6 +21,7 @@ #include #include #include +#include #include #include #include @@ -50,54 +51,13 @@ class winapi_semaphore_functions } void wait() - { - if(winapi::wait_for_single_object(m_sem_hnd, winapi::infinite_time) != winapi::wait_object_0){ - error_info err = system_error_code(); - throw interprocess_exception(err); - } - } + { return winapi_wrapper_wait_for_single_object(m_sem_hnd); } bool try_wait() - { - unsigned long ret = winapi::wait_for_single_object(m_sem_hnd, 0); - if(ret == winapi::wait_object_0){ - return true; - } - else if(ret == winapi::wait_timeout){ - return false; - } - else{ - error_info err = system_error_code(); - throw interprocess_exception(err); - } - } + { return winapi_wrapper_try_wait_for_single_object(m_sem_hnd); } bool timed_wait(const boost::posix_time::ptime &abs_time) - { - if(abs_time == boost::posix_time::pos_infin){ - this->wait(); - return true; - } - - boost::posix_time::ptime cur_time = microsec_clock::universal_time(); - if(abs_time < cur_time){ - return false; - } - else{ - unsigned long ret = winapi::wait_for_single_object - (m_sem_hnd, (abs_time - cur_time).total_milliseconds()); - if(ret == winapi::wait_object_0){ - return true; - } - else if(ret == winapi::wait_timeout){ - return false; - } - else{ - error_info err = system_error_code(); - throw interprocess_exception(err); - } - } - } + { return winapi_wrapper_timed_wait_for_single_object(m_sem_hnd, abs_time); } long value() const { diff --git a/include/boost/interprocess/sync/windows/winapi_wrapper_common.hpp b/include/boost/interprocess/sync/windows/winapi_wrapper_common.hpp new file mode 100644 index 0000000..5f068a4 --- /dev/null +++ b/include/boost/interprocess/sync/windows/winapi_wrapper_common.hpp @@ -0,0 +1,86 @@ + ////////////////////////////////////////////////////////////////////////////// +// +// (C) Copyright Ion Gaztanaga 2011-2012. Distributed under the Boost +// Software License, Version 1.0. (See accompanying file +// LICENSE_1_0.txt or copy at http://www.boost.org/LICENSE_1_0.txt) +// +// See http://www.boost.org/libs/interprocess for documentation. +// +////////////////////////////////////////////////////////////////////////////// + +#ifndef BOOST_INTERPROCESS_DETAIL_WINAPI_WRAPPER_COMMON_HPP +#define BOOST_INTERPROCESS_DETAIL_WINAPI_WRAPPER_COMMON_HPP + +#if defined(_MSC_VER) +# pragma once +#endif + +#include +#include +#include +#include +#include +#include +#include + +namespace boost { +namespace interprocess { +namespace ipcdetail { + +inline void winapi_wrapper_wait_for_single_object(void *handle) +{ + if(winapi::wait_for_single_object(handle, winapi::infinite_time) != winapi::wait_object_0){ + error_info err = system_error_code(); + throw interprocess_exception(err); + } +} + +inline bool winapi_wrapper_try_wait_for_single_object(void *handle) +{ + unsigned long ret = winapi::wait_for_single_object(handle, 0); + if(ret == winapi::wait_object_0){ + return true; + } + else if(ret == winapi::wait_timeout){ + return false; + } + else{ + error_info err = system_error_code(); + throw interprocess_exception(err); + } +} + +inline bool winapi_wrapper_timed_wait_for_single_object(void *handle, const boost::posix_time::ptime &abs_time) +{ + //Windows does not support infinity abs_time so check it + if(abs_time == boost::posix_time::pos_infin){ + winapi_wrapper_wait_for_single_object(handle); + return true; + } + const boost::posix_time::ptime cur_time = microsec_clock::universal_time(); + //Windows uses relative wait times so check for negative waits + //and implement as 0 wait to allow try-semantics as POSIX mandates. + unsigned long ret = winapi::wait_for_single_object + ( handle + , (abs_time <= cur_time) ? 0u + : (abs_time - cur_time).total_milliseconds() + ); + if(ret == winapi::wait_object_0){ + return true; + } + else if(ret == winapi::wait_timeout){ + return false; + } + else{ + error_info err = system_error_code(); + throw interprocess_exception(err); + } +} + +} //namespace ipcdetail { +} //namespace interprocess { +} //namespace boost { + +#include + +#endif //BOOST_INTERPROCESS_DETAIL_WINAPI_MUTEX_WRAPPER_HPP diff --git a/test/file_lock_test.cpp b/test/file_lock_test.cpp index d8c48ab..22738de 100644 --- a/test/file_lock_test.cpp +++ b/test/file_lock_test.cpp @@ -71,8 +71,8 @@ int main () } //test::test_all_lock(); - //test::test_all_mutex(); - //test::test_all_sharable_mutex(); + //test::test_all_mutex(); + //test::test_all_sharable_mutex(); std::remove(get_filename().c_str()); return 0; diff --git a/test/util.hpp b/test/util.hpp index 1cdc8be..84203ab 100644 --- a/test/util.hpp +++ b/test/util.hpp @@ -76,7 +76,7 @@ class thread_adapter template struct data { - data(int id, int secs=0) + explicit data(int id, int secs=0) : m_id(id), m_value(-1), m_secs(secs), m_error(no_error) {} int m_id;