commit c5ae3f030cc24ec73311f1ff85b8ebdf8551479c Author: Jonathan Wakely Date: Fri Mar 12 11:47:20 2021 libstdc++: Revert to old std::call_once implementation [PR 99341] The new std::call_once implementation is not backwards compatible, contrary to my intention. Because std::once_flag::_M_active() doesn't write glibc's "fork generation" into the pthread_once_t object, it's possible for glibc and libstdc++ to run two active executions concurrently. This violates the primary invariant of the feature! This patch reverts std::once_flag and std::call_once to the old implementation that uses pthread_once. This means PR 66146 is a problem again, but glibc has been changed to solve that. A new API similar to pthread_once but supporting failure and resetting the pthread_once_t will be proposed for inclusion in glibc and other C libraries. This change doesn't simply revert r11-4691 because I want to retain the new implementation for non-ghtreads targets (which didn't previously support std::call_once at all, so there's no backwards compatibility concern). This also leaves the new std::call_once::_M_activate() and std::call_once::_M_finish(bool) symbols present in libstdc++.so.6 so that code already compiled against GCC 11 can still use them. Those symbols will be removed in a subsequent commit (which distros can choose to temporarily revert if needed). libstdc++-v3/ChangeLog: PR libstdc++/99341 * include/std/mutex [_GLIBCXX_HAVE_LINUX_FUTEX] (once_flag): Revert to pthread_once_t implementation. [_GLIBCXX_HAVE_LINUX_FUTEX] (call_once): Likewise. * src/c++11/mutex.cc [_GLIBCXX_HAVE_LINUX_FUTEX] (struct __once_flag_compat): New type matching the reverted implementation of once_flag using futexes. (once_flag::_M_activate): Remove, replace with ... (_ZNSt9once_flag11_M_activateEv): ... alias symbol. (once_flag::_M_finish): Remove, replace with ... (_ZNSt9once_flag9_M_finishEb): ... alias symbol. * testsuite/30_threads/call_once/66146.cc: Removed. diff --git a/libstdc++-v3/include/std/mutex b/libstdc++-v3/include/std/mutex index f96c48e88ec..b6a595237bf 100644 --- a/libstdc++-v3/include/std/mutex +++ b/libstdc++-v3/include/std/mutex @@ -669,6 +669,123 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION }; #endif // C++17 +#ifdef _GLIBCXX_HAS_GTHREADS + /// Flag type used by std::call_once + struct once_flag + { + constexpr once_flag() noexcept = default; + + /// Deleted copy constructor + once_flag(const once_flag&) = delete; + /// Deleted assignment operator + once_flag& operator=(const once_flag&) = delete; + + private: + // For gthreads targets a pthread_once_t is used with pthread_once, but + // for most targets this doesn't work correctly for exceptional executions. + __gthread_once_t _M_once = __GTHREAD_ONCE_INIT; + + struct _Prepare_execution; + + template + friend void + call_once(once_flag& __once, _Callable&& __f, _Args&&... __args); + }; + + /// @cond undocumented +# 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)(); + + // RAII type to set up state for pthread_once call. + struct once_flag::_Prepare_execution + { + 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; + } + + _Prepare_execution(const _Prepare_execution&) = delete; + _Prepare_execution& operator=(const _Prepare_execution&) = delete; + }; + +# else + // Without TLS use a global std::mutex and store the callable in a + // global std::function. + extern function __once_functor; + + extern void + __set_once_functor_lock_ptr(unique_lock*); + + extern mutex& + __get_once_mutex(); + + // RAII type to set up state for pthread_once call. + struct once_flag::_Prepare_execution + { + 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: + // XXX This deadlocks if used recursively (PR 97949) + unique_lock _M_functor_lock{__get_once_mutex()}; + + _Prepare_execution(const _Prepare_execution&) = delete; + _Prepare_execution& operator=(const _Prepare_execution&) = delete; + }; +# endif + /// @endcond + + // This function is passed to pthread_once by std::call_once. + // It runs __once_call() or __once_functor(). + extern "C" void __once_proxy(void); + + /// Invoke a callable and synchronize with other calls using the same flag + template + void + call_once(once_flag& __once, _Callable&& __f, _Args&&... __args) + { + // Closure type that runs the function + auto __callable = [&] { + std::__invoke(std::forward<_Callable>(__f), + std::forward<_Args>(__args)...); + }; + + once_flag::_Prepare_execution __exec(__callable); + + // 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); + } + +#else // _GLIBCXX_HAS_GTHREADS + /// Flag type used by std::call_once struct once_flag { @@ -682,27 +799,19 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION 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 + // The single-threaded interface uses the _M_activate() and _M_finish(bool) + // functions, which start and finish an active execution respectively. + // See [thread.once.callonce] in C++11 for the definition of + // active/passive/returning/exceptional executions. 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. + // 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. + // Attempts to begin an active execution. bool _M_activate(); // Must be called to complete an active execution. @@ -723,18 +832,12 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION 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 @@ -759,94 +862,11 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION 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 - // 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 - // Without TLS use a global std::mutex and store the callable in a - // global std::function. - extern function __once_functor; - - extern void - __set_once_functor_lock_ptr(unique_lock*); - - extern mutex& - __get_once_mutex(); -# 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 + inline void call_once(once_flag& __once, _Callable&& __f, _Args&&... __args) { -#if defined _GLIBCXX_HAVE_LINUX_FUTEX || ! defined _GLIBCXX_HAS_GTHREADS if (__once._M_passive()) return; else if (__once._M_activate()) @@ -861,20 +881,8 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION // __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)...); - }; - - once_flag::_Prepare_execution __exec(__callable); - - // 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 79db3180a68..3b48998b7e4 100644 --- a/libstdc++-v3/src/c++11/mutex.cc +++ b/libstdc++-v3/src/c++11/mutex.cc @@ -26,13 +26,27 @@ #ifdef _GLIBCXX_HAS_GTHREADS +#if defined _GLIBCXX_SHARED && ! _GLIBCXX_INLINE_VERSION + #ifdef _GLIBCXX_HAVE_LINUX_FUTEX -#include -#include -#include +# include +# include +# include + +namespace std _GLIBCXX_VISIBILITY(default) +{ +_GLIBCXX_BEGIN_NAMESPACE_VERSION + +struct __once_flag_compat +{ + enum _Bits : int { _Init = 0, _Active = 1, _Done = 2 }; + int _M_once = 0; + bool _M_activate(); + void _M_finish(bool returning) noexcept; +}; bool -std::once_flag::_M_activate() +__once_flag_compat::_M_activate() { if (__gnu_cxx::__is_single_threaded()) { @@ -64,7 +78,7 @@ std::once_flag::_M_activate() } void -std::once_flag::_M_finish(bool returning) noexcept +std::__once_flag_compat::_M_finish(bool returning) noexcept { const int newval = returning ? _Bits::_Done : _Bits::_Init; if (__gnu_cxx::__is_single_threaded()) @@ -83,7 +97,18 @@ std::once_flag::_M_finish(bool returning) noexcept } } -#endif // ! FUTEX +#pragma GCC diagnostic push +#pragma GCC diagnostic ignored "-Wattribute-alias" +extern "C" bool _ZNSt9once_flag11_M_activateEv() + __attribute__((alias ("_ZNSt18__once_flag_compat11_M_activateEv"))); +extern "C" void _ZNSt9once_flag9_M_finishEb() noexcept + __attribute__((alias ("_ZNSt18__once_flag_compat9_M_finishEb"))); +#pragma GCC diagnostic pop + +_GLIBCXX_END_NAMESPACE_VERSION +} // namespace std +#endif // FUTEX +#endif // ONCE_FLAG_COMPAT && SHARED && ! INLINE_VERSION namespace std _GLIBCXX_VISIBILITY(default) { diff --git a/libstdc++-v3/testsuite/30_threads/call_once/66146.cc b/libstdc++-v3/testsuite/30_threads/call_once/66146.cc deleted file mode 100644 index f5529373a05..00000000000 --- a/libstdc++-v3/testsuite/30_threads/call_once/66146.cc +++ /dev/null @@ -1,53 +0,0 @@ -// Copyright (C) 2020-2021 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 -// . - -// { dg-do run { target c++11 } } -// { dg-additional-options "-pthread" { target pthread } } - -// Currently std::call_once is broken for gthreads targets without futexes: -// { dg-skip-if "see PR 66146" { gthreads && { ! futex } } } - -#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() -{ - test01(); -}