Skip to content

Commit

Permalink
Remove use of std::atomic, leaving comments with rationale, why it is…
Browse files Browse the repository at this point in the history
… not needed.
  • Loading branch information
Ralf W. Grosse-Kunstleve committed Oct 10, 2023
1 parent b2f87a8 commit fad1017
Showing 1 changed file with 12 additions and 7 deletions.
19 changes: 12 additions & 7 deletions include/pybind11/gil_safe_call_once.h
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,6 @@
#include "detail/common.h"
#include "gil.h"

#include <atomic>
#include <cassert>
#include <mutex>

Expand Down Expand Up @@ -49,21 +48,25 @@ class gil_safe_call_once_and_store {
// PRECONDITION: The GIL must be held when `call_once_and_store_result()` is called.
template <typename Callable>
gil_safe_call_once_and_store &call_once_and_store_result(Callable &&fn) {
if (!is_initialized_.load(std::memory_order_acquire)) {
gil_scoped_release gil_rel;
if (!is_initialized_) { // This read is guarded by the GIL.
// Multiple threads may enter here, because CPython API calls in the
// `fn()` call below may release and reacquire the GIL.
gil_scoped_release gil_rel; // Needed to establish lock ordering.
std::call_once(once_flag_, [&] {
// Only one thread will ever enter here.
gil_scoped_acquire gil_acq;
::new (storage_) T(fn());
is_initialized_.store(true, std::memory_order_release);
is_initialized_ = true; // This write is guarded by the GIL.
});
}
// Intentionally not returning `T &` to ensure the calling code is self-documenting.
return *this;
}

// This must only be called after `call_once_and_store()` was called.
// This must only be called after `call_once_and_store_result()` was called.
// Not const for simplicity. (Could be made const if there is an unforeseen need.)
T &get_stored() {
assert(is_initialized_.load(std::memory_order_relaxed));
assert(is_initialized_);
PYBIND11_WARNING_PUSH
#if !defined(__clang__) && defined(__GNUC__) && __GNUC__ < 5
// Needed for gcc 4.8.5
Expand All @@ -77,9 +80,11 @@ class gil_safe_call_once_and_store {
PYBIND11_DTOR_CONSTEXPR ~gil_safe_call_once_and_store() = default;

private:
// `is_initialized_` below and `storage_` here can be replaced with `std::optional`
// when pybind11 drops C++11 support.
alignas(T) char storage_[sizeof(T)] = {};
std::once_flag once_flag_ = {};
std::atomic<bool> is_initialized_ = {};
bool is_initialized_ = false;
};

PYBIND11_NAMESPACE_END(PYBIND11_NAMESPACE)

0 comments on commit fad1017

Please sign in to comment.