[RFA/C] Reimplementation of std::call_once.

[RFA/C] Reimplementation of std::call_once.

[Iain Sandoe]

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

Hello.

With the current [posix-y] implementation of std::call_once, we are seeing significant issues for Darwin. The issues vary in strength from “hangs or segvs sometimes depending on optimisation and the day of the week” (newer OS, faster h/w) to “segvs or std::terminates every time” older OS slower h/w.

Actually, I think that in the presence of exceptions it is not working on Linux either.

As a use-case: this problem prevents us from building working LLVM/clang tools on Darwin using GCC as the bootstrap compiler.

the test code I am using is: https://godbolt.org/z/Mxcnv9YEx (derived from a cppreference example, IIRC).

——

So I was investigating what/why there might be issues and...

 *  ISTM that using the underlying pthread_once implementation is not playing at all well with exceptions (since pthread_once is oblivious to such).
 * the current implementation cannot support nested cases.

——

Here is a possible replacement implementation with some advantages:

 * does not require TLS, it is all stack-based.
 * does not use global state (so it can support nested cases).
 * it actually works reliably in real use-cases (on old and new Darwin, at least).

And one immediately noted disadvantage:
 * that once_flag is significantly larger with this implementation (since it gains a mutex and a condition var).

-----

This is a prototype, so I’m looking for advice/comment on:

 (a) if this can be generalised to be part of libstdc++
 (b) what that would take ... 
 (c) ..or if I should figure out how to make this a darwin-specific impl.

——  here is the prototype implementation as a non-patch .. (patch attached).
 
mutex:

#ifdef _GLIBCXX_HAS_GTHREADS

  /// Flag type used by std::call_once
  struct once_flag
  {
    /// Constructor
    constexpr once_flag()  noexcept
    : _M_state_(0), _M_mutx_(__GTHREAD_MUTEX_INIT),
      _M_condv_(__GTHREAD_COND_INIT) {}

    void __do_call_once(void (*)(void*), void*);

    template<typename _Callable, typename... _Args>
      friend void
      call_once(once_flag& __once, _Callable&& __f, _Args&&... __args);

  private:
    __gthread_mutex_t _M_mutx_;
    __gthread_cond_t _M_condv_;
    // call state: 0 = init, 1 = someone is trying, 2 = done.
    atomic_uint _M_state_;

    /// Deleted copy constructor
    once_flag(const once_flag&) = delete;
    /// Deleted assignment operator
    once_flag& operator=(const once_flag&) = delete;
  };

  /// Invoke a callable and synchronize with other calls using the same flag
  template<typename _Callable, typename... _Args>
  void
  call_once (once_flag& __flag, _Callable&& __f, _Args&&... __args)
  {
    if (__flag._M_state_.load (std::memory_order_acquire) == 2)
      return;

    // Closure type that runs the original function with the supplied args.
    auto __callable = [&] {
	  std::__invoke(std::forward<_Callable>(__f),
			std::forward<_Args>(__args)...);
    };
    // Trampoline to call the actual fn; we will pass in the closure address.
    void (*__oc_tramp)(void*)
      = [] (void *ca) { (*static_cast<decltype(__callable)*>(ca))(); };
    // Attempt to do it and synchronize with any other threads that are also
    // trying.
    __flag.__do_call_once (__oc_tramp, std::__addressof(__callable));
}

#else // _GLIBCXX_HAS_GTHREADS

—— mutex.cc:

