public inbox for libstdc++@gcc.gnu.org
 help / color / mirror / Atom feed
* [committed] libstdc++: Rewrite std::call_once to use futexes [PR 66146]
@ 2020-11-03 18:45 Jonathan Wakely
  2020-11-03 20:04 ` Jonathan Wakely
  2020-11-03 20:13 ` Jonathan Wakely
  0 siblings, 2 replies; 3+ messages in thread
From: Jonathan Wakely @ 2020-11-03 18:45 UTC (permalink / raw)
  To: libstdc++, gcc-patches

[-- Attachment #1: Type: text/plain, Size: 4156 bytes --]

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.

Tested x86_64-linux and powerpc-aix. Committed to trunk.


[-- Attachment #2: patch.txt --]
[-- Type: text/plain, Size: 22051 bytes --]

commit 93e79ed391b9c636f087e6eb7e70f14963cd10ad
Author: Jonathan Wakely <jwakely@redhat.com>
Date:   Tue Nov 3 18:44:32 2020

    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.

diff --git a/libstdc++-v3/config/abi/pre/gnu.ver b/libstdc++-v3/config/abi/pre/gnu.ver
index 4dddfd3d2631..707539a17c3a 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 12b7e548d179..92ad7b7b6622 100644
--- a/libstdc++-v3/include/std/mutex
+++ b/libstdc++-v3/include/std/mutex
@@ -46,8 +46,10 @@
 # include <condition_variable>
 # include <thread>
 #endif
-#ifndef _GLIBCXX_HAVE_TLS
-# include <bits/std_function.h>
+#include <ext/atomicity.h>     // __gnu_cxx::__is_single_threaded
+
+#if defined _GLIBCXX_HAS_GTHREADS && ! defined _GLIBCXX_HAVE_TLS
+# include <bits/std_function.h>  // 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<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
+  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<void()> __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<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
     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<mutex> __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);
+      once_flag::_Prepare_execution __exec(__callable);
 
-#ifndef _GLIBCXX_HAVE_TLS
-      if (__functor_lock)
-        __set_once_functor_lock_ptr(0);
-#endif
-
-#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 e24502832d42..286f77f9a454 100644
--- a/libstdc++-v3/src/c++11/mutex.cc
+++ b/libstdc++-v3/src/c++11/mutex.cc
@@ -25,6 +25,65 @@
 #include <mutex>
 
 #ifdef _GLIBCXX_HAS_GTHREADS
+
+#ifdef _GLIBCXX_HAVE_LINUX_FUTEX
+#include <syscall.h>
+#include <unistd.h>
+#include <limits.h>
+
+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 1f35d7f8b9a1..56568ebd154a 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 7eb5426a822a..8fab7c225861 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 9955a9eebed3..a2c7567b5059 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 2643f6c97424..b1ca0eb6fe8f 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
 // <http://www.gnu.org/licenses/>.
 
-#include <mutex>
-#include <testsuite_common_types.h>
+// { dg-do run { target c++11 } }
+// { dg-skip-if "" { pthread && { ! *-*-*linux* } } }
+// { dg-additional-options "-pthread" { target pthread } }
 
-int main()
+#include <mutex>
+#include <cstdlib>
+#include <testsuite_hooks.h>
+
+void
+test01()
 {
-  __gnu_test::constexpr_default_constructible test;
-  test.operator()<std::mutex>();
-  return 0;
+  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();
 }
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 26cfa576e811..d9b6729f6b6b 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 289471015199..61f5cc09ba84 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 5be04349c03d..f6e1607276f4 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 <mutex>
+#include <testsuite_common_types.h>
 
 void test01()
 {
+  static_assert( std::is_default_constructible<std::once_flag>::value, "");
+
   std::once_flag once_flag;
 }
+
+void test02()
+{
+  __gnu_test::constexpr_default_constructible test;
+  test.operator()<std::once_flag>();
+}
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 a356e8c58bc2..000000000000
--- 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
-// <http://www.gnu.org/licenses/>.
-
-#include <mutex>
-#include <testsuite_common_types.h>
-
-int main()
-{
-  __gnu_test::constexpr_default_constructible test;
-  test.operator()<std::once_flag>();
-  return 0;
-}

^ permalink raw reply	[flat|nested] 3+ messages in thread

* Re: [committed] libstdc++: Rewrite std::call_once to use futexes [PR 66146]
  2020-11-03 18:45 [committed] libstdc++: Rewrite std::call_once to use futexes [PR 66146] Jonathan Wakely
@ 2020-11-03 20:04 ` Jonathan Wakely
  2020-11-03 20:13 ` Jonathan Wakely
  1 sibling, 0 replies; 3+ messages in thread
From: Jonathan Wakely @ 2020-11-03 20:04 UTC (permalink / raw)
  To: libstdc++, gcc-patches

[-- Attachment #1: Type: text/plain, Size: 105 bytes --]

This is a minor refactoring for readability.

Tested x86_64-linux and powerpc-aix. Committed to trunk.



[-- Attachment #2: patch.txt --]
[-- Type: text/x-patch, Size: 4055 bytes --]

commit 9f925f3b198e210e0d124a3c69fae034f429942f
Author: Jonathan Wakely <jwakely@redhat.com>
Date:   Tue Nov 3 19:42:07 2020

    libstdc++: Refactor std::call_once internals
    
    This separates the definition of std::__call_proxy into two funcions,
    one for TLS and one for non-TLS, to make them easier to read. It also
    replaces the __get_once_functor_lock_ptr() internal helper with a new
    set_lock_ptr(unique_lock<mutex>*) function so that __once_proxy doesn't
    need to call it twice.
    
    libstdc++-v3/ChangeLog:
    
            * src/c++11/mutex.cc [_GLIBCXX_HAVE_TLS] (__once_proxy): Define
            separately for TLS targets.
            [!_GLIBCXX_HAVE_TLS] (__get_once_functor_lock_ptr): Replace with ...
            (set_lock_ptr): ... this. Set new value and return previous
            value.
            [!_GLIBCXX_HAVE_TLS] (__set_once_functor_lock_ptr): Adjust to
            use set_lock_ptr.
            [!_GLIBCXX_HAVE_TLS] (__once_proxy): Likewise.

diff --git a/libstdc++-v3/src/c++11/mutex.cc b/libstdc++-v3/src/c++11/mutex.cc
index 286f77f9a454..4d42bed8ecc9 100644
--- a/libstdc++-v3/src/c++11/mutex.cc
+++ b/libstdc++-v3/src/c++11/mutex.cc
@@ -84,18 +84,6 @@ std::once_flag::_M_finish(bool returning) noexcept
 
 #endif // ! FUTEX
 
-#ifndef _GLIBCXX_HAVE_TLS
-namespace
-{
-  inline std::unique_lock<std::mutex>*&
-  __get_once_functor_lock_ptr()
-  {
-    static std::unique_lock<std::mutex>* __once_functor_lock_ptr = 0;
-    return __once_functor_lock_ptr;
-  }
-}
-#endif
-
 namespace std _GLIBCXX_VISIBILITY(default)
 {
 _GLIBCXX_BEGIN_NAMESPACE_VERSION
@@ -103,9 +91,19 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION
 #ifdef _GLIBCXX_HAVE_TLS
   __thread void* __once_callable;
   __thread void (*__once_call)();
-#else
+
+  extern "C" void __once_proxy()
+  {
+    // The caller stored a function pointer in __once_call. If it requires
+    // any state, it gets it from __once_callable.
+    __once_call();
+  }
+
+#else // ! TLS
+
   // Explicit instantiation due to -fno-implicit-instantiation.
   template class function<void()>;
+
   function<void()> __once_functor;
 
   mutex&
@@ -115,11 +113,22 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION
     return once_mutex;
   }
 
+namespace
+{
+  // Store ptr in a global variable and return the previous value.
+  inline unique_lock<mutex>*
+  set_lock_ptr(unique_lock<mutex>* ptr)
+  {
+    static unique_lock<mutex>* __once_functor_lock_ptr = nullptr;
+    return std::__exchange(__once_functor_lock_ptr, ptr);
+  }
+}
+
   // code linked against ABI 3.4.12 and later uses this
   void
   __set_once_functor_lock_ptr(unique_lock<mutex>* __ptr)
   {
-    __get_once_functor_lock_ptr() = __ptr;
+    (void) set_lock_ptr(__ptr);
   }
 
   // unsafe - retained for compatibility with ABI 3.4.11
@@ -129,26 +138,27 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION
     static unique_lock<mutex> once_functor_lock(__get_once_mutex(), defer_lock);
     return once_functor_lock;
   }
-#endif
 
-  extern "C"
+  // This is called via pthread_once while __get_once_mutex() is locked.
+  extern "C" void
+  __once_proxy()
   {
-    void __once_proxy()
+    // Get the callable out of the global functor.
+    function<void()> callable = std::move(__once_functor);
+
+    // Then unlock the global mutex
+    if (unique_lock<mutex>* lock = set_lock_ptr(nullptr))
     {
-#ifndef _GLIBCXX_HAVE_TLS
-      function<void()> __once_call = std::move(__once_functor);
-      if (unique_lock<mutex>* __lock = __get_once_functor_lock_ptr())
-      {
-        // caller is using new ABI and provided lock ptr
-        __get_once_functor_lock_ptr() = 0;
-        __lock->unlock();
-      }
-      else
-        __get_once_functor_lock().unlock();  // global lock
-#endif
-      __once_call();
+      // Caller is using the new ABI and provided a pointer to its lock.
+      lock->unlock();
     }
+    else
+      __get_once_functor_lock().unlock();  // global lock
+
+    // Finally, invoke the callable.
+    callable();
   }
+#endif // ! TLS
 
 _GLIBCXX_END_NAMESPACE_VERSION
 } // namespace std

^ permalink raw reply	[flat|nested] 3+ messages in thread

* Re: [committed] libstdc++: Rewrite std::call_once to use futexes [PR 66146]
  2020-11-03 18:45 [committed] libstdc++: Rewrite std::call_once to use futexes [PR 66146] Jonathan Wakely
  2020-11-03 20:04 ` Jonathan Wakely
@ 2020-11-03 20:13 ` Jonathan Wakely
  1 sibling, 0 replies; 3+ messages in thread
From: Jonathan Wakely @ 2020-11-03 20:13 UTC (permalink / raw)
  To: libstdc++, gcc-patches

[-- Attachment #1: Type: text/plain, Size: 3347 bytes --]

On 03/11/20 18:45 +0000, Jonathan Wakely wrote:
>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.

Something like the attached (completely untested) patch might work so
we could stop using pthread_once for the BSDs and Solaris (and so fix
the bug with exceptional executions).

It's a kluge though (and fragile for Solaris because it assumes the
pthread_once_t type will always have the same layout - in the public
header it's just got lots of 64-bit integers for padding).

I should probably test this some time.

In theory something similar could be done for AIX, but its
pthread_once_t is another struct full of padding integers, and unlike
Solaris we can't look at the code to see how the OS uses those bits.

I'm not sure if this can work for Darwin, as its pthread_once_t isn't
just an aggregate of a pthread_mutex_t and a flag, I think it contains
state used by GCD. That means we can't perform equivalent operations
directly just in terms of Pthread APIs.


[-- Attachment #2: patch.txt --]
[-- Type: text/x-patch, Size: 5391 bytes --]

diff --git a/libstdc++-v3/config/os/bsd/dragonfly/os_defines.h b/libstdc++-v3/config/os/bsd/dragonfly/os_defines.h
index c42c6e661630..0c63a0ee1fdd 100644
--- a/libstdc++-v3/config/os/bsd/dragonfly/os_defines.h
+++ b/libstdc++-v3/config/os/bsd/dragonfly/os_defines.h
@@ -38,4 +38,8 @@
 #define _GLIBCXX_USE_C99_LONG_LONG_CHECK 1
 #define _GLIBCXX_USE_C99_LONG_LONG_DYNAMIC (_GLIBCXX_USE_C99_DYNAMIC || !defined __LONG_LONG_SUPPORTED)
 
+// Make std::call_once access pthread_once_t members directly
+#define _GLIBCXX_COMPAT_PTHREAD_ONCE_T \
+  { int _M_done; pthread_mutex_t _M_mutex; }
+
 #endif
diff --git a/libstdc++-v3/config/os/bsd/freebsd/os_defines.h b/libstdc++-v3/config/os/bsd/freebsd/os_defines.h
index d896de4254f4..fbae94c0370d 100644
--- a/libstdc++-v3/config/os/bsd/freebsd/os_defines.h
+++ b/libstdc++-v3/config/os/bsd/freebsd/os_defines.h
@@ -40,4 +40,8 @@
 #define _GLIBCXX_USE_C99_FLOAT_TRANSCENDENTALS_CHECK 1
 #define _GLIBCXX_USE_C99_FLOAT_TRANSCENDENTALS_DYNAMIC defined _XOPEN_SOURCE
 
+// Make std::call_once access pthread_once_t members directly
+#define _GLIBCXX_COMPAT_PTHREAD_ONCE_T \
+  { int _M_done; pthread_mutex_t _M_mutex; }
+
 #endif
diff --git a/libstdc++-v3/config/os/bsd/netbsd/os_defines.h b/libstdc++-v3/config/os/bsd/netbsd/os_defines.h
index d15727e76e0a..0ab2e6528228 100644
--- a/libstdc++-v3/config/os/bsd/netbsd/os_defines.h
+++ b/libstdc++-v3/config/os/bsd/netbsd/os_defines.h
@@ -30,4 +30,8 @@
 
 #define __ssize_t ssize_t
 
+// Make std::call_once access pthread_once_t members directly
+#define _GLIBCXX_COMPAT_PTHREAD_ONCE_T \
+  { pthread_mutex_t _M_mutex; int _M_done; }
+
 #endif
diff --git a/libstdc++-v3/config/os/bsd/openbsd/os_defines.h b/libstdc++-v3/config/os/bsd/openbsd/os_defines.h
index 1761a10e3836..d248bd2cab29 100644
--- a/libstdc++-v3/config/os/bsd/openbsd/os_defines.h
+++ b/libstdc++-v3/config/os/bsd/openbsd/os_defines.h
@@ -38,4 +38,8 @@
 #define _GLIBCXX_USE_C99_FLOAT_TRANSCENDENTALS_DYNAMIC _GLIBCXX_USE_C99_DYNAMIC
 #define _GLIBCXX_USE_C99_FP_MACROS_DYNAMIC _GLIBCXX_USE_C99_DYNAMIC
 
+// Make std::call_once access pthread_once_t members directly
+#define _GLIBCXX_COMPAT_PTHREAD_ONCE_T \
+  { int _M_done; pthread_mutex_t _M_mutex; }
+
 #endif
diff --git a/libstdc++-v3/config/os/solaris/os_defines.h b/libstdc++-v3/config/os/solaris/os_defines.h
index 36e8cb786cbc..58b7a9f309d4 100644
--- a/libstdc++-v3/config/os/solaris/os_defines.h
+++ b/libstdc++-v3/config/os/solaris/os_defines.h
@@ -35,5 +35,9 @@
 #define __CORRECT_ISO_CPP_WCHAR_H_PROTO
 #endif
 
+// Make std::call_once access pthread_once_t members directly
+#define _GLIBCXX_COMPAT_PTHREAD_ONCE_T \
+  { pthread_mutex_t _M_mutex; unsigned _M_unused; unsigned _M_done; }
+
 #endif
 
diff --git a/libstdc++-v3/include/std/mutex b/libstdc++-v3/include/std/mutex
index 8c7664bff550..6d87834b3bfd 100644
--- a/libstdc++-v3/include/std/mutex
+++ b/libstdc++-v3/include/std/mutex
@@ -684,6 +684,18 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION
     enum _Bits : int { _Init = 0, _Active = 1, _Done = 2 };
 
     int _M_once = _Bits::_Init;
+#elif defined _GLIBCXX_COMPAT_PTHREAD_ONCE_T
+    struct _Once _GLIBCXX_COMPAT_PTHREAD_ONCE_T;
+    static_assert(sizeof(_Once) == sizeof(__gthread_once_t), "config error");
+    static_assert(alignof(_Once) == alignof(__gthread_once_t), "config error");
+    union
+    {
+      __gthread_once_t _M_native = __GTHREAD_ONCE_INIT;
+      _Once _M_once;
+    };
+#else
+    __gthread_once_t _M_once = __GTHREAD_ONCE_INIT;
+#endif // ! GTHREADS
 
     // Non-blocking query whether this is known to be a passive execution.
     bool
@@ -706,11 +718,8 @@ _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
@@ -753,7 +762,13 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION
   once_flag::_M_exceptional() noexcept
   { _M_once = _Bits::_Init; }
 
-#else // ! FUTEX && GTHREADS
+#elif defined _GLIBCXX_COMPAT_PTHREAD_ONCE_T
+
+  inline bool
+  once_flag::_M_passive() const noexcept
+  { return false; }
+
+#else
 
   /// @cond undocumented
 # ifdef _GLIBCXX_HAVE_TLS
@@ -814,7 +829,8 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION
     void
     call_once(once_flag& __once, _Callable&& __f, _Args&&... __args)
     {
-#if defined _GLIBCXX_HAVE_LINUX_FUTEX || ! defined _GLIBCXX_HAS_GTHREADS
+#if defined _GLIBCXX_HAVE_LINUX_FUTEX || ! defined _GLIBCXX_HAS_GTHREADS \
+      || _GLIBCXX_COMPAT_PTHREAD_ONCE_T
       if (__once._M_passive())
 	return;
       else if (__once._M_activate())
diff --git a/libstdc++-v3/src/c++11/mutex.cc b/libstdc++-v3/src/c++11/mutex.cc
index 286f77f9a454..e63b709749a6 100644
--- a/libstdc++-v3/src/c++11/mutex.cc
+++ b/libstdc++-v3/src/c++11/mutex.cc
@@ -82,6 +82,26 @@ std::once_flag::_M_finish(bool returning) noexcept
     }
 }
 
+#elif defined _GLIBCXX_COMPAT_PTHREAD_ONCE_T
+
+bool
+std::once_flag::_M_activate()
+{
+  pthread_mutex_lock(&_M_once._M_mutex);
+  if (_M_once._M_done == 0)
+    return true;
+  pthread_mutex_unlock(&_M_once._M_mutex);
+  return false;
+}
+
+void
+std::once_flag::_M_finish(bool returning) noexcept
+{
+  if (returning)
+    _M_once._M_done = 1;
+  pthread_mutex_unlock(&_M_once._M_mutex);
+}
+
 #endif // ! FUTEX
 
 #ifndef _GLIBCXX_HAVE_TLS

^ permalink raw reply	[flat|nested] 3+ messages in thread

end of thread, other threads:[~2020-11-03 20:13 UTC | newest]

Thread overview: 3+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-11-03 18:45 [committed] libstdc++: Rewrite std::call_once to use futexes [PR 66146] Jonathan Wakely
2020-11-03 20:04 ` Jonathan Wakely
2020-11-03 20:13 ` Jonathan Wakely

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).