From 6ee24638ed0ad51e568c799bacf149ba9bd7628b Mon Sep 17 00:00:00 2001 From: Jonathan Wakely Date: Fri, 12 Mar 2021 11:47:20 +0000 Subject: [PATCH] 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. --- libstdc++-v3/include/std/mutex | 242 +++++++++--------- libstdc++-v3/src/c++11/mutex.cc | 37 ++- .../testsuite/30_threads/call_once/66146.cc | 53 ---- 3 files changed, 156 insertions(+), 176 deletions(-) delete mode 100644 libstdc++-v3/testsuite/30_threads/call_once/66146.cc diff --git a/libstdc++-v3/include/std/mutex b/libstdc++-v3/include/std/mutex index f96c48e88ecf..b6a595237bf6 100644 --- a/libstdc++-v3/include/std/mutex +++ b/libstdc++-v3/include/std/mutex @@ -669,6 +669,7 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION }; #endif // C++17 +#ifdef _GLIBCXX_HAS_GTHREADS /// Flag type used by std::call_once struct once_flag { @@ -680,125 +681,27 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION 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. - // The argument is true if the active execution was a returning execution, - // false if it was an exceptional 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 + // 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; -#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) [[__likely__]] - { - _M_once = _Bits::_Active; - return true; - } - else if (_M_passive()) // Caller should have checked this already. - return false; - else - __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 // 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) @@ -815,7 +718,25 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION __once_callable = nullptr; __once_call = nullptr; } -#else // ! TLS + + _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) @@ -832,21 +753,120 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION } private: + // XXX This deadlocks if used recursively (PR 97949) unique_lock _M_functor_lock{__get_once_mutex()}; -#endif // ! TLS _Prepare_execution(const _Prepare_execution&) = delete; _Prepare_execution& operator=(const _Prepare_execution&) = delete; }; +# endif /// @endcond -#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); /// Invoke a callable and synchronize with other calls using the same flag template void call_once(once_flag& __once, _Callable&& __f, _Args&&... __args) { -#if defined _GLIBCXX_HAVE_LINUX_FUTEX || ! defined _GLIBCXX_HAS_GTHREADS + // 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 + { + 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: + // There are two different std::once_flag interfaces, abstracting four + // different implementations. + // 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; + + // Check to see if all executions will be passive now. + bool + _M_passive() const noexcept; + + // Attempts to begin an active execution. + bool _M_activate(); + + // Must be called to complete an active execution. + // The argument is true if the active execution was a returning execution, + // false if it was an exceptional 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; + }; + + template + friend void + call_once(once_flag& __once, _Callable&& __f, _Args&&... __args); + }; + + // 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) [[__likely__]] + { + _M_once = _Bits::_Active; + return true; + } + else if (_M_passive()) // Caller should have checked this already. + return false; + else + __throw_system_error(EDEADLK); + } + + inline void + once_flag::_M_finish(bool __returning) noexcept + { _M_once = __returning ? _Bits::_Done : _Bits::_Init; } + + /// Invoke a callable and synchronize with other calls using the same flag + template + inline void + call_once(once_flag& __once, _Callable&& __f, _Args&&... __args) + { 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 79db3180a685..3b48998b7e41 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 f5529373a058..000000000000 --- 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(); -} -- 2.43.5