// This calls the trampoline lambda, passing the address of the closure
// repesenting the original function and its arguments.
void
once_flag::__do_call_once (void (*func)(void*), void *arg)
{
  __gthread_mutex_lock(&_M_mutx_);
  while (this->_M_state_.load (std::memory_order_relaxed) == 1)
    __gthread_cond_wait(&_M_condv_, &_M_mutx_);

  // mutex locked, the most likely outcome is that the once-call completed
  // on some other thread, so we are done.
  if (_M_state_.load (std::memory_order_acquire) == 2)
    {
      __gthread_mutex_unlock(&_M_mutx_);
      return;
    }

  // mutex locked; if we get here, we expect the state to be 0, this would
  // correspond to an exception throw by the previous thread that tried to
  // do the once_call.
  __glibcxx_assert (_M_state_.load (std::memory_order_acquire) == 0);

  try
    {
      // mutex locked.
      _M_state_.store (1, std::memory_order_relaxed);
      __gthread_mutex_unlock (&_M_mutx_);
      func (arg);
      // We got here without an exception, so the call is done.
      // If the underlying implementation is pthreads, then it is possible
      // to trigger a sequence of events where wake-ups are lost - unless the
      // mutex associated with the condition var is locked around the relevant
      // broadcast (or signal).
      __gthread_mutex_lock(&_M_mutx_);
      _M_state_.store (2, std::memory_order_release);
      __gthread_cond_broadcast (&_M_condv_);
      __gthread_mutex_unlock (&_M_mutx_);
    }
  catch (...)
    {
      // mutex unlocked.
      // func raised an exception, let someone else try ...
      // See above.
      __gthread_mutex_lock(&_M_mutx_);
      _M_state_.store (0, std::memory_order_release);
      __gthread_cond_broadcast (&_M_condv_);
      __gthread_mutex_unlock (&_M_mutx_);
      // ... and pass the exeception to our caller.
      throw;
    }
}

===== 
the implementation in mutex.cc can sit togethe with the old version so that the symbols for that remain available (or, for versioned libraries, the old code can be deleted).

thanks
Iain


[-- Attachment #2: 0001-libstdc-Possible-reimplementation-of-std-call_once.patch --]
[-- Type: application/octet-stream, Size: 9629 bytes --]

From 5b08bec16094cd6937e8b8473f24289618adcfc2 Mon Sep 17 00:00:00 2001
From: Iain Sandoe <iain@sandoe.co.uk>
Date: Wed, 9 Aug 2023 12:13:54 +0100
Subject: [PATCH] libstdc++: Possible reimplementation of std::call_once.

This reimplements std::call_once using a stack-based approach which
does not depend on pthread_once for its action.

Signed-off-by: Iain Sandoe <iain@sandoe.co.uk>

libstdc++-v3/ChangeLog:

	* config/abi/pre/gnu.ver: Export once_flag::__do_call_once.
	* include/std/mutex: Reimplement std::call_once for threaded cases.
	* src/c++11/mutex.cc (once_flag::__do_call_once): New.
---
 libstdc++-v3/config/abi/pre/gnu.ver |   2 +
 libstdc++-v3/include/std/mutex      | 127 +++++++---------------------
 libstdc++-v3/src/c++11/mutex.cc     |  60 ++++++++++++-
 3 files changed, 92 insertions(+), 97 deletions(-)

diff --git a/libstdc++-v3/config/abi/pre/gnu.ver b/libstdc++-v3/config/abi/pre/gnu.ver
index a2e5f3b4e74..c34058d5ed7 100644
--- a/libstdc++-v3/config/abi/pre/gnu.ver
+++ b/libstdc++-v3/config/abi/pre/gnu.ver
@@ -1315,6 +1315,8 @@ GLIBCXX_3.4.11 {
     _ZNKSt10lock_error4whatEv;
 
     _ZSt11__once_call;
+    _ZNSt9once_flag14__do_call_onceEPFvPvES0_;
+    # old implementation
     _ZSt15__once_callable;
     _ZSt14__once_functor;
     _ZSt23__get_once_functor_lockv;
diff --git a/libstdc++-v3/include/std/mutex b/libstdc++-v3/include/std/mutex
index 2b0059fcfe8..f47fe9e9459 100644
--- a/libstdc++-v3/include/std/mutex
+++ b/libstdc++-v3/include/std/mutex
@@ -50,6 +50,7 @@
 # include <condition_variable>
 # include <thread>
 #endif
+#include <atomic>
 #include <ext/atomicity.h>     // __gnu_cxx::__is_single_threaded
 
 #if defined _GLIBCXX_HAS_GTHREADS && ! defined _GLIBCXX_HAVE_TLS
@@ -796,119 +797,53 @@ _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;
+    /// Constructor
+    constexpr once_flag()  noexcept
+    : _M_state_(0), _M_mutx_(__GTHREAD_MUTEX_INIT),
+      _M_condv_(__GTHREAD_COND_INIT) {}
 
-    struct _Prepare_execution;
+    void __do_call_once(void (*)(void*), void*);
 
     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()};
+    __gthread_mutex_t _M_mutx_;
+    __gthread_cond_t _M_condv_;
+    // call state: 0 = init, 1 = someone is trying, 2 = done.
+    atomic_uint _M_state_;
 
-    _Prepare_execution(const _Prepare_execution&) = delete;
-    _Prepare_execution& operator=(const _Prepare_execution&) = delete;
+    /// Deleted copy constructor
+    once_flag(const once_flag&) = delete;
+    /// Deleted assignment operator
+    once_flag& operator=(const once_flag&) = 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 = [&] {
+  void
+  call_once (once_flag& __flag, _Callable&& __f, _Args&&... __args)
+  {
+    if (__flag._M_state_.load (std::memory_order_acquire) == 2)
+      return;
+
+    // Closure type that runs the original function with the supplied args.
+    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);
-    }
+    };
+    // Trampoline to call the actual fn; we will pass in the closure address.
+    void (*__oc_tramp)(void*)
+      = [] (void *ca) { (*static_cast<decltype(__callable)*>(ca))(); };
+    // Attempt to do it and synchronize with any other threads that are also
+    // trying.
+    __flag.__do_call_once (__oc_tramp, std::__addressof(__callable));
+}
 
 #else // _GLIBCXX_HAS_GTHREADS
 
