From eb00dacf07fb53db520ac6cd359dd45bfd735252 Mon Sep 17 00:00:00 2001 From: Andrey Semashev Date: Sun, 20 Oct 2013 13:54:27 +0000 Subject: [PATCH] Made timed_mutex incompatible with condition variable when mutex_timedwait is not available. The test had to be disabled for now because it requires condition_variable_any. [SVN r86367] --- .../condition_variable_posix.hpp | 8 +-- .../detail/events/auto_reset_event_futex.hpp | 69 +++++++------------ .../events/manual_reset_event_futex.hpp | 2 - .../sync/detail/mutexes/timed_mutex_posix.hpp | 26 ++++--- .../detail/mutexes/timed_mutex_windows.hpp | 6 +- include/boost/sync/detail/pthread.hpp | 26 +++++-- .../boost/sync/detail/pthread_mutex_locks.hpp | 8 +-- .../detail/semaphore/basic_semaphore_mach.hpp | 12 ++-- test/run/mutex_test.cpp | 2 +- test/run/utils.hpp | 8 +-- 10 files changed, 85 insertions(+), 82 deletions(-) diff --git a/include/boost/sync/detail/condition_variables/condition_variable_posix.hpp b/include/boost/sync/detail/condition_variables/condition_variable_posix.hpp index 9bb167f..9cc9561 100644 --- a/include/boost/sync/detail/condition_variables/condition_variable_posix.hpp +++ b/include/boost/sync/detail/condition_variables/condition_variable_posix.hpp @@ -75,13 +75,13 @@ public: { int const res = pthread_cond_init(&m_cond, NULL); if (res) - BOOST_SYNC_DETAIL_THROW(resource_error, (res)("boost::sync::condition_variable constructor failed in pthread_cond_init")); + BOOST_SYNC_DETAIL_THROW(resource_error, (res)("condition_variable constructor failed in pthread_cond_init")); } #endif // defined(PTHREAD_COND_INITIALIZER) ~condition_variable() { - BOOST_VERIFY(::pthread_cond_destroy(&m_cond) == 0); + BOOST_VERIFY(sync::detail::posix::pthread_cond_destroy(&m_cond) == 0); } void notify_one() BOOST_NOEXCEPT @@ -205,7 +205,7 @@ private: { int const res = sync::detail::posix::pthread_cond_wait(&m_cond, mtx); if (res != 0) - BOOST_SYNC_DETAIL_THROW(wait_error, (res)("boost::sync::condition_variable::wait failed in pthread_cond_wait")); + BOOST_SYNC_DETAIL_THROW(wait_error, (res)("condition_variable::wait failed in pthread_cond_wait")); } template< typename Mutex > @@ -221,7 +221,7 @@ private: if (res == ETIMEDOUT) return sync::cv_status::timeout; else if (res != 0) - BOOST_SYNC_DETAIL_THROW(wait_error, (res)("boost::sync::condition_variable timedwait failed in pthread_cond_timedwait")); + BOOST_SYNC_DETAIL_THROW(wait_error, (res)("condition_variable::timed_wait failed in pthread_cond_timedwait")); return sync::cv_status::no_timeout; } diff --git a/include/boost/sync/detail/events/auto_reset_event_futex.hpp b/include/boost/sync/detail/events/auto_reset_event_futex.hpp index 30a153b..2dc2736 100644 --- a/include/boost/sync/detail/events/auto_reset_event_futex.hpp +++ b/include/boost/sync/detail/events/auto_reset_event_futex.hpp @@ -171,7 +171,7 @@ private: again: sync::detail::system_duration::native_type time_left = (abs_timeout - sync::detail::system_time_point::now()).get(); if (time_left <= 0) - goto timeout; + return on_wait_timed_out(); const int status = sync::detail::linux_::futex_timedwait(reinterpret_cast< int* >(&m_state), old_state, time_left); if (status != 0) { @@ -179,8 +179,7 @@ private: switch (err) { case ETIMEDOUT: - old_state = m_state.load(detail::atomic_ns::memory_order_acquire); - goto timeout; + return on_wait_timed_out(); case EINTR: // signal received goto again; @@ -205,26 +204,6 @@ private: } return true; - - timeout: - while (true) - { - const unsigned int posts = old_state >> post_count_lowest_bit; - if (posts == 0) - { - // Remove one waiter - if (m_state.compare_exchange_weak(old_state, old_state - 1u, detail::atomic_ns::memory_order_acquire, detail::atomic_ns::memory_order_release)) - return false; - } - else - { - // Remove one post and one waiter from the counters - if (m_state.compare_exchange_weak(old_state, old_state - (post_count_one + 1u), detail::atomic_ns::memory_order_acquire, detail::atomic_ns::memory_order_release)) - return true; - } - - detail::pause(); - } } bool priv_timed_wait(sync::detail::system_duration dur) @@ -252,26 +231,7 @@ private: switch (err) { case ETIMEDOUT: - old_state = m_state.load(detail::atomic_ns::memory_order_acquire); - while (true) - { - posts = old_state >> post_count_lowest_bit; - if (posts == 0) - { - // Remove one waiter - if (m_state.compare_exchange_weak(old_state, old_state - 1u, detail::atomic_ns::memory_order_acquire, detail::atomic_ns::memory_order_release)) - return false; - } - else - { - // Remove one post and one waiter from the counters - if (m_state.compare_exchange_weak(old_state, old_state - (post_count_one + 1u), detail::atomic_ns::memory_order_acquire, detail::atomic_ns::memory_order_release)) - return true; - } - - detail::pause(); - } - break; + return on_wait_timed_out(); case EINTR: // signal received goto again; @@ -316,6 +276,29 @@ private: return false; } + bool on_wait_timed_out() + { + unsigned int old_state = m_state.load(detail::atomic_ns::memory_order_acquire); + while (true) + { + unsigned int posts = old_state >> post_count_lowest_bit; + if (posts == 0) + { + // Remove one waiter + if (m_state.compare_exchange_weak(old_state, old_state - 1u, detail::atomic_ns::memory_order_acquire, detail::atomic_ns::memory_order_release)) + return false; + } + else + { + // Remove one post and one waiter from the counters + if (m_state.compare_exchange_weak(old_state, old_state - (post_count_one + 1u), detail::atomic_ns::memory_order_acquire, detail::atomic_ns::memory_order_release)) + return true; + } + + detail::pause(); + } + } + private: BOOST_STATIC_ASSERT_MSG(sizeof(detail::atomic_ns::atomic< unsigned int >) == sizeof(int), "Boost.Sync: unexpected size of atomic< unsigned int >"); detail::atomic_ns::atomic< unsigned int > m_state; diff --git a/include/boost/sync/detail/events/manual_reset_event_futex.hpp b/include/boost/sync/detail/events/manual_reset_event_futex.hpp index 1ec9135..8570fc9 100644 --- a/include/boost/sync/detail/events/manual_reset_event_futex.hpp +++ b/include/boost/sync/detail/events/manual_reset_event_futex.hpp @@ -111,7 +111,6 @@ private: // Check that system time resolution is nanoseconds BOOST_STATIC_ASSERT(sync::detail::system_duration::subsecond_fraction == 1000000000u); - // Note that futex timeout must be relative const int status = sync::detail::linux_::futex_timedwait(reinterpret_cast< int* >(&m_state), 0, time_left); if (status == 0) break; @@ -145,7 +144,6 @@ private: BOOST_STATIC_ASSERT(sync::detail::system_duration::subsecond_fraction == 1000000000u); do { - // Note that futex timeout must be relative const int status = sync::detail::linux_::futex_timedwait(reinterpret_cast< int* >(&m_state), 0, time_left); if (status == 0) break; diff --git a/include/boost/sync/detail/mutexes/timed_mutex_posix.hpp b/include/boost/sync/detail/mutexes/timed_mutex_posix.hpp index d434fca..720efa0 100644 --- a/include/boost/sync/detail/mutexes/timed_mutex_posix.hpp +++ b/include/boost/sync/detail/mutexes/timed_mutex_posix.hpp @@ -17,6 +17,7 @@ #ifndef BOOST_SYNC_DETAIL_MUTEXES_TIMED_MUTEX_POSIX_HPP_INCLUDED_ #define BOOST_SYNC_DETAIL_MUTEXES_TIMED_MUTEX_POSIX_HPP_INCLUDED_ +#include #include #include #include @@ -47,7 +48,9 @@ BOOST_SYNC_DETAIL_OPEN_ABI_NAMESPACE { class timed_mutex { public: +#if defined(BOOST_SYNC_DETAIL_PTHREAD_HAS_TIMEDLOCK) typedef void _is_condition_variable_compatible; +#endif typedef pthread_mutex_t* native_handle_type; @@ -91,12 +94,12 @@ public: { int const res = pthread_mutex_init(&m_mutex, NULL); if (res) - BOOST_SYNC_DETAIL_THROW(resource_error, (res)("boost:: timed_mutex constructor failed in pthread_mutex_init")); + BOOST_SYNC_DETAIL_THROW(resource_error, (res)("timed_mutex constructor failed in pthread_mutex_init")); #if !defined(BOOST_SYNC_DETAIL_PTHREAD_HAS_TIMEDLOCK) int const res2 = pthread_cond_init(&m_cond, NULL); if (res2) - BOOST_SYNC_DETAIL_THROW(resource_error, (res2)("boost:: timed_mutex constructor failed in pthread_cond_init")); + BOOST_SYNC_DETAIL_THROW(resource_error, (res2)("timed_mutex constructor failed in pthread_cond_init")); m_is_locked = false; #endif } @@ -106,7 +109,7 @@ public: { BOOST_VERIFY(sync::detail::posix::pthread_mutex_destroy(&m_mutex) == 0); #if !defined(BOOST_SYNC_DETAIL_PTHREAD_HAS_TIMEDLOCK) - BOOST_VERIFY(::pthread_cond_destroy(&m_cond) == 0); + BOOST_VERIFY(sync::detail::posix::pthread_cond_destroy(&m_cond) == 0); #endif } @@ -116,7 +119,7 @@ public: { int const res = sync::detail::posix::pthread_mutex_lock(&m_mutex); if (res) - BOOST_SYNC_DETAIL_THROW(lock_error, (res)("boost: timed_mutex lock failed in pthread_mutex_lock")); + BOOST_SYNC_DETAIL_THROW(lock_error, (res)("timed_mutex lock failed in pthread_mutex_lock")); } void unlock() BOOST_NOEXCEPT @@ -131,7 +134,7 @@ public: if (res == 0) return true; else if (res != EBUSY) - BOOST_SYNC_DETAIL_THROW(lock_error, (res)("boost: timed_mutex trylock failed in pthread_mutex_trylock")); + BOOST_SYNC_DETAIL_THROW(lock_error, (res)("timed_mutex try_lock failed in pthread_mutex_trylock")); return false; } @@ -206,7 +209,7 @@ private: if (res == 0) return true; else if (res != ETIMEDOUT) - BOOST_SYNC_DETAIL_THROW(lock_error, (res)("boost: timed_mutex timedlock failed in pthread_mutex_timedlock")); + BOOST_SYNC_DETAIL_THROW(lock_error, (res)("timed_mutex timed_lock failed in pthread_mutex_timedlock")); return false; #else // defined(BOOST_SYNC_DETAIL_PTHREAD_HAS_TIMEDLOCK) @@ -216,9 +219,16 @@ private: { int const cond_res = sync::detail::posix::pthread_cond_timedwait(&m_cond, &m_mutex, &t.get()); if (cond_res == ETIMEDOUT) - return false; + { + if (!m_is_locked) + break; + else + return false; + } else if (cond_res != 0) - BOOST_SYNC_DETAIL_THROW(lock_error, (cond_res)("boost: timed_mutex timedlock failed in pthread_cond_timedwait")); + { + BOOST_SYNC_DETAIL_THROW(lock_error, (cond_res)("timed_mutex timed_lock failed in pthread_cond_timedwait")); + } } m_is_locked = true; return true; diff --git a/include/boost/sync/detail/mutexes/timed_mutex_windows.hpp b/include/boost/sync/detail/mutexes/timed_mutex_windows.hpp index 219c554..a5efdb2 100644 --- a/include/boost/sync/detail/mutexes/timed_mutex_windows.hpp +++ b/include/boost/sync/detail/mutexes/timed_mutex_windows.hpp @@ -122,7 +122,7 @@ private: if (!boost::detail::winapi::SetWaitableTimer(handles[1], reinterpret_cast< const boost::detail::winapi::LARGE_INTEGER_* >(&t.get()), 0, NULL, NULL, false)) { const boost::detail::winapi::DWORD_ err = boost::detail::winapi::GetLastError(); - BOOST_SYNC_DETAIL_THROW(lock_error, (err)("timed_mutex::timedlock failed to set a timeout")); + BOOST_SYNC_DETAIL_THROW(lock_error, (err)("timed_mutex::timed_lock failed to set a timeout")); } while (true) @@ -131,7 +131,7 @@ private: if (res == boost::detail::winapi::wait_failed) { const boost::detail::winapi::DWORD_ err = boost::detail::winapi::GetLastError(); - BOOST_SYNC_DETAIL_THROW(lock_error, (err)("timed_mutex::timedlock failed in WaitForMultipleObjects")); + BOOST_SYNC_DETAIL_THROW(lock_error, (err)("timed_mutex::timed_lock failed in WaitForMultipleObjects")); } switch (res) @@ -189,7 +189,7 @@ private: default: { const boost::detail::winapi::DWORD_ err = boost::detail::winapi::GetLastError(); - BOOST_SYNC_DETAIL_THROW(lock_error, (err)("timed_mutex::timedlock failed in WaitForSingleObject")); + BOOST_SYNC_DETAIL_THROW(lock_error, (err)("timed_mutex::timed_lock failed in WaitForSingleObject")); } } } diff --git a/include/boost/sync/detail/pthread.hpp b/include/boost/sync/detail/pthread.hpp index 7726a14..cb13728 100644 --- a/include/boost/sync/detail/pthread.hpp +++ b/include/boost/sync/detail/pthread.hpp @@ -53,6 +53,7 @@ using ::pthread_mutex_trylock; using ::pthread_mutex_timedlock; #endif using ::pthread_mutex_unlock; +using ::pthread_cond_destroy; using ::pthread_cond_wait; using ::pthread_cond_timedwait; @@ -60,7 +61,7 @@ using ::pthread_cond_timedwait; // Workaround for https://svn.boost.org/trac/boost/ticket/6200 -BOOST_FORCEINLINE int pthread_mutex_destroy(pthread_mutex_t* m) +BOOST_FORCEINLINE int pthread_mutex_destroy(pthread_mutex_t* m) BOOST_NOEXCEPT { int ret; do @@ -71,7 +72,7 @@ BOOST_FORCEINLINE int pthread_mutex_destroy(pthread_mutex_t* m) return ret; } -BOOST_FORCEINLINE int pthread_mutex_lock(pthread_mutex_t* m) +BOOST_FORCEINLINE int pthread_mutex_lock(pthread_mutex_t* m) BOOST_NOEXCEPT { int ret; do @@ -82,7 +83,7 @@ BOOST_FORCEINLINE int pthread_mutex_lock(pthread_mutex_t* m) return ret; } -BOOST_FORCEINLINE int pthread_mutex_trylock(pthread_mutex_t* m) +BOOST_FORCEINLINE int pthread_mutex_trylock(pthread_mutex_t* m) BOOST_NOEXCEPT { int ret; do @@ -94,7 +95,7 @@ BOOST_FORCEINLINE int pthread_mutex_trylock(pthread_mutex_t* m) } #if defined(BOOST_SYNC_DETAIL_PTHREAD_HAS_TIMEDLOCK) -BOOST_FORCEINLINE int pthread_mutex_timedlock(pthread_mutex_t* m, const struct ::timespec* t) +BOOST_FORCEINLINE int pthread_mutex_timedlock(pthread_mutex_t* m, const struct ::timespec* t) BOOST_NOEXCEPT { int ret; do @@ -106,7 +107,7 @@ BOOST_FORCEINLINE int pthread_mutex_timedlock(pthread_mutex_t* m, const struct : } #endif -BOOST_FORCEINLINE int pthread_mutex_unlock(pthread_mutex_t* m) +BOOST_FORCEINLINE int pthread_mutex_unlock(pthread_mutex_t* m) BOOST_NOEXCEPT { int ret; do @@ -117,7 +118,18 @@ BOOST_FORCEINLINE int pthread_mutex_unlock(pthread_mutex_t* m) return ret; } -BOOST_FORCEINLINE int pthread_cond_wait(pthread_cond_t* c, pthread_mutex_t* m) +BOOST_FORCEINLINE int pthread_cond_destroy(pthread_cond_t* c) BOOST_NOEXCEPT +{ + int ret; + do + { + ret = ::pthread_cond_destroy(c); + } + while (ret == EINTR); + return ret; +} + +BOOST_FORCEINLINE int pthread_cond_wait(pthread_cond_t* c, pthread_mutex_t* m) BOOST_NOEXCEPT { int ret; do @@ -128,7 +140,7 @@ BOOST_FORCEINLINE int pthread_cond_wait(pthread_cond_t* c, pthread_mutex_t* m) return ret; } -BOOST_FORCEINLINE int pthread_cond_timedwait(pthread_cond_t* c, pthread_mutex_t* m, const struct ::timespec* t) +BOOST_FORCEINLINE int pthread_cond_timedwait(pthread_cond_t* c, pthread_mutex_t* m, const struct ::timespec* t) BOOST_NOEXCEPT { int ret; do diff --git a/include/boost/sync/detail/pthread_mutex_locks.hpp b/include/boost/sync/detail/pthread_mutex_locks.hpp index e4cc555..52f8f39 100644 --- a/include/boost/sync/detail/pthread_mutex_locks.hpp +++ b/include/boost/sync/detail/pthread_mutex_locks.hpp @@ -40,12 +40,12 @@ class pthread_mutex_lock_guard public: explicit pthread_mutex_lock_guard(pthread_mutex_t& m) BOOST_NOEXCEPT : m_mutex(&m) { - BOOST_VERIFY(pthread_mutex_lock(m_mutex) == 0); + BOOST_VERIFY(sync::detail::posix::pthread_mutex_lock(m_mutex) == 0); } ~pthread_mutex_lock_guard() { - BOOST_VERIFY(pthread_mutex_unlock(m_mutex) == 0); + BOOST_VERIFY(sync::detail::posix::pthread_mutex_unlock(m_mutex) == 0); } BOOST_DELETED_FUNCTION(pthread_mutex_lock_guard(pthread_mutex_lock_guard const&)) @@ -59,12 +59,12 @@ class pthread_mutex_unlock_guard public: explicit pthread_mutex_unlock_guard(pthread_mutex_t& m) BOOST_NOEXCEPT : m_mutex(&m) { - BOOST_VERIFY(pthread_mutex_unlock(m_mutex) == 0); + BOOST_VERIFY(sync::detail::posix::pthread_mutex_unlock(m_mutex) == 0); } ~pthread_mutex_unlock_guard() { - BOOST_VERIFY(pthread_mutex_lock(m_mutex) == 0); + BOOST_VERIFY(sync::detail::posix::pthread_mutex_lock(m_mutex) == 0); } BOOST_DELETED_FUNCTION(pthread_mutex_unlock_guard(pthread_mutex_unlock_guard const&)) diff --git a/include/boost/sync/detail/semaphore/basic_semaphore_mach.hpp b/include/boost/sync/detail/semaphore/basic_semaphore_mach.hpp index e66697c..131744c 100644 --- a/include/boost/sync/detail/semaphore/basic_semaphore_mach.hpp +++ b/include/boost/sync/detail/semaphore/basic_semaphore_mach.hpp @@ -97,21 +97,21 @@ public: sync::detail::system_duration::native_type time_left = dur.get(); if (time_left < 0) time_left = 0; - sync::detail::system_duration::native_type time_left_sec = time_left / system_duration::subsecond_fraction; - sync::detail::system_duration::native_type time_left_subsec = time_left % system_duration::subsecond_fraction; + sync::detail::system_duration::native_type time_left_sec = time_left / sync::detail::system_duration::subsecond_fraction; + sync::detail::system_duration::native_type time_left_subsec = time_left % sync::detail::system_duration::subsecond_fraction; enum { nanoseconds_fraction = 1000000000u, - conversion_ratio = static_cast< uint64_t >(nanoseconds_fraction) >= system_duration::subsecond_fraction ? - nanoseconds_fraction / system_duration::subsecond_fraction : - system_duration::subsecond_fraction / nanoseconds_fraction + conversion_ratio = static_cast< uint64_t >(nanoseconds_fraction) >= sync::detail::system_duration::subsecond_fraction ? + nanoseconds_fraction / sync::detail::system_duration::subsecond_fraction : + sync::detail::system_duration::subsecond_fraction / nanoseconds_fraction }; const mach_timespec_t wait_time = { time_left_sec, - static_cast< uint64_t >(nanoseconds_fraction) >= system_duration::subsecond_fraction ? + static_cast< uint64_t >(nanoseconds_fraction) >= sync::detail::system_duration::subsecond_fraction ? time_left_subsec / conversion_ratio : time_left_subsec * conversion_ratio }; diff --git a/test/run/mutex_test.cpp b/test/run/mutex_test.cpp index 4d0eb58..c4fd183 100644 --- a/test/run/mutex_test.cpp +++ b/test/run/mutex_test.cpp @@ -284,6 +284,7 @@ BOOST_AUTO_TEST_CASE(test_try_mutex) timed_test(&do_test_try_mutex, 3); } +/* void do_test_timed_mutex() { test_lock()(); @@ -296,7 +297,6 @@ BOOST_AUTO_TEST_CASE(test_timed_mutex) timed_test(&do_test_timed_mutex, 3); } -/* void do_test_recursive_mutex() { test_lock()(); diff --git a/test/run/utils.hpp b/test/run/utils.hpp index 4efab7a..4e4e642 100644 --- a/test/run/utils.hpp +++ b/test/run/utils.hpp @@ -2,7 +2,7 @@ // William E. Kempf // Copyright (C) 2007-8 Anthony Williams // -// Distributed under the Boost Software License, Version 1.0. (See accompanying +// 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) #ifndef BOOST_SYNC_LIBS_TEST_UTILS_HPP_INCLUDED_ @@ -150,7 +150,7 @@ private: } // boostinspect:nounnamed -namespace +namespace { template @@ -191,7 +191,7 @@ private: } // boostinspect:nounnamed -namespace +namespace { template inline thread_detail_anon::thread_binder bind(const F& func, const T& param) @@ -226,7 +226,7 @@ private: } // boostinspect:nounnamed -namespace +namespace { template inline thread_detail_anon::thread_member_binder bind(R (T::*func)(), T& param)