[gcc(refs/users/redi/heads/pr92895)] Implement synchronization for stop_callback destructor
Jonathan Wakely
redi@gcc.gnu.org
Thu Jan 23 00:36:00 GMT 2020
https://gcc.gnu.org/g:84965830c12e3cbff716a11ca6dee72096f10d73
commit 84965830c12e3cbff716a11ca6dee72096f10d73
Author: Jonathan Wakely <jwakely@redhat.com>
Date: Thu Jan 23 00:33:57 2020 +0000
Implement synchronization for stop_callback destructor
Diff:
---
libstdc++-v3/include/std/stop_token | 80 +++++++++++++++++++++++++++++++++----
1 file changed, 73 insertions(+), 7 deletions(-)
diff --git a/libstdc++-v3/include/std/stop_token b/libstdc++-v3/include/std/stop_token
index 61c1867..16355cd 100644
--- a/libstdc++-v3/include/std/stop_token
+++ b/libstdc++-v3/include/std/stop_token
@@ -36,9 +36,11 @@
#ifdef _GLIBCXX_HAS_GTHREADS
# define __cpp_lib_jthread 201907L
# include <bits/gthr.h>
+# if __has_include(<semaphore>)
+# include <semaphore>
+# endif
#endif
-
namespace std _GLIBCXX_VISIBILITY(default)
{
_GLIBCXX_BEGIN_NAMESPACE_VERSION
@@ -98,12 +100,43 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION
template<typename _Callback>
friend class stop_callback;
+#ifndef __cpp_lib_semaphore
+ // TODO: replace this with a real implementation of std::binary_semaphore
+ struct binary_semaphore
+ {
+ explicit binary_semaphore(int __d) : _M_counter(__d > 0) { }
+
+ void release() { _M_counter.fetch_add(1, memory_order::release); }
+
+ void acquire()
+ {
+ int __old = 1;
+ while (_M_counter.compare_exchange_weak(__old, 0,
+ memory_order::acquire,
+ memory_order::relaxed))
+ {
+ __old = 1;
+ if constexpr (__has_builtin(__builtin_ia32_pause))
+ __builtin_ia32_pause();
+#ifdef _GLIBCXX_USE_SCHED_YIELD
+ else
+ __gthread_yield();
+#endif
+ }
+ }
+
+ atomic<int> _M_counter;
+ };
+#endif
+
struct _Stop_cb
{
using __cb_type = void(_Stop_cb*) noexcept;
__cb_type* _M_callback;
_Stop_cb* _M_prev = nullptr;
_Stop_cb* _M_next = nullptr;
+ bool* _M_destroyed = nullptr;
+ binary_semaphore _M_done{0};
[[__gnu__::__nonnull__]]
explicit
@@ -124,6 +157,9 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION
std::atomic<value_type> _M_owners{1};
std::atomic<value_type> _M_value{_S_ssrc_counter_inc};
_Stop_cb* _M_head = nullptr;
+#if _GLIBCXX_HAS_GTHREADS
+ __gthread_t _M_requester;
+#endif
_Stop_state_t() = default;
@@ -196,6 +232,10 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION
}
while (!_M_try_lock_and_stop(__old));
+#if _GLIBCXX_HAS_GTHREADS
+ _M_requester = __gthread_self();
+#endif
+
while (_M_head)
{
bool __last_cb;
@@ -212,12 +252,20 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION
// Allow other callbacks to be unregistered while __cb runs.
_M_unlock();
+ bool __destroyed = false;
+ __cb->_M_destroyed = &__destroyed;
+
// run callback
- // TODO: synchronize with owning stop_callback's destructor
__cb->_M_run();
- // TODO: check if __cb is still accessible, might be destroyed
- __cb->_M_prev = __cb->_M_next = nullptr;
+ if (!__destroyed)
+ {
+ __cb->_M_destroyed = nullptr;
+#if _GLIBCXX_HAS_GTHREADS
+ // synchronize with destructor of stop_callback that owns *__cb
+ __cb->_M_done.release();
+#endif
+ }
// Avoid relocking if we already know there are no more callbacks.
if (__last_cb)
@@ -245,7 +293,9 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION
if (__old < _S_ssrc_counter_inc) // no stop_source owns *this
// No need to register callback if no stop request can be made.
- return true;
+ // Returning false also means the stop_callback does not share
+ // ownership of this state, but that's not observable.
+ return false;
}
while (!_M_try_lock(__old));
@@ -259,6 +309,7 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION
return true;
}
+ // Called by ~stop_callback just before destroying *__cb.
[[__gnu__::__nonnull__]]
void
_M_remove_callback(_Stop_cb* __cb)
@@ -284,8 +335,23 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION
_M_unlock();
- // Callback is not in the list, so must be currently executing.
- // TODO: synchronize with completion of callback
+ // Callback is not in the list, so must have been removed by a call to
+ // _M_request_stop.
+
+#if _GLIBCXX_HAS_GTHREADS
+ // Despite appearances there is no data race on _M_requester. The only
+ // write to it happens before the callback is removed from the list,
+ // and removing it from the list happens before this read.
+ if (_M_requester != __gthread_self())
+ {
+ // Synchronize with completion of callback.
+ __cb->_M_done.acquire();
+ // Safe for ~stop_callback to destroy *__cb now.
+ return;
+ }
+#endif
+ if (__cb->_M_destroyed)
+ *__cb->_M_destroyed = true;
}
// Try to obtain the lock.
More information about the Libstdc++-cvs
mailing list