diff --git a/libstdc++-v3/src/c++11/mutex.cc b/libstdc++-v3/src/c++11/mutex.cc
index 1b1caa3bf0f..948a4b1b387 100644
--- a/libstdc++-v3/src/c++11/mutex.cc
+++ b/libstdc++-v3/src/c++11/mutex.cc
@@ -22,6 +22,7 @@
 // see the files COPYING3 and COPYING.RUNTIME respectively.  If not, see
 // <http://www.gnu.org/licenses/>.
 
+#include <atomic>
 #include <mutex>
 
 #ifdef _GLIBCXX_HAS_GTHREADS
@@ -30,6 +31,63 @@ namespace std _GLIBCXX_VISIBILITY(default)
 {
 _GLIBCXX_BEGIN_NAMESPACE_VERSION
 
+// This calls the trampoline lambda, passing the address of the closure
+// repesenting the original function and its arguments.
+void
+once_flag::__do_call_once (void (*func)(void*), void *arg)
+{
+  __gthread_mutex_lock(&_M_mutx_);
+  while (this->_M_state_.load (std::memory_order_relaxed) == 1)
+    __gthread_cond_wait(&_M_condv_, &_M_mutx_);
+
+  // mutex locked, the most likely outcome is that the once-call completed
+  // on some other thread, so we are done.
+  if (_M_state_.load (std::memory_order_acquire) == 2)
+    {
+      __gthread_mutex_unlock(&_M_mutx_);
+      return;
+    }
+
+  // mutex locked; if we get here, we expect the state to be 0, this would
+  // correspond to an exception throw by the previous thread that tried to
+  // do the once_call.
+  __glibcxx_assert (_M_state_.load (std::memory_order_acquire) == 0);
+
+  try
+    {
+      // mutex locked.
+      _M_state_.store (1, std::memory_order_relaxed);
+      __gthread_mutex_unlock (&_M_mutx_);
+      func (arg);
+      // We got here without an exception, so the call is done.
+      // If the underlying implementation is pthreads, then it is possible
+      // to trigger a sequence of events where wake-ups are lost - unless the
+      // mutex associated with the condition var is locked around the relevant
+      // broadcast (or signal).
+      __gthread_mutex_lock(&_M_mutx_);
+      _M_state_.store (2, std::memory_order_release);
+      __gthread_cond_broadcast (&_M_condv_);
+      __gthread_mutex_unlock (&_M_mutx_);
+    }
+  catch (...)
+    {
+      // mutex unlocked.
+      // func raised an exception, let someone else try ...
+      // See above.
+      __gthread_mutex_lock(&_M_mutx_);
+      _M_state_.store (0, std::memory_order_release);
+      __gthread_cond_broadcast (&_M_condv_);
+      __gthread_mutex_unlock (&_M_mutx_);
+      // ... and pass the exeception to our caller.
+      throw;
+    }
+}
+
+#if !_GLIBCXX_INLINE_VERSION
+
+// Unless we have a versioned library, provide the symbols for the previous
+// once call impl.
+
 #ifdef _GLIBCXX_HAVE_TLS
   __thread void* __once_callable;
   __thread void (*__once_call)();
@@ -101,7 +159,7 @@ namespace
     callable();
   }
 #endif // ! TLS
