Hi Jonathan,
thanks for the off-list discussions ..
>>The current implementation as used on (particularly older) Darwin
>>does not function properly. It is possible (even likely) that there
>>are bugs in the pthreads implementations. However, Posix does say
>>"The behaviour of pthread_once() is undefined if once_control has
>>automatic storage duration or is not initialised by PTHREAD_ONCE_INIT."
>>The pthread_once Linux manual page has similar wording.
>>However, at least from my reading, std::once_flag does not make such a
>>requirement
>Correct. I think the original intent was for call_once to be a thin
>wrapper over pthread_once, but that was not actually possible, and was
>a mistake.
>Implementing it with a mutex and condvar makes it very large, the
>optimal implementation is just a futex but we don't have that on all
>targets.
Well, a selling point of doing this is that it is opt-in and currently
restricted to Darwin. A second target with futexes (IDK if Darwin will
ever have these) could just choose to declare the ABI2 as one using them.
....
>>Since this is an ABI break, the new implementation is wrapped in an
>>abi_tag and the library continues to provide the existing symbols, and
>>their implementation.
>Surely the name of the tag and namespace should be "twice" not "once
>v2" (just kidding).
ah... so tempting :)
>I do think ocv2 is a bit cryptic, but keeping the mangled name shorter
>is useful. But I think we're supposed to use reserved names for
>abi-tags because we share the tag "namespace" with user code.
I have re-named it "__c1v2" which is also cryptic, but short.
<snip, various typographical fixes made>
>>+ __gthread_mutex_t _M_mutx_ = __GTHREAD_MUTEX_INIT;
>>+ __gthread_cond_t _M_condv_ = __GTHREAD_COND_INIT;
>We could use std::mutex and std::__condvar here, then use a
>unique_lock to lock it.
As discussed off-list, that is not going to work because ___condvar
has a non-constexpr constructor.
>>+ {
>>+ if (__flag._M_state_ == 2)
>This should be read with the mutex locked, or all accesses to the
>variable should be atomic.
I read some wisdom somewhere that says that atomic access is not enough
in this case, the accesses must be guarded by the mutex (done).
<snip typographical fixes done>
>>+ // 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).
>Is that true?
I do not claim to have such in-depth knowledge of pthread, therefore it must
be input read from the Open pages. When I re-checked yesterday, I could not
find this exact statement - but:
"The pthread_cond_broadcast() or pthread_cond_signal() functions may be
called by a thread whether or not it currently owns the mutex that threads
calling pthread_cond_wait() or pthread_cond_timedwait() have associated
with the condition variable during their waits; however, if predictable
scheduling behavior is required, then that mutex shall be locked by the
thread calling pthread_cond_broadcast() or pthread_cond_signal()."
IDK whether deterministic can be equated to missing wake-ups, but it does
seem plausible - since if a wakeup is sent with no sleepers, it is ignored.
>>+ _M_state_ = 0;
>>+ __gthread_cond_broadcast (&_M_condv_);
>Do we want to wake all waiters here? Only one of them will be able
>acquire the mutex and try the once-call again. We could wake just one,
>I think.
Good point.
I have hopefully caught all the muscle-memory extra spaces between function
names and opening parentheses.
Re-tested on Darwin9 (approx 100 progressions) and Darwin 24 (no changes),
but not asked the macports folks to re-test yet.
As for deployment, we're between a rock and an hard place .. what's there
now fails completely on earlier systems, so a replacement seems essential.
That means a careful roll-out ..
OK for trunk?
thanks
Iain
--- 8< ---
The current implementation as used on (particularly older) Darwin
does not function properly. It is possible (even likely) that there
are bugs in the pthreads implementations. However, Posix does say
"The behaviour of pthread_once() is undefined if once_control has
automatic storage duration or is not initialised by PTHREAD_ONCE_INIT."
The pthread_once Linux manual page has similar wording.
However, at least from my reading, std::once_flag does not make such a
requirement (although the examples in the WD and, for example,
cppreference all show suitable file-scope vars in use).
We also have known issues with exceptions in some implementations.
The patch here reimplements std::call_once avoiding pthread_once.
Since this is an ABI break, the new implementation is wrapped in an
abi_tag and the library continues to provide the existing symbols, and
their implementation.
libstdc++-v3/ChangeLog:
* config/abi/pre/gnu.ver: Export __do_call_once.
* config/os/bsd/darwin/os_defines.h
(_GLIBCXX_ONCE_CALL_ABI2): Enable for Darwin.
* include/std/mutex (once_flag): Revised impl.
placed in abi_tag '__c1v2'. (call_once): Likewise.
* src/c++11/mutex.cc
(once_flag::__do_call_once): Revised impl.
Signed-off-by: Iain Sandoe <[email protected]>
---
libstdc++-v3/config/abi/pre/gnu.ver | 3 +
.../config/os/bsd/darwin/os_defines.h | 4 ++
libstdc++-v3/include/std/mutex | 54 +++++++++++++++-
libstdc++-v3/src/c++11/mutex.cc | 61 ++++++++++++++++++-
4 files changed, 120 insertions(+), 2 deletions(-)
diff --git a/libstdc++-v3/config/abi/pre/gnu.ver
b/libstdc++-v3/config/abi/pre/gnu.ver
index 3a6afac8308..c2443143c40 100644
--- a/libstdc++-v3/config/abi/pre/gnu.ver
+++ b/libstdc++-v3/config/abi/pre/gnu.ver
@@ -2632,6 +2632,9 @@ GLIBCXX_3.4.36 {
_ZNSt7__cxx1112basic_stringI[cw]St11char_traitsI[cw]ESaI[cw]EE*_S_allocate_*;
_ZNSt7__cxx1112basic_stringI[cw]St11char_traitsI[cw]ESaI[cw]EE*_M_create_*;
+ # Re-implementation of call_once
+ _ZNSt6__c1v29once_flag14__do_call_onceEPFvPvES1_;
+
} GLIBCXX_3.4.35;
# Symbols in the support library (libsupc++) have their own tag.
diff --git a/libstdc++-v3/config/os/bsd/darwin/os_defines.h
b/libstdc++-v3/config/os/bsd/darwin/os_defines.h
index b6a5b76de21..151ee6cd169 100644
--- a/libstdc++-v3/config/os/bsd/darwin/os_defines.h
+++ b/libstdc++-v3/config/os/bsd/darwin/os_defines.h
@@ -57,4 +57,8 @@
// read(2) can return EINVAL for n >= INT_MAX.
#define _GLIBCXX_MAX_READ_SIZE (__INT_MAX__ - 1)
+// Use the V2 ABI for once_call, the pthreads version does not work for
+// OS versions less than 10.11 (darwin15).
+#define _GLIBCXX_ONCE_CALL_ABI2 1
+
#endif
diff --git a/libstdc++-v3/include/std/mutex b/libstdc++-v3/include/std/mutex
index fb5b5073834..c00615d749d 100644
--- a/libstdc++-v3/include/std/mutex
+++ b/libstdc++-v3/include/std/mutex
@@ -791,6 +791,58 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION
#endif // __cpp_lib_scoped_lock
#ifdef _GLIBCXX_HAS_GTHREADS
+#ifdef _GLIBCXX_ONCE_CALL_ABI2
+ // The revised ABI does not use TLS
+ /// Flag type used by std::call_once
+ inline namespace __c1v2 __attribute__((__abi_tag__ ("__c1v2"))) {
+ struct once_flag
+ {
+ /// Constructor
+ constexpr once_flag() = default;
+
+ /// Deleted copy constructor
+ once_flag(const once_flag&) = delete;
+ /// Deleted assignment operator
+ once_flag& operator=(const once_flag&) = delete;
+
+ private:
+ // call state: 0 = init, 1 = someone is trying, 2 = done.
+ unsigned int _M_state = 0;
+ __gthread_mutex_t _M_mutx = __GTHREAD_MUTEX_INIT;
+ __gthread_cond_t _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);
+ };
+
+ /// 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)
+ {
+ __gthread_mutex_lock(&__flag._M_mutx);
+ bool done = (__flag._M_state == 2);
+ __gthread_mutex_unlock(&__flag._M_mutx);
+ if (done)
+ 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));
+}
+} // namespace __c1v2
+#else // ! _GLIBCXX_ONCE_CALL_ABI2
/// Flag type used by std::call_once
struct once_flag
{
@@ -923,7 +975,7 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION
if (int __e = __gthread_once(&__once._M_once, &__once_proxy))
__throw_system_error(__e);
}
-
+#endif // _GLIBCXX_ONCE_CALL_ABI2
#else // _GLIBCXX_HAS_GTHREADS
/// Flag type used by std::call_once
diff --git a/libstdc++-v3/src/c++11/mutex.cc b/libstdc++-v3/src/c++11/mutex.cc
index 8f04494620b..d6e588f08d0 100644
--- a/libstdc++-v3/src/c++11/mutex.cc
+++ b/libstdc++-v3/src/c++11/mutex.cc
@@ -30,6 +30,66 @@ namespace std _GLIBCXX_VISIBILITY(default)
{
_GLIBCXX_BEGIN_NAMESPACE_VERSION
+#ifdef _GLIBCXX_ONCE_CALL_ABI2
+inline namespace __c1v2 __attribute__((__abi_tag__ ("__c1v2"))) {
+// Version 2 ABI without global state, is callable recursively.
+// 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 (_M_state == 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 == 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 == 0);
+
+ try
+ {
+ // mutex locked.
+ _M_state = 1;
+ __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 = 2;
+ __gthread_cond_signal(&_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 = 0;
+ __gthread_cond_signal(&_M_condv);
+ __gthread_mutex_unlock(&_M_mutx);
+ // ... and pass the exception to our caller.
+ throw;
+ }
+}
+} // namespace __c1v2
+#endif // _GLIBCXX_ONCE_CALL_ABI2
+
+// 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)();
@@ -115,7 +175,6 @@ namespace
callable();
}
#endif // ! TLS
-
_GLIBCXX_END_NAMESPACE_VERSION
} // namespace std
--
2.50.1 (Apple Git-155)
