From a7334019b11798a9e791edef62a690b521e78a5b Mon Sep 17 00:00:00 2001 From: Mike Crowe Date: Mon, 2 Dec 2019 16:23:06 +0000 Subject: [PATCH] libstdc++: Fix timed_mutex::try_lock_until on arbitrary clock (PR 91906) A non-standard clock may tick more slowly than std::chrono::steady_clock. This means that we risk returning false early when the specified timeout may not have expired. This can be avoided by looping until the timeout time as reported by the non-standard clock has been reached. Unfortunately, we have no way to tell whether the non-standard clock ticks more quickly that std::chrono::steady_clock. If it does then we risk returning later than would be expected, but that is unavoidable and permitted by the standard. 2019-12-02 Mike Crowe PR libstdc++/91906 Fix timed_mutex::try_lock_until on arbitrary clock * include/std/mutex (__timed_mutex_impl::_M_try_lock_until): Loop until the absolute timeout time is reached as measured against the appropriate clock. * testsuite/util/slow_clock.h: New file. Move implementation of slow_clock test class. * testsuite/30_threads/condition_variable/members/2.cc: Include slow_clock from header. * testsuite/30_threads/shared_timed_mutex/try_lock/3.cc: Convert existing test to templated function so that it can be called with both system_clock and steady_clock. * testsuite/30_threads/timed_mutex/try_lock_until/3.cc: Also run test using slow_clock to test above fix. * testsuite/30_threads/recursive_timed_mutex/try_lock_until/3.cc: Likewise. * testsuite/30_threads/recursive_timed_mutex/try_lock_until/4.cc: Add new test that try_lock_until behaves as try_lock if the timeout has already expired or exactly matches the current time. From-SVN: r278902 --- libstdc++-v3/ChangeLog | 19 ++++++ libstdc++-v3/include/std/mutex | 13 +++- .../condition_variable/members/2.cc | 17 +---- .../recursive_timed_mutex/try_lock_until/3.cc | 2 + .../shared_timed_mutex/try_lock/3.cc | 17 +++-- .../timed_mutex/try_lock_until/3.cc | 2 + .../timed_mutex/try_lock_until/4.cc | 68 +++++++++++++++++++ libstdc++-v3/testsuite/util/slow_clock.h | 38 +++++++++++ 8 files changed, 153 insertions(+), 23 deletions(-) create mode 100644 libstdc++-v3/testsuite/30_threads/timed_mutex/try_lock_until/4.cc create mode 100644 libstdc++-v3/testsuite/util/slow_clock.h diff --git a/libstdc++-v3/ChangeLog b/libstdc++-v3/ChangeLog index d39a06f962f..a67fb0c23ea 100644 --- a/libstdc++-v3/ChangeLog +++ b/libstdc++-v3/ChangeLog @@ -1,5 +1,24 @@ 2019-12-02 Mike Crowe + PR libstdc++/91906 Fix timed_mutex::try_lock_until on arbitrary clock + * include/std/mutex (__timed_mutex_impl::_M_try_lock_until): Loop + until the absolute timeout time is reached as measured against the + appropriate clock. + * testsuite/util/slow_clock.h: New file. Move implementation of + slow_clock test class. + * testsuite/30_threads/condition_variable/members/2.cc: Include + slow_clock from header. + * testsuite/30_threads/shared_timed_mutex/try_lock/3.cc: Convert + existing test to templated function so that it can be called with + both system_clock and steady_clock. + * testsuite/30_threads/timed_mutex/try_lock_until/3.cc: Also run test + using slow_clock to test above fix. + * testsuite/30_threads/recursive_timed_mutex/try_lock_until/3.cc: + Likewise. + * testsuite/30_threads/recursive_timed_mutex/try_lock_until/4.cc: Add + new test that try_lock_until behaves as try_lock if the timeout has + already expired or exactly matches the current time. + PR libstdc++/78237 Add full steady_clock support to timed_mutex * acinclude.m4 (GLIBCXX_CHECK_PTHREAD_MUTEX_CLOCKLOCK): Define to detect presence of pthread_mutex_clocklock function. diff --git a/libstdc++-v3/include/std/mutex b/libstdc++-v3/include/std/mutex index 26ee084b979..10e1ea5a61b 100644 --- a/libstdc++-v3/include/std/mutex +++ b/libstdc++-v3/include/std/mutex @@ -189,8 +189,17 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION bool _M_try_lock_until(const chrono::time_point<_Clock, _Duration>& __atime) { - auto __rtime = __atime - _Clock::now(); - return _M_try_lock_for(__rtime); + // The user-supplied clock may not tick at the same rate as + // steady_clock, so we must loop in order to guarantee that + // the timeout has expired before returning false. + auto __now = _Clock::now(); + do { + auto __rtime = __atime - __now; + if (_M_try_lock_for(__rtime)) + return true; + __now = _Clock::now(); + } while (__atime > __now); + return false; } }; diff --git a/libstdc++-v3/testsuite/30_threads/condition_variable/members/2.cc b/libstdc++-v3/testsuite/30_threads/condition_variable/members/2.cc index cbac3fa1932..f821d3fe6a5 100644 --- a/libstdc++-v3/testsuite/30_threads/condition_variable/members/2.cc +++ b/libstdc++-v3/testsuite/30_threads/condition_variable/members/2.cc @@ -25,6 +25,7 @@ #include #include #include +#include template void test01() @@ -52,22 +53,6 @@ void test01() } } -struct slow_clock -{ - using rep = std::chrono::system_clock::rep; - using period = std::chrono::system_clock::period; - using duration = std::chrono::system_clock::duration; - using time_point = std::chrono::time_point; - static constexpr bool is_steady = false; - - static time_point now() - { - auto real = std::chrono::system_clock::now(); - return time_point{real.time_since_epoch() / 3}; - } -}; - - void test01_alternate_clock() { try diff --git a/libstdc++-v3/testsuite/30_threads/recursive_timed_mutex/try_lock_until/3.cc b/libstdc++-v3/testsuite/30_threads/recursive_timed_mutex/try_lock_until/3.cc index 162d0f8542b..182c60b11ad 100644 --- a/libstdc++-v3/testsuite/30_threads/recursive_timed_mutex/try_lock_until/3.cc +++ b/libstdc++-v3/testsuite/30_threads/recursive_timed_mutex/try_lock_until/3.cc @@ -26,6 +26,7 @@ #include #include #include +#include template void test() @@ -71,4 +72,5 @@ int main() { test(); test(); + test(); } diff --git a/libstdc++-v3/testsuite/30_threads/shared_timed_mutex/try_lock/3.cc b/libstdc++-v3/testsuite/30_threads/shared_timed_mutex/try_lock/3.cc index fd210335440..1cf191c452f 100644 --- a/libstdc++-v3/testsuite/30_threads/shared_timed_mutex/try_lock/3.cc +++ b/libstdc++-v3/testsuite/30_threads/shared_timed_mutex/try_lock/3.cc @@ -27,7 +27,8 @@ #include #include -int main() +template +void test() { typedef std::shared_timed_mutex mutex_type; @@ -42,15 +43,15 @@ int main() { using namespace std::chrono; auto timeout = 100ms; - auto start = system_clock::now(); + auto start = clock_type::now(); b = m.try_lock_for(timeout); - auto t = system_clock::now() - start; + auto t = clock_type::now() - start; VERIFY( !b ); VERIFY( t >= timeout ); - start = system_clock::now(); + start = clock_type::now(); b = m.try_lock_until(start + timeout); - t = system_clock::now() - start; + t = clock_type::now() - start; VERIFY( !b ); VERIFY( t >= timeout ); } @@ -71,3 +72,9 @@ int main() VERIFY( false ); } } + +int main() +{ + test(); + test(); +} diff --git a/libstdc++-v3/testsuite/30_threads/timed_mutex/try_lock_until/3.cc b/libstdc++-v3/testsuite/30_threads/timed_mutex/try_lock_until/3.cc index f0bf90f9b9e..a28dfd971f5 100644 --- a/libstdc++-v3/testsuite/30_threads/timed_mutex/try_lock_until/3.cc +++ b/libstdc++-v3/testsuite/30_threads/timed_mutex/try_lock_until/3.cc @@ -26,6 +26,7 @@ #include #include #include +#include template void test() @@ -71,4 +72,5 @@ int main() { test(); test(); + test(); } diff --git a/libstdc++-v3/testsuite/30_threads/timed_mutex/try_lock_until/4.cc b/libstdc++-v3/testsuite/30_threads/timed_mutex/try_lock_until/4.cc new file mode 100644 index 00000000000..cd94ede7020 --- /dev/null +++ b/libstdc++-v3/testsuite/30_threads/timed_mutex/try_lock_until/4.cc @@ -0,0 +1,68 @@ +// { dg-do run } +// { dg-options "-pthread" } +// { dg-require-effective-target c++14 } +// { dg-require-effective-target pthread } +// { dg-require-gthreads "" } + +// Copyright (C) 2019 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 +#include + +template +void test() +{ + typedef std::timed_mutex mutex_type; + + try + { + using namespace std::chrono; + mutex_type m; + + // Confirm that try_lock_until acts like try_lock if the timeout has + // already passed. + + // First test with a timeout that is definitely in the past. + VERIFY( m.try_lock_until( clock_type::now() - 1s ) ); + m.unlock(); + + // Then attempt to test with a timeout that might exactly match the + // current time. + VERIFY( m.try_lock_until( clock_type::now() ) ); + m.unlock(); + } + catch (const std::system_error& e) + { + VERIFY( false ); + } + catch (...) + { + VERIFY( false ); + } +} + +int main() +{ + test(); + test(); + test(); +} diff --git a/libstdc++-v3/testsuite/util/slow_clock.h b/libstdc++-v3/testsuite/util/slow_clock.h new file mode 100644 index 00000000000..b81754ec240 --- /dev/null +++ b/libstdc++-v3/testsuite/util/slow_clock.h @@ -0,0 +1,38 @@ +// -*- C++ -*- + +// Copyright (C) 2019 Free Software Foundation, Inc. + +// 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 +// . + + +// A clock that ticks at a third of the speed of system_clock that can be used +// to ensure that functions with timeouts don't erroneously return early. + +#include + +struct slow_clock +{ + using rep = std::chrono::system_clock::rep; + using period = std::chrono::system_clock::period; + using duration = std::chrono::system_clock::duration; + using time_point = std::chrono::time_point; + static constexpr bool is_steady = false; + + static time_point now() + { + auto real = std::chrono::system_clock::now(); + return time_point{real.time_since_epoch() / 3}; + } +}; -- 2.30.2