-
+#endif // ! _GLIBCXX_INLINE_VERSION
 _GLIBCXX_END_NAMESPACE_VERSION
 } // namespace std
 
-- 
2.39.2 (Apple Git-143)


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

Re: [RFA/C] Reimplementation of std::call_once.

[Jonathan Wakely]

On Wed, 9 Aug 2023 at 13:22, Iain Sandoe <iain@sandoe.co.uk> wrote:
>
> Hello.
>
> With the current [posix-y] implementation of std::call_once, we are seeing significant issues for Darwin. The issues vary in strength from “hangs or segvs sometimes depending on optimisation and the day of the week” (newer OS, faster h/w) to “segvs or std::terminates every time” older OS slower h/w.
>
> Actually, I think that in the presence of exceptions it is not working on Linux either.
>
> As a use-case: this problem prevents us from building working LLVM/clang tools on Darwin using GCC as the bootstrap compiler.
>
> the test code I am using is: https://godbolt.org/z/Mxcnv9YEx (derived from a cppreference example, IIRC).
>
> ——
>
> So I was investigating what/why there might be issues and...
>
>  *  ISTM that using the underlying pthread_once implementation is not playing at all well with exceptions (since pthread_once is oblivious to such).
>  * the current implementation cannot support nested cases.
>
> ——
>
> Here is a possible replacement implementation with some advantages:
>
>  * does not require TLS, it is all stack-based.
>  * does not use global state (so it can support nested cases).
>  * it actually works reliably in real use-cases (on old and new Darwin, at least).
>
> And one immediately noted disadvantage:
>  * that once_flag is significantly larger with this implementation (since it gains a mutex and a condition var).
>
> -----
>
> This is a prototype, so I’m looking for advice/comment on:
>
>  (a) if this can be generalised to be part of libstdc++
>  (b) what that would take ...
>  (c) ..or if I should figure out how to make this a darwin-specific impl.


It's an ABI break. I already tried to replace std::call_once once, see
r11-4691-g93e79ed391b9c6 and r11-7688-g6ee24638ed0ad5 and PR 99341.



