Skip to content

Commit e741760

Browse files
committed
Make ~gil_safe_call_once_and_store a no-op
1 parent d2b7605 commit e741760

File tree

1 file changed

+21
-36
lines changed

1 file changed

+21
-36
lines changed

include/pybind11/gil_safe_call_once.h

Lines changed: 21 additions & 36 deletions
Original file line numberDiff line numberDiff line change
@@ -60,18 +60,16 @@ class gil_safe_call_once_and_store {
6060
// PRECONDITION: The GIL must be held when `call_once_and_store_result()` is called.
6161
template <typename Callable>
6262
gil_safe_call_once_and_store &call_once_and_store_result(Callable &&fn,
63-
void (*finalize_fn)(T &) = nullptr) {
64-
63+
void (*)(T &) /*unused*/ = nullptr) {
6564
if (!is_initialized_) { // This read is guarded by the GIL.
6665
// Multiple threads may enter here, because the GIL is released in the next line and
6766
// CPython API calls in the `fn()` call below may release and reacquire the GIL.
6867
gil_scoped_release gil_rel; // Needed to establish lock ordering.
6968
std::call_once(once_flag_, [&] {
7069
// Only one thread will ever enter here.
7170
gil_scoped_acquire gil_acq;
72-
::new (storage_) T(fn()); // fn may release, but will reacquire, the GIL.
73-
finalize_fn_ = finalize_fn; // Store the finalizer.
74-
is_initialized_ = true; // This write is guarded by the GIL.
71+
::new (storage_) T(fn()); // fn may release, but will reacquire, the GIL.
72+
is_initialized_ = true; // This write is guarded by the GIL.
7573
});
7674
// All threads will observe `is_initialized_` as true here.
7775
}
@@ -92,17 +90,15 @@ class gil_safe_call_once_and_store {
9290
}
9391

9492
constexpr gil_safe_call_once_and_store() = default;
95-
PYBIND11_DTOR_CONSTEXPR ~gil_safe_call_once_and_store() {
96-
if (is_initialized_ && finalize_fn_ != nullptr) {
97-
finalize_fn_(*reinterpret_cast<T *>(storage_));
98-
}
99-
}
93+
// The instance is a global static, so its destructor runs when the process
94+
// is terminating. Therefore, do nothing here because the Python interpreter
95+
// may have been finalized already.
96+
PYBIND11_DTOR_CONSTEXPR ~gil_safe_call_once_and_store() = default;
10097

10198
private:
10299
// Global static storage (per process) when subinterpreter support is disabled.
103100
alignas(T) char storage_[sizeof(T)] = {};
104101
std::once_flag once_flag_;
105-
void (*finalize_fn_)(T &) = nullptr;
106102

107103
// The `is_initialized_`-`storage_` pair is very similar to `std::optional`,
108104
// but the latter does not have the triviality properties of former,
@@ -124,19 +120,19 @@ class gil_safe_call_once_and_store {
124120
if (!is_initialized_by_atleast_one_interpreter_
125121
|| detail::get_num_interpreters_seen() > 1) {
126122
detail::with_internals([&](detail::internals &internals) {
127-
const void *key = reinterpret_cast<const void *>(this);
123+
const void *k = reinterpret_cast<const void *>(this);
128124
auto &storage_map = internals.call_once_storage_map;
129-
auto it = storage_map.find(key);
125+
auto it = storage_map.find(k);
130126
if (it == storage_map.end()) {
131127
gil_scoped_release gil_rel; // Needed to establish lock ordering.
132128
{
133129
// Only one thread will ever enter here.
134130
gil_scoped_acquire gil_acq;
135-
auto s = new detail::call_once_storage<T>{};
136-
::new (s->storage) T(fn()); // fn may release, but will reacquire, the GIL.
137-
s->finalize = finalize_fn;
138-
last_storage_ = reinterpret_cast<T *>(s->storage);
139-
storage_map.emplace(key, s);
131+
auto v = new detail::call_once_storage<T>{};
132+
::new (v->storage) T(fn()); // fn may release, but will reacquire, the GIL.
133+
v->finalize = finalize_fn;
134+
last_storage_ = reinterpret_cast<T *>(v->storage);
135+
storage_map.emplace(k, v);
140136
};
141137
}
142138
is_initialized_by_atleast_one_interpreter_ = true;
@@ -153,32 +149,21 @@ class gil_safe_call_once_and_store {
153149
if (!is_initialized_by_atleast_one_interpreter_
154150
|| detail::get_num_interpreters_seen() > 1) {
155151
detail::with_internals([&](detail::internals &internals) {
156-
const void *key = reinterpret_cast<const void *>(this);
152+
const void *k = reinterpret_cast<const void *>(this);
157153
auto &storage_map = internals.call_once_storage_map;
158-
auto it = storage_map.find(key);
159-
assert(it != storage_map.end());
160-
auto *s = static_cast<detail::call_once_storage<T> *>(it->second);
161-
result = last_storage_ = reinterpret_cast<T *>(s->storage);
154+
auto *v = static_cast<detail::call_once_storage<T> *>(storage_map.at(k));
155+
result = last_storage_ = reinterpret_cast<T *>(v->storage);
162156
});
163157
}
164158
assert(result != nullptr);
165159
return *result;
166160
}
167161

168162
constexpr gil_safe_call_once_and_store() = default;
169-
PYBIND11_DTOR_CONSTEXPR ~gil_safe_call_once_and_store() {
170-
if (is_initialized_by_atleast_one_interpreter_) {
171-
detail::with_internals([&](detail::internals &internals) {
172-
const void *key = reinterpret_cast<const void *>(this);
173-
auto &storage_map = internals.call_once_storage_map;
174-
auto it = storage_map.find(key);
175-
if (it != storage_map.end()) {
176-
delete it->second;
177-
storage_map.erase(it);
178-
}
179-
});
180-
}
181-
}
163+
// The instance is a global static, so its destructor runs when the process
164+
// is terminating. Therefore, do nothing here because the Python interpreter
165+
// may have been finalized already.
166+
PYBIND11_DTOR_CONSTEXPR ~gil_safe_call_once_and_store() = default;
182167

183168
private:
184169
// No storage needed when subinterpreter support is enabled.

0 commit comments

Comments
 (0)