From 25e00ab67444a01dce446e95308521d1a73f8232 Mon Sep 17 00:00:00 2001 From: Jonathan Wakely Date: Tue, 18 Jun 2013 22:55:02 +0000 Subject: [PATCH] re PR libstdc++/57641 (std::timed_mutex.try_lock_until() is broken) PR libstdc++/57641 * include/std/mutex (timed_mutex, recursive_timed_mutex): Move common functionality to new __timed_mutex_impl mixin. Overload try_lock_until to handle conversion between different clocks. Replace constrained __try_lock_for_impl overloads with conditional increment. * include/std/shared_mutex (shared_mutex::_Mutex): Use the new mixin. * testsuite/30_threads/timed_mutex/try_lock_until/57641.cc: New. From-SVN: r200180 --- libstdc++-v3/ChangeLog | 10 ++ libstdc++-v3/include/std/mutex | 142 +++++++----------- libstdc++-v3/include/std/shared_mutex | 48 +----- .../timed_mutex/try_lock_until/57641.cc | 69 +++++++++ 4 files changed, 138 insertions(+), 131 deletions(-) create mode 100644 libstdc++-v3/testsuite/30_threads/timed_mutex/try_lock_until/57641.cc diff --git a/libstdc++-v3/ChangeLog b/libstdc++-v3/ChangeLog index cbbef5914474..4428e94a7f9e 100644 --- a/libstdc++-v3/ChangeLog +++ b/libstdc++-v3/ChangeLog @@ -1,3 +1,13 @@ +2013-06-18 Jonathan Wakely + + PR libstdc++/57641 + * include/std/mutex (timed_mutex, recursive_timed_mutex): Move common + functionality to new __timed_mutex_impl mixin. Overload try_lock_until + to handle conversion between different clocks. Replace constrained + __try_lock_for_impl overloads with conditional increment. + * include/std/shared_mutex (shared_mutex::_Mutex): Use the new mixin. + * testsuite/30_threads/timed_mutex/try_lock_until/57641.cc: New. + 2013-06-17 Jonathan Wakely Chris Jefferson diff --git a/libstdc++-v3/include/std/mutex b/libstdc++-v3/include/std/mutex index cdd05a37cbee..40b2e31dc7f9 100644 --- a/libstdc++-v3/include/std/mutex +++ b/libstdc++-v3/include/std/mutex @@ -199,15 +199,57 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION }; #if _GTHREAD_USE_MUTEX_TIMEDLOCK - /// timed_mutex - class timed_mutex : private __mutex_base - { + template + class __timed_mutex_impl + { + protected: #ifdef _GLIBCXX_USE_CLOCK_MONOTONIC - typedef chrono::steady_clock __clock_t; + typedef chrono::steady_clock __clock_t; #else - typedef chrono::high_resolution_clock __clock_t; + typedef chrono::high_resolution_clock __clock_t; #endif + template + bool + _M_try_lock_for(const chrono::duration<_Rep, _Period>& __rtime) + { + auto __rt = chrono::duration_cast<__clock_t::duration>(__rtime); + if (ratio_greater<__clock_t::period, _Period>()) + ++__rt; + + return _M_try_lock_until(__clock_t::now() + __rt); + } + + template + bool + _M_try_lock_until(const chrono::time_point<__clock_t, + _Duration>& __atime) + { + chrono::time_point<__clock_t, chrono::seconds> __s = + chrono::time_point_cast(__atime); + + chrono::nanoseconds __ns = + chrono::duration_cast(__atime - __s); + + __gthread_time_t __ts = { + static_cast(__s.time_since_epoch().count()), + static_cast(__ns.count()) + }; + + auto __mutex = static_cast<_Derived*>(this)->native_handle(); + return !__gthread_mutex_timedlock(__mutex, &__ts); + } + + template + bool + _M_try_lock_until(const chrono::time_point<_Clock, _Duration>& __atime) + { return _M_try_lock_for(__atime - _Clock::now()); } + }; + + /// timed_mutex + class timed_mutex + : private __mutex_base, public __timed_mutex_impl + { public: typedef __native_type* native_handle_type; @@ -237,25 +279,12 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION template bool try_lock_for(const chrono::duration<_Rep, _Period>& __rtime) - { return __try_lock_for_impl(__rtime); } + { return _M_try_lock_for(__rtime); } template bool try_lock_until(const chrono::time_point<_Clock, _Duration>& __atime) - { - chrono::time_point<_Clock, chrono::seconds> __s = - chrono::time_point_cast(__atime); - - chrono::nanoseconds __ns = - chrono::duration_cast(__atime - __s); - - __gthread_time_t __ts = { - static_cast(__s.time_since_epoch().count()), - static_cast(__ns.count()) - }; - - return !__gthread_mutex_timedlock(&_M_mutex, &__ts); - } + { return _M_try_lock_until(__atime); } void unlock() @@ -267,40 +296,13 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION native_handle_type native_handle() { return &_M_mutex; } - - private: - template - typename enable_if< - ratio_less_equal<__clock_t::period, _Period>::value, bool>::type - __try_lock_for_impl(const chrono::duration<_Rep, _Period>& __rtime) - { - __clock_t::time_point __atime = __clock_t::now() - + chrono::duration_cast<__clock_t::duration>(__rtime); - - return try_lock_until(__atime); - } - - template - typename enable_if< - !ratio_less_equal<__clock_t::period, _Period>::value, bool>::type - __try_lock_for_impl(const chrono::duration<_Rep, _Period>& __rtime) - { - __clock_t::time_point __atime = __clock_t::now() - + ++chrono::duration_cast<__clock_t::duration>(__rtime); - - return try_lock_until(__atime); - } }; /// recursive_timed_mutex - class recursive_timed_mutex : private __recursive_mutex_base + class recursive_timed_mutex + : private __recursive_mutex_base, + public __timed_mutex_impl { -#ifdef _GLIBCXX_USE_CLOCK_MONOTONIC - typedef chrono::steady_clock __clock_t; -#else - typedef chrono::high_resolution_clock __clock_t; -#endif - public: typedef __native_type* native_handle_type; @@ -330,25 +332,12 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION template bool try_lock_for(const chrono::duration<_Rep, _Period>& __rtime) - { return __try_lock_for_impl(__rtime); } + { return _M_try_lock_for(__rtime); } template bool try_lock_until(const chrono::time_point<_Clock, _Duration>& __atime) - { - chrono::time_point<_Clock, chrono::seconds> __s = - chrono::time_point_cast(__atime); - - chrono::nanoseconds __ns = - chrono::duration_cast(__atime - __s); - - __gthread_time_t __ts = { - static_cast(__s.time_since_epoch().count()), - static_cast(__ns.count()) - }; - - return !__gthread_recursive_mutex_timedlock(&_M_mutex, &__ts); - } + { return _M_try_lock_until(__atime); } void unlock() @@ -360,29 +349,6 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION native_handle_type native_handle() { return &_M_mutex; } - - private: - template - typename enable_if< - ratio_less_equal<__clock_t::period, _Period>::value, bool>::type - __try_lock_for_impl(const chrono::duration<_Rep, _Period>& __rtime) - { - __clock_t::time_point __atime = __clock_t::now() - + chrono::duration_cast<__clock_t::duration>(__rtime); - - return try_lock_until(__atime); - } - - template - typename enable_if< - !ratio_less_equal<__clock_t::period, _Period>::value, bool>::type - __try_lock_for_impl(const chrono::duration<_Rep, _Period>& __rtime) - { - __clock_t::time_point __atime = __clock_t::now() - + ++chrono::duration_cast<__clock_t::duration>(__rtime); - - return try_lock_until(__atime); - } }; #endif #endif // _GLIBCXX_HAS_GTHREADS diff --git a/libstdc++-v3/include/std/shared_mutex b/libstdc++-v3/include/std/shared_mutex index 39ab83a28455..ff58825928e4 100644 --- a/libstdc++-v3/include/std/shared_mutex +++ b/libstdc++-v3/include/std/shared_mutex @@ -56,55 +56,17 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION class shared_mutex { #if _GTHREAD_USE_MUTEX_TIMEDLOCK - struct _Mutex : mutex + struct _Mutex : mutex, __timed_mutex_impl<_Mutex> { - typedef chrono::steady_clock __clock_t; - - template + template bool try_lock_for(const chrono::duration<_Rep, _Period>& __rtime) - { return __try_lock_for_impl(__rtime); } + { return _M_try_lock_for(__rtime); } - template + template bool try_lock_until(const chrono::time_point<_Clock, _Duration>& __atime) - { - chrono::time_point<_Clock, chrono::seconds> __s = - chrono::time_point_cast(__atime); - - chrono::nanoseconds __ns = - chrono::duration_cast(__atime - __s); - - __gthread_time_t __ts = { - static_cast(__s.time_since_epoch().count()), - static_cast(__ns.count()) - }; - - return !__gthread_mutex_timedlock(native_handle(), &__ts); - } - - private: - template - typename enable_if< - ratio_less_equal<__clock_t::period, _Period>::value, bool>::type - __try_lock_for_impl(const chrono::duration<_Rep, _Period>& __rtime) - { - __clock_t::time_point __atime = __clock_t::now() - + chrono::duration_cast<__clock_t::duration>(__rtime); - - return try_lock_until(__atime); - } - - template - typename enable_if< - !ratio_less_equal<__clock_t::period, _Period>::value, bool>::type - __try_lock_for_impl(const chrono::duration<_Rep, _Period>& __rtime) - { - __clock_t::time_point __atime = __clock_t::now() - + ++chrono::duration_cast<__clock_t::duration>(__rtime); - - return try_lock_until(__atime); - } + { return _M_try_lock_until(__atime); } }; #else typedef mutex _Mutex; diff --git a/libstdc++-v3/testsuite/30_threads/timed_mutex/try_lock_until/57641.cc b/libstdc++-v3/testsuite/30_threads/timed_mutex/try_lock_until/57641.cc new file mode 100644 index 000000000000..94fe5b327614 --- /dev/null +++ b/libstdc++-v3/testsuite/30_threads/timed_mutex/try_lock_until/57641.cc @@ -0,0 +1,69 @@ +// { dg-do run { target *-*-freebsd* *-*-netbsd* *-*-linux* *-*-gnu* *-*-solaris* *-*-cygwin *-*-darwin* powerpc-ibm-aix* } } +// { dg-options " -std=gnu++0x -pthread" { target *-*-freebsd* *-*-netbsd* *-*-linux* *-*-gnu* powerpc-ibm-aix* } } +// { dg-options " -std=gnu++0x -pthreads" { target *-*-solaris* } } +// { dg-options " -std=gnu++0x " { target *-*-cygwin *-*-darwin* } } +// { dg-require-cstdint "" } +// { dg-require-gthreads-timed "" } + +// Copyright (C) 2013 Free Software Foundation, Inc. +// +// This file is part of the GNU ISO C++ Library. This library is free +// software; you can redistribute it and/or modify it under the +// terms of the GNU General Public License as published by the +// Free Software Foundation; either version 3, or (at your option) +// any later version. + +// This library is distributed in the hope that it will be useful, +// but WITHOUT ANY WARRANTY; without even the implied warranty of +// MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the +// GNU General Public License for more details. + +// You should have received a copy of the GNU General Public License along +// with this library; see the file COPYING3. If not see +// . + +#include +#include +#include +#include + +// PR libstdc++/57641 + +namespace C = std::chrono; + +// custom clock with epoch 10s before system_clock's +struct clock +{ + typedef C::system_clock::rep rep; + typedef C::system_clock::period period; + typedef C::system_clock::duration duration; + typedef C::time_point time_point; + static constexpr bool is_steady = C::system_clock::is_steady; + + static time_point + now() + { + auto sys_time = C::system_clock::now().time_since_epoch(); + return time_point(sys_time + C::seconds(10)); + } +}; + +std::timed_mutex mx; +bool test = false; + +void f() +{ + test = mx.try_lock_until(clock::now() + C::milliseconds(1)); +} + +int main() +{ + bool test = false; + std::lock_guard l(mx); + auto start = C::system_clock::now(); + std::thread t(f); + t.join(); + auto stop = C::system_clock::now(); + VERIFY( (stop - start) < C::seconds(9) ); + VERIFY( !test ); +}