>
> ——  here is the prototype implementation as a non-patch .. (patch attached).
>
> mutex:
>
> #ifdef _GLIBCXX_HAS_GTHREADS
>
>   /// Flag type used by std::call_once
>   struct once_flag
>   {
>     /// Constructor
>     constexpr once_flag()  noexcept
>     : _M_state_(0), _M_mutx_(__GTHREAD_MUTEX_INIT),
>       _M_condv_(__GTHREAD_COND_INIT) {}
>
>     void __do_call_once(void (*)(void*), void*);
>
>     template<typename _Callable, typename... _Args>
>       friend void
>       call_once(once_flag& __once, _Callable&& __f, _Args&&... __args);
>
>   private:
>     __gthread_mutex_t _M_mutx_;
>     __gthread_cond_t _M_condv_;
>     // call state: 0 = init, 1 = someone is trying, 2 = done.
>     atomic_uint _M_state_;
>
>     /// Deleted copy constructor
>     once_flag(const once_flag&) = delete;
>     /// Deleted assignment operator
>     once_flag& operator=(const once_flag&) = delete;
>   };
>
>   /// Invoke a callable and synchronize with other calls using the same flag
>   template<typename _Callable, typename... _Args>
>   void
>   call_once (once_flag& __flag, _Callable&& __f, _Args&&... __args)
>   {
>     if (__flag._M_state_.load (std::memory_order_acquire) == 2)
>       return;
>
>     // Closure type that runs the original function with the supplied args.
>     auto __callable = [&] {
>           std::__invoke(std::forward<_Callable>(__f),
>                         std::forward<_Args>(__args)...);
>     };
>     // Trampoline to call the actual fn; we will pass in the closure address.
>     void (*__oc_tramp)(void*)
>       = [] (void *ca) { (*static_cast<decltype(__callable)*>(ca))(); };
>     // Attempt to do it and synchronize with any other threads that are also
>     // trying.
>     __flag.__do_call_once (__oc_tramp, std::__addressof(__callable));
> }
>
> #else // _GLIBCXX_HAS_GTHREADS
>
> —— mutex.cc:
>
> // This calls the trampoline lambda, passing the address of the closure
> // repesenting the original function and its arguments.
> void
> once_flag::__do_call_once (void (*func)(void*), void *arg)
> {
>   __gthread_mutex_lock(&_M_mutx_);
>   while (this->_M_state_.load (std::memory_order_relaxed) == 1)
>     __gthread_cond_wait(&_M_condv_, &_M_mutx_);
>
>   // mutex locked, the most likely outcome is that the once-call completed
>   // on some other thread, so we are done.
>   if (_M_state_.load (std::memory_order_acquire) == 2)
>     {
>       __gthread_mutex_unlock(&_M_mutx_);
>       return;
>     }
>
>   // mutex locked; if we get here, we expect the state to be 0, this would
>   // correspond to an exception throw by the previous thread that tried to
>   // do the once_call.
>   __glibcxx_assert (_M_state_.load (std::memory_order_acquire) == 0);
>
>   try
>     {
>       // mutex locked.
>       _M_state_.store (1, std::memory_order_relaxed);
>       __gthread_mutex_unlock (&_M_mutx_);
>       func (arg);
>       // We got here without an exception, so the call is done.
>       // If the underlying implementation is pthreads, then it is possible
>       // to trigger a sequence of events where wake-ups are lost - unless the
>       // mutex associated with the condition var is locked around the relevant
>       // broadcast (or signal).
>       __gthread_mutex_lock(&_M_mutx_);
>       _M_state_.store (2, std::memory_order_release);
>       __gthread_cond_broadcast (&_M_condv_);
>       __gthread_mutex_unlock (&_M_mutx_);
>     }
>   catch (...)
>     {
>       // mutex unlocked.
>       // func raised an exception, let someone else try ...
>       // See above.
>       __gthread_mutex_lock(&_M_mutx_);
>       _M_state_.store (0, std::memory_order_release);
>       __gthread_cond_broadcast (&_M_condv_);
>       __gthread_mutex_unlock (&_M_mutx_);
>       // ... and pass the exeception to our caller.
>       throw;
>     }
> }
>
> =====
> the implementation in mutex.cc can sit togethe with the old version so that the symbols for that remain available (or, for versioned libraries, the old code can be deleted).

If you have old and new code sharing a std::once_flag object you're dead.

Something like the abi_tag transition for std::string in GCC 5 would be needed.

Re: [RFA/C] Reimplementation of std::call_once.

[Iain Sandoe]

Hi Jonathan,

> On 9 Aug 2023, at 13:38, Jonathan Wakely <jwakely.gcc@gmail.com> wrote:
> 
> On Wed, 9 Aug 2023 at 13:22, Iain Sandoe <iain@sandoe.co.uk> wrote:
>> 
>> Hello.
>> 
>> With the current [posix-y] implementation of std::call_once, we are seeing significant issues for Darwin. The issues vary in strength from “hangs or segvs sometimes depending on optimisation and the day of the week” (newer OS, faster h/w) to “segvs or std::terminates every time” older OS slower h/w.
>> 
>> Actually, I think that in the presence of exceptions it is not working on Linux either.
>> 
>> As a use-case: this problem prevents us from building working LLVM/clang tools on Darwin using GCC as the bootstrap compiler.
>> 
>> the test code I am using is: https://godbolt.org/z/Mxcnv9YEx (derived from a cppreference example, IIRC).
>> 

