diff mbox series

[1/2] libstdc++: Revert to old std::call_once implementation [PR 99341]

Message ID YEuotRcAX+cCknpe@redhat.com
State New
Headers show
Series [1/2] libstdc++: Revert to old std::call_once implementation [PR 99341] | expand

Commit Message

Jonathan Wakely March 12, 2021, 5:45 p.m. UTC
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.

Tested x86_64-linux, powerpc64-linux and powerpc64le-linux, but not yet
committed (it's not really appropriate for a last-minute Friday
change!)
commit c5ae3f030cc24ec73311f1ff85b8ebdf8551479c
Author: Jonathan Wakely <jwakely@redhat.com>
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 mbox series

Patch

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<typename _Callable, typename... _Args>
+      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<typename _Callable>
+      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<void()> __once_functor;
+
+  extern void
+  __set_once_functor_lock_ptr(unique_lock<mutex>*);
+
+  extern mutex&
+  __get_once_mutex();
+
+  // RAII type to set up state for pthread_once call.
+  struct once_flag::_Prepare_execution
+  {
+    template<typename _Callable>
+      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<mutex> _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<typename _Callable, typename... _Args>
+    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<typename _Callable, typename... _Args>
       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<void()> __once_functor;
-
-  extern void
-  __set_once_functor_lock_ptr(unique_lock<mutex>*);
-
-  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<typename _Callable>
-      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<typename _Callable>
-      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<mutex> _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<typename _Callable, typename... _Args>
-    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 <syscall.h>
-#include <unistd.h>
-#include <limits.h>
+# include <syscall.h>
+# include <unistd.h>
+# include <limits.h>
+
+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
-// <http://www.gnu.org/licenses/>.
-
-// { 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 <mutex>
-#include <cstdlib>
-#include <testsuite_hooks.h>
-
-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();
-}