Skip to content

Commit 6f8308b

Browse files
committed
Add a shutdown method to internals.
shutdown can safely DECREF Python objects owned by the internals.
1 parent ca1d996 commit 6f8308b

File tree

2 files changed

+34
-17
lines changed

2 files changed

+34
-17
lines changed

include/pybind11/detail/internals.h

Lines changed: 31 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -308,7 +308,16 @@ struct internals {
308308
internals(internals &&other) = delete;
309309
internals &operator=(const internals &other) = delete;
310310
internals &operator=(internals &&other) = delete;
311-
~internals() = default;
311+
~internals() = default; // NOTE: destruct/decref python objects in shutdown()
312+
313+
/// shutdown is run during interpreter finalization and can (carefully) interact with Python.
314+
void shutdown() {
315+
Py_XDECREF(static_property_type);
316+
static_property_type = nullptr;
317+
318+
Py_XDECREF(default_metaclass);
319+
default_metaclass = nullptr;
320+
}
312321
};
313322

314323
// the internals struct (above) is shared between all the modules. local_internals are only
@@ -325,6 +334,12 @@ struct local_internals {
325334

326335
std::forward_list<ExceptionTranslator> registered_exception_translators;
327336
PyTypeObject *function_record_py_type = nullptr;
337+
338+
/// shutdown is run during interpreter finalization and can (carefully) interact with Python.
339+
void shutdown() {
340+
Py_XDECREF(function_record_py_type);
341+
function_record_py_type = nullptr;
342+
}
328343
};
329344

330345
enum class holder_enum_t : uint8_t {
@@ -569,7 +584,7 @@ inline object get_python_state_dict() {
569584
// The bool follows std::map::insert convention: true = created, false = existed.
570585
template <typename Payload>
571586
std::pair<Payload *, bool> atomic_get_or_create_in_state_dict(const char *key,
572-
bool clear_destructor = false) {
587+
void (*dtor)(void *) = nullptr) {
573588
error_scope err_scope; // preserve any existing Python error states
574589

575590
auto state_dict = reinterpret_borrow<dict>(get_python_state_dict());
@@ -595,7 +610,7 @@ std::pair<Payload *, bool> atomic_get_or_create_in_state_dict(const char *key,
595610
// - If our capsule is NOT inserted (another thread inserted first), it will be
596611
// destructed when going out of scope here, so the destructor will be called
597612
// immediately, which will also free the storage.
598-
/*destructor=*/[](void *ptr) -> void { delete static_cast<Payload *>(ptr); });
613+
/*destructor=*/dtor);
599614
// At this point, the capsule object is created successfully.
600615
// Release the unique_ptr and let the capsule object own the storage to avoid double-free.
601616
(void) storage_ptr.release();
@@ -613,13 +628,6 @@ std::pair<Payload *, bool> atomic_get_or_create_in_state_dict(const char *key,
613628
throw error_already_set();
614629
}
615630
created = (capsule_obj == new_capsule.ptr());
616-
if (clear_destructor && created) {
617-
// Our capsule was inserted.
618-
// Remove the destructor to leak the storage on interpreter shutdown.
619-
if (PyCapsule_SetDestructor(capsule_obj, nullptr) < 0) {
620-
throw error_already_set();
621-
}
622-
}
623631
// - If key already existed, our `new_capsule` is not inserted, it will be destructed when
624632
// going out of scope here, which will also free the storage.
625633
// - Otherwise, our `new_capsule` is now in the dict, and it owns the storage and the state
@@ -707,14 +715,21 @@ class internals_pp_manager {
707715
internals_pp_manager(char const *id, on_fetch_function *on_fetch)
708716
: holder_id_(id), on_fetch_(on_fetch) {}
709717

710-
std::unique_ptr<InternalsType> *get_or_create_pp_in_state_dict() {
711-
// The `unique_ptr<InternalsType>` output is leaked on interpreter shutdown. Once an
712-
// instance is created, it will never be deleted until the process exits (compare to
713-
// interpreter shutdown in multiple-interpreter scenarios).
718+
static void internals_shutdown(void *vpp) {
719+
auto *pp = static_cast<std::unique_ptr<InternalsType> *>(vpp);
720+
if (pp && *pp) {
721+
(*pp)->shutdown();
722+
}
714723
// Because we cannot guarantee the order of destruction of capsules in the interpreter
715-
// state dict, leaking avoids potential use-after-free issues during interpreter shutdown.
724+
// state dict, the internals unique_ptr is not deleted in this capsule destructor.
725+
// The internals (and their unique_ptr owner) cannot be deleted until after the interpreter
726+
// has completely shut down. Final cleanup will be done pybind11::finalize_interpreter if
727+
// pybind11 was embedded, or it will be leaked if this is an extension module.
728+
}
729+
730+
std::unique_ptr<InternalsType> *get_or_create_pp_in_state_dict() {
716731
auto result = atomic_get_or_create_in_state_dict<std::unique_ptr<InternalsType>>(
717-
holder_id_, /*clear_destructor=*/true);
732+
holder_id_, &internals_shutdown);
718733
auto *pp = result.first;
719734
bool created = result.second;
720735
// Only call on_fetch_ when fetching existing internals, not when creating new ones.

include/pybind11/gil_safe_call_once.h

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -250,7 +250,9 @@ class gil_safe_call_once_and_store {
250250
// Get or create per-storage capsule in the current interpreter's state dict.
251251
// The storage is interpreter-dependent and will not be shared across interpreters.
252252
storage_type *get_or_create_storage_in_state_dict() {
253-
return detail::atomic_get_or_create_in_state_dict<storage_type>(get_storage_key().c_str())
253+
return detail::atomic_get_or_create_in_state_dict<storage_type>(
254+
get_storage_key().c_str(),
255+
[](void *ptr) -> void { delete static_cast<storage_type *>(ptr); })
254256
.first;
255257
}
256258

0 commit comments

Comments
 (0)