From 93e79ed391b9c636f087e6eb7e70f14963cd10ad Mon Sep 17 00:00:00 2001 From: Jonathan Wakely Date: Tue, 3 Nov 2020 18:44:32 +0000 Subject: [PATCH] libstdc++: Rewrite std::call_once to use futexes [PR 66146] The current implementation of std::call_once uses pthread_once, which only meets the C++ requirements when compiled with support for exceptions. For most glibc targets and all non-glibc targets, pthread_once does not work correctly if the init_routine exits via an exception. The pthread_once_t object is left in the "active" state, and any later attempts to run another init_routine will block forever. This change makes std::call_once work correctly for Linux targets, by replacing the use of pthread_once with a futex, based on the code from __cxa_guard_acquire. For both glibc and musl, the Linux implementation of pthread_once is already based on futexes, and pthread_once_t is just a typedef for int, so this change does not alter the layout of std::once_flag. By choosing the values for the int appropriately, the new code is even ABI compatible. Code that calls the old implementation of std::call_once will use pthread_once to manipulate the int, while new code will use the new std::once_flag members to manipulate it, but they should interoperate correctly. In both cases, the int is initially zero, has the lowest bit set when there is an active execution, and equals 2 after a successful returning execution. The difference with the new code is that exceptional exceptions are correctly detected and the int is reset to zero. The __cxa_guard_acquire code (and musl's pthread_once) use an additional state to say there are other threads waiting. This allows the futex wake syscall to be skipped if there is no contention. Glibc doesn't use a waiter bit, so we have to unconditionally issue the wake in order to be compatible with code calling the old std::call_once that uses Glibc's pthread_once. If we know that we're using musl (and musl's pthread_once doesn't change) it would be possible to set a waiting state and check for it in std::once_flag::_M_finish(bool), but this patch doesn't do that. This doesn't fix the bug for non-linux targets. A similar approach could be used for targets where we know the definition of pthread_once_t is a mutex and an integer. We could make once_flag._M_activate() use pthread_mutex_lock on the mutex member within the pthread_once_t, and then only set the integer if the execution finishes, and then unlock the mutex. That would require careful study of each target's pthread_once implementation and that work is left for a later date. This also fixes PR 55394 because pthread_once is no longer needed, and PR 84323 because the fast path is now just an atomic load. As a consequence of the new implementation that doesn't use pthread_once, we can also make std::call_once work for targets with no gthreads support. The code for the single-threaded implementation follows the same methods as on Linux, but with no need for atomics or futexes. libstdc++-v3/ChangeLog: PR libstdc++/55394 PR libstdc++/66146 PR libstdc++/84323 * config/abi/pre/gnu.ver (GLIBCXX_3.4.29): Add new symbols. * include/std/mutex [!_GLIBCXX_HAS_GTHREADS] (once_flag): Define even when gthreads is not supported. (once_flag::_M_once) [_GLIBCXX_HAVE_LINUX_FUTEX]: Change type from __gthread_once_t to int. (once_flag::_M_passive(), once_flag::_M_activate()) (once_flag::_M_finish(bool), once_flag::_Active_execution): Define new members for futex and non-threaded implementation. [_GLIBCXX_HAS_GTHREADS] (once_flag::_Prepare_execution): New RAII helper type. (call_once): Use new members of once_flag. * src/c++11/mutex.cc (std::once_flag::_M_activate): Define. (std::once_flag::_M_finish): Define. * testsuite/30_threads/call_once/39909.cc: Do not require gthreads. * testsuite/30_threads/call_once/49668.cc: Likewise. * testsuite/30_threads/call_once/60497.cc: Likewise. * testsuite/30_threads/call_once/call_once1.cc: Likewise. * testsuite/30_threads/call_once/dr2442.cc: Likewise. * testsuite/30_threads/call_once/once_flag.cc: Add test for constexpr constructor. * testsuite/30_threads/call_once/66146.cc: New test. * testsuite/30_threads/call_once/constexpr.cc: Removed. * testsuite/30_threads/once_flag/cons/constexpr.cc: Removed. --- libstdc++-v3/config/abi/pre/gnu.ver | 5 + libstdc++-v3/include/std/mutex | 200 ++++++++++++++---- libstdc++-v3/src/c++11/mutex.cc | 59 ++++++ .../testsuite/30_threads/call_once/39909.cc | 3 +- .../testsuite/30_threads/call_once/49668.cc | 4 +- .../testsuite/30_threads/call_once/60497.cc | 4 +- .../call_once/{constexpr.cc => 66146.cc} | 40 +++- .../30_threads/call_once/call_once1.cc | 6 +- .../testsuite/30_threads/call_once/dr2442.cc | 4 +- .../30_threads/call_once/once_flag.cc | 10 +- .../30_threads/once_flag/cons/constexpr.cc | 29 --- 11 files changed, 274 insertions(+), 90 deletions(-) rename libstdc++-v3/testsuite/30_threads/call_once/{constexpr.cc => 66146.cc} (54%) delete mode 100644 libstdc++-v3/testsuite/30_threads/once_flag/cons/constexpr.cc diff --git a/libstdc++-v3/config/abi/pre/gnu.ver b/libstdc++-v3/config/abi/pre/gnu.ver index 4dddfd3d263..707539a17c3 100644 --- a/libstdc++-v3/config/abi/pre/gnu.ver +++ b/libstdc++-v3/config/abi/pre/gnu.ver @@ -2371,6 +2371,11 @@ GLIBCXX_3.4.29 { # basic_stringstream::view() _ZNKSt7__cxx1118basic_stringstreamI[cw]St11char_traitsI[cw]ESaI[cw]EE4viewEv; + # std::once_flag::_M_activate() + _ZNSt9once_flag11_M_activateEv; + # std::once_flag::_M_finish(bool) + _ZNSt9once_flag9_M_finishEb; + } GLIBCXX_3.4.28; # Symbols in the support library (libsupc++) have their own tag. diff --git a/libstdc++-v3/include/std/mutex b/libstdc++-v3/include/std/mutex index 12b7e548d17..92ad7b7b662 100644 --- a/libstdc++-v3/include/std/mutex +++ b/libstdc++-v3/include/std/mutex @@ -46,8 +46,10 @@ # include # include #endif -#ifndef _GLIBCXX_HAVE_TLS -# include +#include // __gnu_cxx::__is_single_threaded + +#if defined _GLIBCXX_HAS_GTHREADS && ! defined _GLIBCXX_HAVE_TLS +# include // std::function #endif namespace std _GLIBCXX_VISIBILITY(default) @@ -667,16 +669,9 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION }; #endif // C++17 -#ifdef _GLIBCXX_HAS_GTHREADS /// Flag type used by std::call_once struct once_flag { - private: - typedef __gthread_once_t __native_type; - __native_type _M_once = __GTHREAD_ONCE_INIT; - - public: - /// Constructor constexpr once_flag() noexcept = default; /// Deleted copy constructor @@ -684,16 +679,105 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION /// Deleted assignment operator once_flag& operator=(const once_flag&) = delete; + private: + // There are two different std::once_flag interfaces, abstracting four + // different implementations. + // The preferred interface uses the _M_activate() and _M_finish(bool) + // member functions (introduced in GCC 11), which start and finish an + // active execution respectively. See [thread.once.callonce] in C++11 + // for the definition of active/passive/returning/exceptional executions. + // This interface is supported for Linux (using atomics and futexes) and + // for single-threaded targets with no gthreads support. + // For other targets a pthread_once_t is used with pthread_once, but that + // doesn't work correctly for exceptional executions. That interface + // uses an object of type _Prepare_execution and a lambda expression. +#if defined _GLIBCXX_HAVE_LINUX_FUTEX || ! defined _GLIBCXX_HAS_GTHREADS + enum _Bits : int { _Init = 0, _Active = 1, _Done = 2 }; + + int _M_once = _Bits::_Init; + + // Non-blocking check to see if all executions will be passive now. + bool + _M_passive() const noexcept; + + // Attempts to begin an active execution. Blocks until it either: + // - returns true if an active execution has started on this thread, or + // - returns false if a returning execution happens on another thread. + bool _M_activate(); + + // Must be called to complete an active execution. + void _M_finish(bool __returning) noexcept; + + // RAII helper to call _M_finish. + struct _Active_execution + { + explicit _Active_execution(once_flag& __flag) : _M_flag(__flag) { } + + ~_Active_execution() { _M_flag._M_finish(_M_returning); } + + _Active_execution(const _Active_execution&) = delete; + _Active_execution& operator=(const _Active_execution&) = delete; + + once_flag& _M_flag; + bool _M_returning = false; + }; +#else + __gthread_once_t _M_once = __GTHREAD_ONCE_INIT; + + struct _Prepare_execution; +#endif // ! GTHREADS + template friend void call_once(once_flag& __once, _Callable&& __f, _Args&&... __args); }; +#if ! defined _GLIBCXX_HAS_GTHREADS + // Inline definitions of std::once_flag members for single-threaded targets. + + inline bool + once_flag::_M_passive() const noexcept + { return _M_once == _Bits::_Done; } + + inline bool + once_flag::_M_activate() + { + if (_M_once == _Bits::_Init) + { + _M_once = _Bits::_Active; + return true; + } + else if (!_M_passive()) + __throw_system_error(EDEADLK); + } + + inline void + once_flag::_M_finish(bool returning) noexcept + { _M_once = returning ? _Bits::_Done : _Bits::_Init; } + +#elif defined _GLIBCXX_HAVE_LINUX_FUTEX + + // Define this inline to make passive executions fast. + inline bool + once_flag::_M_passive() const noexcept + { + if (__gnu_cxx::__is_single_threaded()) + return _M_once == _Bits::_Done; + else + return __atomic_load_n(&_M_once, __ATOMIC_ACQUIRE) == _Bits::_Done; + } + +#else // GTHREADS && ! FUTEX + /// @cond undocumented -#ifdef _GLIBCXX_HAVE_TLS +# ifdef _GLIBCXX_HAVE_TLS + // If TLS is available use thread-local state for the type-erased callable + // that is being run by std::call_once in the current thread. extern __thread void* __once_callable; extern __thread void (*__once_call)(); -#else +# else + // Without TLS use a global std::mutex and store the callable in a + // global std::function. extern function __once_functor; extern void @@ -701,48 +785,92 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION extern mutex& __get_once_mutex(); -#endif +# endif + // This function is passed to pthread_once by std::call_once. + // It runs __once_call() or __once_functor(). extern "C" void __once_proxy(void); + + // RAII type to set up state for pthread_once call. + struct once_flag::_Prepare_execution + { +#ifdef _GLIBCXX_HAVE_TLS + template + explicit + _Prepare_execution(_Callable& __c) + { + // Store address in thread-local pointer: + __once_callable = std::__addressof(__c); + // Trampoline function to invoke the closure via thread-local pointer: + __once_call = [] { (*static_cast<_Callable*>(__once_callable))(); }; + } + + ~_Prepare_execution() + { + // PR libstdc++/82481 + __once_callable = nullptr; + __once_call = nullptr; + } +#else // ! TLS + template + explicit + _Prepare_execution(_Callable& __c) + { + // Store the callable in the global std::function + __once_functor = __c; + __set_once_functor_lock_ptr(&_M_functor_lock); + } + + ~_Prepare_execution() + { + if (_M_functor_lock) + __set_once_functor_lock_ptr(nullptr); + } + + private: + unique_lock _M_functor_lock{__get_once_mutex()}; +#endif // ! TLS + + _Prepare_execution(const _Prepare_execution&) = delete; + _Prepare_execution& operator=(const _Prepare_execution&) = delete; + }; /// @endcond +#endif /// Invoke a callable and synchronize with other calls using the same flag template void call_once(once_flag& __once, _Callable&& __f, _Args&&... __args) { - // _GLIBCXX_RESOLVE_LIB_DEFECTS - // 2442. call_once() shouldn't DECAY_COPY() +#if defined _GLIBCXX_HAVE_LINUX_FUTEX || ! defined _GLIBCXX_HAS_GTHREADS + if (__once._M_passive()) + return; + else if (__once._M_activate()) + { + once_flag::_Active_execution __exec(__once); + + // _GLIBCXX_RESOLVE_LIB_DEFECTS + // 2442. call_once() shouldn't DECAY_COPY() + std::__invoke(std::forward<_Callable>(__f), + std::forward<_Args>(__args)...); + + // __f(__args...) did not throw + __exec._M_returning = true; + } +#else + // Closure type that runs the function auto __callable = [&] { std::__invoke(std::forward<_Callable>(__f), std::forward<_Args>(__args)...); }; -#ifdef _GLIBCXX_HAVE_TLS - __once_callable = std::__addressof(__callable); - __once_call = []{ (*(decltype(__callable)*)__once_callable)(); }; -#else - unique_lock __functor_lock(__get_once_mutex()); - __once_functor = __callable; - __set_once_functor_lock_ptr(&__functor_lock); -#endif - - int __e = __gthread_once(&__once._M_once, &__once_proxy); -#ifndef _GLIBCXX_HAVE_TLS - if (__functor_lock) - __set_once_functor_lock_ptr(0); -#endif + once_flag::_Prepare_execution __exec(__callable); -#ifdef __clang_analyzer__ - // PR libstdc++/82481 - __once_callable = nullptr; - __once_call = nullptr; -#endif - - if (__e) + // XXX pthread_once does not reset the flag if an exception is thrown. + if (int __e = __gthread_once(&__once._M_once, &__once_proxy)) __throw_system_error(__e); +#endif } -#endif // _GLIBCXX_HAS_GTHREADS // @} group mutexes _GLIBCXX_END_NAMESPACE_VERSION diff --git a/libstdc++-v3/src/c++11/mutex.cc b/libstdc++-v3/src/c++11/mutex.cc index e24502832d4..286f77f9a45 100644 --- a/libstdc++-v3/src/c++11/mutex.cc +++ b/libstdc++-v3/src/c++11/mutex.cc @@ -25,6 +25,65 @@ #include #ifdef _GLIBCXX_HAS_GTHREADS + +#ifdef _GLIBCXX_HAVE_LINUX_FUTEX +#include +#include +#include + +bool +std::once_flag::_M_activate() +{ + if (__gnu_cxx::__is_single_threaded()) + { + if (_M_once == _Bits::_Done) + return false; + _M_once = _Bits::_Active; + return true; + } + + while (true) + { + int expected = _Bits::_Init; + constexpr int active = _Bits::_Active; + if (__atomic_compare_exchange_n(&_M_once, &expected, active, false, + __ATOMIC_ACQ_REL, + __ATOMIC_ACQUIRE)) + { + // This thread is now doing an active execution. + return true; + } + + if (expected == _Bits::_Done) + return false; // A returning execution happened, this is passive. + + // Otherwise, an active execution is happening. Wait for it to finish. + constexpr int futex_wait = 128; // FUTEX_WAIT_PRIVATE + syscall (SYS_futex, &_M_once, futex_wait, expected, 0); + } +} + +void +std::once_flag::_M_finish(bool returning) noexcept +{ + const int newval = returning ? _Bits::_Done : _Bits::_Init; + if (__gnu_cxx::__is_single_threaded()) + { + __glibcxx_assert(_M_once == _Bits::_Active); + _M_once = newval; + } + else + { + int prev = __atomic_exchange_n(&_M_once, newval, __ATOMIC_RELEASE); + __glibcxx_assert(prev & _Bits::_Active); + // Wake any other threads waiting for this execution to finish. + constexpr int futex_wake = 129; // FUTEX_WAKE_PRIVATE + syscall (SYS_futex, &_M_once, futex_wake, INT_MAX); + } +} + +#endif // ! FUTEX + #ifndef _GLIBCXX_HAVE_TLS namespace { diff --git a/libstdc++-v3/testsuite/30_threads/call_once/39909.cc b/libstdc++-v3/testsuite/30_threads/call_once/39909.cc index 1f35d7f8b9a..56568ebd154 100644 --- a/libstdc++-v3/testsuite/30_threads/call_once/39909.cc +++ b/libstdc++-v3/testsuite/30_threads/call_once/39909.cc @@ -1,6 +1,5 @@ -// { dg-do run } +// { dg-do run { target c++11 } } // { dg-additional-options "-pthread" { target pthread } } -// { dg-require-effective-target c++11 } // { dg-require-gthreads "" } // Copyright (C) 2009-2020 Free Software Foundation, Inc. diff --git a/libstdc++-v3/testsuite/30_threads/call_once/49668.cc b/libstdc++-v3/testsuite/30_threads/call_once/49668.cc index 7eb5426a822..8fab7c22586 100644 --- a/libstdc++-v3/testsuite/30_threads/call_once/49668.cc +++ b/libstdc++-v3/testsuite/30_threads/call_once/49668.cc @@ -1,7 +1,5 @@ -// { dg-do run } +// { dg-do run { target c++11 } } // { dg-additional-options "-pthread" { target pthread } } -// { dg-require-effective-target c++11 } -// { dg-require-gthreads "" } // Copyright (C) 2011-2020 Free Software Foundation, Inc. // diff --git a/libstdc++-v3/testsuite/30_threads/call_once/60497.cc b/libstdc++-v3/testsuite/30_threads/call_once/60497.cc index 9955a9eebed..a2c7567b505 100644 --- a/libstdc++-v3/testsuite/30_threads/call_once/60497.cc +++ b/libstdc++-v3/testsuite/30_threads/call_once/60497.cc @@ -1,7 +1,5 @@ -// { dg-do compile } +// { dg-do compile { target c++11 } } // { dg-additional-options "-pthread" { target pthread } } -// { dg-require-effective-target c++11 } -// { dg-require-gthreads "" } // Copyright (C) 2014-2020 Free Software Foundation, Inc. // diff --git a/libstdc++-v3/testsuite/30_threads/call_once/constexpr.cc b/libstdc++-v3/testsuite/30_threads/call_once/66146.cc similarity index 54% rename from libstdc++-v3/testsuite/30_threads/call_once/constexpr.cc rename to libstdc++-v3/testsuite/30_threads/call_once/66146.cc index 2643f6c9742..b1ca0eb6fe8 100644 --- a/libstdc++-v3/testsuite/30_threads/call_once/constexpr.cc +++ b/libstdc++-v3/testsuite/30_threads/call_once/66146.cc @@ -1,7 +1,4 @@ -// { dg-do compile { target c++11 } } -// { dg-require-gthreads "" } - -// Copyright (C) 2010-2020 Free Software Foundation, Inc. +// Copyright (C) 2020 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 @@ -18,12 +15,37 @@ // with this library; see the file COPYING3. If not see // . +// { dg-do run { target c++11 } } +// { dg-skip-if "" { pthread && { ! *-*-*linux* } } } +// { dg-additional-options "-pthread" { target pthread } } + #include -#include +#include +#include + +void +test01() +{ + std::once_flag once; + int counter = 0; + for (int i = 0; i < 10; ++i) + { + try + { + std::call_once(once, [&]{ if (i < 3) throw i; ++counter; }); + VERIFY(i >= 3); + } + catch (int ex) + { + VERIFY(i < 3); + } + } + VERIFY(counter == 1); + std::call_once(once, []{ std::abort(); }); +} -int main() +int +main() { - __gnu_test::constexpr_default_constructible test; - test.operator()(); - return 0; + test01(); } diff --git a/libstdc++-v3/testsuite/30_threads/call_once/call_once1.cc b/libstdc++-v3/testsuite/30_threads/call_once/call_once1.cc index 26cfa576e81..d9b6729f6b6 100644 --- a/libstdc++-v3/testsuite/30_threads/call_once/call_once1.cc +++ b/libstdc++-v3/testsuite/30_threads/call_once/call_once1.cc @@ -1,7 +1,5 @@ -// { dg-do run } +// { dg-do run { target c++11 } } // { dg-additional-options "-pthread" { target pthread } } -// { dg-require-effective-target c++11 } -// { dg-require-gthreads "" } // Copyright (C) 2008-2020 Free Software Foundation, Inc. // @@ -35,7 +33,7 @@ void add_to_value(int i) int main() { - try + try { std::call_once(value_flag, add_to_value, 2); std::call_once(value_flag, add_to_value, 2); diff --git a/libstdc++-v3/testsuite/30_threads/call_once/dr2442.cc b/libstdc++-v3/testsuite/30_threads/call_once/dr2442.cc index 28947101519..61f5cc09ba8 100644 --- a/libstdc++-v3/testsuite/30_threads/call_once/dr2442.cc +++ b/libstdc++-v3/testsuite/30_threads/call_once/dr2442.cc @@ -1,7 +1,5 @@ -// { dg-do run } +// { dg-do run { target c++11 } } // { dg-additional-options "-pthread" { target pthread } } -// { dg-require-effective-target c++11 } -// { dg-require-gthreads "" } // Copyright (C) 2016-2020 Free Software Foundation, Inc. // diff --git a/libstdc++-v3/testsuite/30_threads/call_once/once_flag.cc b/libstdc++-v3/testsuite/30_threads/call_once/once_flag.cc index 5be04349c03..f6e1607276f 100644 --- a/libstdc++-v3/testsuite/30_threads/call_once/once_flag.cc +++ b/libstdc++-v3/testsuite/30_threads/call_once/once_flag.cc @@ -1,5 +1,4 @@ // { dg-do compile { target c++11 } } -// { dg-require-gthreads "" } // Copyright (C) 2008-2020 Free Software Foundation, Inc. // @@ -20,8 +19,17 @@ #include +#include void test01() { + static_assert( std::is_default_constructible::value, ""); + std::once_flag once_flag; } + +void test02() +{ + __gnu_test::constexpr_default_constructible test; + test.operator()(); +} diff --git a/libstdc++-v3/testsuite/30_threads/once_flag/cons/constexpr.cc b/libstdc++-v3/testsuite/30_threads/once_flag/cons/constexpr.cc deleted file mode 100644 index a356e8c58bc..00000000000 --- a/libstdc++-v3/testsuite/30_threads/once_flag/cons/constexpr.cc +++ /dev/null @@ -1,29 +0,0 @@ -// { dg-do compile { target c++11 } } -// { dg-require-gthreads "" } - -// Copyright (C) 2010-2020 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 - -int main() -{ - __gnu_test::constexpr_default_constructible test; - test.operator()(); - return 0; -} -- 2.30.2