>> This is a prototype, so I’m looking for advice/comment on:
>> 
>> (a) if this can be generalised to be part of libstdc++
>> (b) what that would take ...
>> (c) ..or if I should figure out how to make this a darwin-specific impl.
> 
> 
> It's an ABI break. I already tried to replace std::call_once once, see
> r11-4691-g93e79ed391b9c6 and r11-7688-g6ee24638ed0ad5 and PR 99341.

blast.

>> ——  here is the prototype implementation as a non-patch .. (patch attached).
>> 

>> =====
>> the implementation in mutex.cc can sit togethe with the old version so that the symbols for that remain available (or, for versioned libraries, the old code can be deleted).
> 
> If you have old and new code sharing a std::once_flag object you're dead.

indeed - I was only considering the case where existing binaries needed to run with the new lib, not the case where code compiled with two different impls was combined.

> Something like the abi_tag transition for std::string in GCC 5 would be needed.

That sounds quite complicated and likely to produce similar pain?

====

Presumably an alternative is that I just have to accept that Darwin needs to use a versioned library (which is a direction I am close to heading in because of co-existence of mulitple c++ runtimes anyway).

thanks
Iain

Re: [RFA/C] Reimplementation of std::call_once.

[Jonathan Wakely]

On Wed, 9 Aug 2023 at 13:51, Iain Sandoe <iain@sandoe.co.uk> wrote:
>
> Hi Jonathan,
>
> > On 9 Aug 2023, at 13:38, Jonathan Wakely <jwakely.gcc@gmail.com> wrote:
> >
> > On Wed, 9 Aug 2023 at 13:22, Iain Sandoe <iain@sandoe.co.uk> wrote:
> >>
> >> Hello.
> >>
> >> With the current [posix-y] implementation of std::call_once, we are seeing significant issues for Darwin. The issues vary in strength from “hangs or segvs sometimes depending on optimisation and the day of the week” (newer OS, faster h/w) to “segvs or std::terminates every time” older OS slower h/w.
> >>
> >> Actually, I think that in the presence of exceptions it is not working on Linux either.
> >>
> >> As a use-case: this problem prevents us from building working LLVM/clang tools on Darwin using GCC as the bootstrap compiler.
> >>
> >> the test code I am using is: https://godbolt.org/z/Mxcnv9YEx (derived from a cppreference example, IIRC).
> >>
>
> >> This is a prototype, so I’m looking for advice/comment on:
> >>
> >> (a) if this can be generalised to be part of libstdc++
> >> (b) what that would take ...
> >> (c) ..or if I should figure out how to make this a darwin-specific impl.
> >
> >
> > It's an ABI break. I already tried to replace std::call_once once, see
> > r11-4691-g93e79ed391b9c6 and r11-7688-g6ee24638ed0ad5 and PR 99341.
>
> blast.
>
> >> ——  here is the prototype implementation as a non-patch .. (patch attached).
> >>
>
> >> =====
> >> the implementation in mutex.cc can sit togethe with the old version so that the symbols for that remain available (or, for versioned libraries, the old code can be deleted).
> >
> > If you have old and new code sharing a std::once_flag object you're dead.
>
> indeed - I was only considering the case where existing binaries needed to run with the new lib, not the case where code compiled with two different impls was combined.
>
> > Something like the abi_tag transition for std::string in GCC 5 would be needed.
>
> That sounds quite complicated and likely to produce similar pain?

Not nearly as complicated (since we don't use std::call_once
throughout the entire library) but it would still cause pain for the
ecosystem.

>
> ====
>
> Presumably an alternative is that I just have to accept that Darwin needs to use a versioned library (which is a direction I am close to heading in because of co-existence of mulitple c++ runtimes anyway).

We do need to fix PR 99341 somehow, I just don't know how.

Re: [RFA/C] Reimplementation of std::call_once.

[Iain Sandoe]

