Skip to content

Destruct internals during interpreter finalization #5958

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
b-pass merged 10 commits into from
Jan 18, 2026
Merged

Destruct internals during interpreter finalization #5958

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
10 commits
Select commit Hold shift + click to select a range
6f8308b
Add a shutdown method to internals.
b-pass Jan 13, 2026
50c319d
Actually free internals during interpreter shutdown (instead of after)
b-pass Jan 14, 2026
cb5a5a5
Make sure python is alive before DECREFing
b-pass Jan 14, 2026
f5676ea
make clang-tidy happy
b-pass Jan 14, 2026
d12be34
Check IsFinalizing and use Py_CLEAR, make capsule creation safe if th…
b-pass Jan 14, 2026
959b1e1
oops, put TLS destructor back how it was.
b-pass Jan 15, 2026
717e99f
Oops, proper spelling of unstable _Py_IsFinalizing
b-pass Jan 15, 2026
5f9c30b
Add cleanup step to CI workflow
henryiii Jan 15, 2026
2848fd6
Accept suggested comment
b-pass Jan 18, 2026
75bd292
Avoid recreating internals during type deallocation at shutdown.
b-pass Jan 18, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Next Next commit
Add a shutdown method to internals. 
shutdown can safely DECREF Python objects owned by the internals.
  • Loading branch information
@b-pass
b-pass committed Jan 13, 2026
commit 6f8308b0f9d47956549fecefab04ad7a230c0b2f
47 changes: 31 additions & 16 deletions include/pybind11/detail/internals.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -308,7 +308,16 @@ struct internals {
internals(internals &&other) = delete;
internals &operator=(const internals &other) = delete;
internals &operator=(internals &&other) = delete;
~internals() = default;
~internals() = default; // NOTE: destruct/decref python objects in shutdown()

/// shutdown is run during interpreter finalization and can (carefully) interact with Python.
void shutdown() {
Py_XDECREF(static_property_type);
static_property_type = nullptr;

Py_XDECREF(default_metaclass);
default_metaclass = nullptr;
}
};

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

std::forward_list<ExceptionTranslator> registered_exception_translators;
PyTypeObject *function_record_py_type = nullptr;

/// shutdown is run during interpreter finalization and can (carefully) interact with Python.
void shutdown() {
Comment thread
b-pass marked this conversation as resolved.
Outdated
Py_XDECREF(function_record_py_type);
function_record_py_type = nullptr;
}
};

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

auto state_dict = reinterpret_borrow<dict>(get_python_state_dict());
Expand All @@ -595,7 +610,7 @@ std::pair<Payload *, bool> atomic_get_or_create_in_state_dict(const char *key,
// - If our capsule is NOT inserted (another thread inserted first), it will be
// destructed when going out of scope here, so the destructor will be called
// immediately, which will also free the storage.
/*destructor=*/[](void *ptr) -> void { delete static_cast<Payload *>(ptr); });
/*destructor=*/dtor);

@oremanj oremanj Jan 14, 2026 •
edited
Loading

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is not a semantics-preserving change.

There are two possibilities for when the capsule gets destroyed:

  • It could be destroyed almost immediately, if another thread inserted the same key before we did. See the comment on line 639. In that case, no one else has seen our newly-allocated payload, so we should delete it.
  • It could be destroyed at interpreter finalization. That's when custom destruction logic might come into play.

If two threads concurrently try to create internals, your use of the final dtor from the start means that one of them will leak its unique_ptr. Not a terrible outcome, but easy enough to avoid.

Suggest you change the dtor parameter to dtor_if_inserted. Create the capsule initially with a destructor that deallocates the payload. If insertion succeeds and dtor_if_inserted is not null, then swap the capsule destructor to be dtor_if_inserted instead. Effectively, the old clear_destructor parameter is a special case of the dtor_if_inserted semantics: dtor_if_inserted is a no-op lambda if clear_destructor is true, or is null (i.e. continue with the original dtor that deletes the payload) if clear_destructor is false.

@b-pass b-pass Jan 15, 2026

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ok, I believe the latest change addresses this.

// At this point, the capsule object is created successfully.
// Release the unique_ptr and let the capsule object own the storage to avoid double-free.
(void) storage_ptr.release();
Expand All @@ -613,13 +628,6 @@ std::pair<Payload *, bool> atomic_get_or_create_in_state_dict(const char *key,
throw error_already_set();
}
created = (capsule_obj == new_capsule.ptr());
if (clear_destructor && created) {
// Our capsule was inserted.
// Remove the destructor to leak the storage on interpreter shutdown.
if (PyCapsule_SetDestructor(capsule_obj, nullptr) < 0) {
throw error_already_set();
}
}
// - If key already existed, our `new_capsule` is not inserted, it will be destructed when
// going out of scope here, which will also free the storage.
// - Otherwise, our `new_capsule` is now in the dict, and it owns the storage and the state
Expand Down Expand Up @@ -707,14 +715,21 @@ class internals_pp_manager {
internals_pp_manager(char const *id, on_fetch_function *on_fetch)
: holder_id_(id), on_fetch_(on_fetch) {}

std::unique_ptr<InternalsType> *get_or_create_pp_in_state_dict() {
// The `unique_ptr<InternalsType>` output is leaked on interpreter shutdown. Once an
// instance is created, it will never be deleted until the process exits (compare to
// interpreter shutdown in multiple-interpreter scenarios).
static void internals_shutdown(void *vpp) {
auto *pp = static_cast<std::unique_ptr<InternalsType> *>(vpp);
if (pp && *pp) {
(*pp)->shutdown();
}
// Because we cannot guarantee the order of destruction of capsules in the interpreter
// state dict, leaking avoids potential use-after-free issues during interpreter shutdown.
// state dict, the internals unique_ptr is not deleted in this capsule destructor.
// The internals (and their unique_ptr owner) cannot be deleted until after the interpreter
// has completely shut down. Final cleanup will be done pybind11::finalize_interpreter if
// pybind11 was embedded, or it will be leaked if this is an extension module.
}

std::unique_ptr<InternalsType> *get_or_create_pp_in_state_dict() {
auto result = atomic_get_or_create_in_state_dict<std::unique_ptr<InternalsType>>(
holder_id_, /*clear_destructor=*/true);
holder_id_, &internals_shutdown);
auto *pp = result.first;
bool created = result.second;
// Only call on_fetch_ when fetching existing internals, not when creating new ones.
Expand Down
4 changes: 3 additions & 1 deletion include/pybind11/gil_safe_call_once.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -250,7 +250,9 @@ class gil_safe_call_once_and_store {
// Get or create per-storage capsule in the current interpreter's state dict.
// The storage is interpreter-dependent and will not be shared across interpreters.
storage_type *get_or_create_storage_in_state_dict() {
return detail::atomic_get_or_create_in_state_dict<storage_type>(get_storage_key().c_str())
return detail::atomic_get_or_create_in_state_dict<storage_type>(
get_storage_key().c_str(),
[](void *ptr) -> void { delete static_cast<storage_type *>(ptr); })
.first;
}

Expand Down
Loading