public inbox for libstdc++@gcc.gnu.org
 help / color / mirror / Atom feed
From: Jonathan Wakely <jwakely.gcc@gmail.com>
To: Iain Sandoe <iain@sandoe.co.uk>
Cc: libstdc++@gcc.gnu.org
Subject: Re: [RFA/C] Reimplementation of std::call_once.
Date: Wed, 9 Aug 2023 13:38:15 +0100	[thread overview]
Message-ID: <CAH6eHdRQy=zZ2OWAfcBcp6bGjfX9b17gqWjCOA5rGO9XQ+3ycA@mail.gmail.com> (raw)
In-Reply-To: <CB28304D-021F-4CBC-82CC-A6CAE412CFF5@sandoe.co.uk>

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.

  reply	other threads:[~2023-08-09 12:38 UTC|newest]

Thread overview: 6+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2023-08-09 12:21 Iain Sandoe
2023-08-09 12:38 ` Jonathan Wakely [this message]
2023-08-09 12:51   ` Iain Sandoe
2023-08-09 12:53     ` Jonathan Wakely
2023-08-09 13:12       ` Iain Sandoe
2023-08-09 14:10         ` Jonathan Wakely

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to='CAH6eHdRQy=zZ2OWAfcBcp6bGjfX9b17gqWjCOA5rGO9XQ+3ycA@mail.gmail.com' \
    --to=jwakely.gcc@gmail.com \
    --cc=iain@sandoe.co.uk \
    --cc=libstdc++@gcc.gnu.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
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).