> On 9 Aug 2023, at 13:53, Jonathan Wakely <jwakely.gcc@gmail.com> wrote:
> 
> On Wed, 9 Aug 2023 at 13:51, Iain Sandoe <iain@sandoe.co.uk> wrote:
>> 
>>> On 9 Aug 2023, at 13:38, Jonathan Wakely <jwakely.gcc@gmail.com> wrote:
>>> 
>>> On Wed, 9 Aug 2023 at 13:22, Iain Sandoe <iain@sandoe.co.uk> wrote:
>>>> =====
>>>> the implementation in mutex.cc can sit togethe with the old version so that the symbols for that remain available (or, for versioned libraries, the old code can be deleted).
>>> 
>>> If you have old and new code sharing a std::once_flag object you're dead.
>> 
>> indeed - I was only considering the case where existing binaries needed to run with the new lib, not the case where code compiled with two different impls was combined.
>> 
>>> Something like the abi_tag transition for std::string in GCC 5 would be needed.
>> 
>> That sounds quite complicated and likely to produce similar pain?
> 
> Not nearly as complicated (since we don't use std::call_once
> throughout the entire library) but it would still cause pain for the
> ecosystem.

I was, at one stage, considering the idea of [the new impl] copying the trampoline lambda address to the (old) => __once_call and the closure address to (old) __once_callable and then amending __once_proxy to handle this.

It would still be broken w.r.t. nested call_once cases, but no more broken than existing.

However, that means lying about the signature of the old __once_call.

.. and it does not solve the issue that the size of the once_flag has changed.

>> ====
>> 
>> Presumably an alternative is that I just have to accept that Darwin needs to use a versioned library (which is a direction I am close to heading in because of co-existence of mulitple c++ runtimes anyway).
> 
> We do need to fix PR 99341 somehow, I just don't know how.

yeah, tricky.

that PR shows as closed, FWIW - but as noted at the start I think x86_64-linux-gnu is also broken w.r.t once-called fns throwing

Iain

Re: [RFA/C] Reimplementation of std::call_once.

[Jonathan Wakely]

On Wed, 9 Aug 2023 at 14:12, Iain Sandoe <iain@sandoe.co.uk> wrote:
>
>
>
> > On 9 Aug 2023, at 13:53, Jonathan Wakely <jwakely.gcc@gmail.com> wrote:
> >
> > On Wed, 9 Aug 2023 at 13:51, Iain Sandoe <iain@sandoe.co.uk> wrote:
> >>
> >>> On 9 Aug 2023, at 13:38, Jonathan Wakely <jwakely.gcc@gmail.com> wrote:
> >>>
> >>> On Wed, 9 Aug 2023 at 13:22, Iain Sandoe <iain@sandoe.co.uk> wrote:
> >>>> =====
> >>>> the implementation in mutex.cc can sit togethe with the old version so that the symbols for that remain available (or, for versioned libraries, the old code can be deleted).
> >>>
> >>> If you have old and new code sharing a std::once_flag object you're dead.
> >>
> >> indeed - I was only considering the case where existing binaries needed to run with the new lib, not the case where code compiled with two different impls was combined.
> >>
> >>> Something like the abi_tag transition for std::string in GCC 5 would be needed.
> >>
> >> That sounds quite complicated and likely to produce similar pain?
> >
> > Not nearly as complicated (since we don't use std::call_once
> > throughout the entire library) but it would still cause pain for the
> > ecosystem.
>
> I was, at one stage, considering the idea of [the new impl] copying the trampoline lambda address to the (old) => __once_call and the closure address to (old) __once_callable and then amending __once_proxy to handle this.
>
> It would still be broken w.r.t. nested call_once cases, but no more broken than existing.
>
> However, that means lying about the signature of the old __once_call.
>
> .. and it does not solve the issue that the size of the once_flag has changed.
>
> >> ====
> >>
> >> Presumably an alternative is that I just have to accept that Darwin needs to use a versioned library (which is a direction I am close to heading in because of co-existence of mulitple c++ runtimes anyway).
> >
> > We do need to fix PR 99341 somehow, I just don't know how.
>
> yeah, tricky.
>
> that PR shows as closed, FWIW - but as noted at the start I think x86_64-linux-gnu is also broken w.r.t once-called fns throwing

Oh sorry, PR 66146 is the right one.