Skip to content

Commit 51a8187

Browse files
committed
Avoid recreating internals during type dealloc
Introduce a non-creating internals lookup and use it in pybind11_meta_dealloc so teardown paths don’t accidentally rebuild internals and their static types during Py_EndInterpreter. This prevents with_internals from triggering make_static_property_type while the interpreter is finalizing, which heaptrack showed as leaked allocations. Changes: - add is_interpreter_finalizing() helper - add internals_pp_manager::get_pp_if_exists() and a state-dict lookup helper that does not create capsules - switch reset() to use the non-creating lookup - update pybind11_meta_dealloc to operate only when internals exist, avoiding implicit internals creation during GC/finalization This reduces the leaked allocations tied to pybind11_meta_dealloc and keeps teardown from reinitializing internals after they were reset.
1 parent 9eb5239 commit 51a8187

File tree

2 files changed

+110
-56
lines changed

2 files changed

+110
-56
lines changed

include/pybind11/detail/class.h

Lines changed: 54 additions & 39 deletions
Original file line numberDiff line numberDiff line change
@@ -207,49 +207,64 @@ extern "C" inline PyObject *pybind11_meta_call(PyObject *type, PyObject *args, P
207207

208208
/// Cleanup the type-info for a pybind11-registered type.
209209
extern "C" inline void pybind11_meta_dealloc(PyObject *obj) {
210-
with_internals([obj](internals &internals) {
211-
auto *type = (PyTypeObject *) obj;
212-
213-
// A pybind11-registered type will:
214-
// 1) be found in internals.registered_types_py
215-
// 2) have exactly one associated `detail::type_info`
216-
auto found_type = internals.registered_types_py.find(type);
217-
if (found_type != internals.registered_types_py.end() && found_type->second.size() == 1
218-
&& found_type->second[0]->type == type) {
219-
220-
auto *tinfo = found_type->second[0];
221-
auto tindex = std::type_index(*tinfo->cpptype);
222-
internals.direct_conversions.erase(tindex);
223-
224-
auto &local_internals = get_local_internals();
225-
if (tinfo->module_local) {
226-
local_internals.registered_types_cpp.erase(tinfo->cpptype);
227-
} else {
228-
internals.registered_types_cpp.erase(tindex);
229-
#if PYBIND11_INTERNALS_VERSION >= 12
230-
internals.registered_types_cpp_fast.erase(tinfo->cpptype);
231-
for (const std::type_info *alias : tinfo->alias_chain) {
232-
auto num_erased = internals.registered_types_cpp_fast.erase(alias);
233-
(void) num_erased;
234-
assert(num_erased > 0);
235-
}
236-
#endif
210+
if (is_interpreter_finalizing()) {
211+
PyType_Type.tp_dealloc(obj);
212+
return;
213+
}
214+
215+
auto *internals_pp = get_internals_pp_manager().get_pp_if_exists();
216+
if (!internals_pp || internals_pp->get() == nullptr) {
217+
PyType_Type.tp_dealloc(obj);
218+
return;
219+
}
220+
221+
auto &internals = *internals_pp->get();
222+
PYBIND11_LOCK_INTERNALS(internals);
223+
224+
auto *type = (PyTypeObject *) obj;
225+
226+
// A pybind11-registered type will:
227+
// 1) be found in internals.registered_types_py
228+
// 2) have exactly one associated `detail::type_info`
229+
auto found_type = internals.registered_types_py.find(type);
230+
if (found_type != internals.registered_types_py.end() && found_type->second.size() == 1
231+
&& found_type->second[0]->type == type) {
232+
233+
auto *tinfo = found_type->second[0];
234+
auto tindex = std::type_index(*tinfo->cpptype);
235+
internals.direct_conversions.erase(tindex);
236+
237+
auto *local_internals_pp = get_local_internals_pp_manager().get_pp_if_exists();
238+
auto *local_internals_ptr = local_internals_pp ? local_internals_pp->get() : nullptr;
239+
if (tinfo->module_local) {
240+
if (local_internals_ptr) {
241+
local_internals_ptr->registered_types_cpp.erase(tinfo->cpptype);
237242
}
238-
internals.registered_types_py.erase(tinfo->type);
239-
240-
// Actually just `std::erase_if`, but that's only available in C++20
241-
auto &cache = internals.inactive_override_cache;
242-
for (auto it = cache.begin(), last = cache.end(); it != last;) {
243-
if (it->first == (PyObject *) tinfo->type) {
244-
it = cache.erase(it);
245-
} else {
246-
++it;
247-
}
243+
} else {
244+
internals.registered_types_cpp.erase(tindex);
245+
#if PYBIND11_INTERNALS_VERSION >= 12
246+
internals.registered_types_cpp_fast.erase(tinfo->cpptype);
247+
for (const std::type_info *alias : tinfo->alias_chain) {
248+
auto num_erased = internals.registered_types_cpp_fast.erase(alias);
249+
(void) num_erased;
250+
assert(num_erased > 0);
248251
}
252+
#endif
253+
}
254+
internals.registered_types_py.erase(tinfo->type);
249255

250-
delete tinfo;
256+
// Actually just `std::erase_if`, but that's only available in C++20
257+
auto &cache = internals.inactive_override_cache;
258+
for (auto it = cache.begin(), last = cache.end(); it != last;) {
259+
if (it->first == (PyObject *) tinfo->type) {
260+
it = cache.erase(it);
261+
} else {
262+
++it;
263+
}
251264
}
252-
});
265+
266+
delete tinfo;
267+
}
253268

254269
PyType_Type.tp_dealloc(obj);
255270
}

include/pybind11/detail/internals.h

Lines changed: 56 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -203,6 +203,14 @@ inline bool is_interpreter_alive() {
203203
#endif
204204
}
205205

206+
inline bool is_interpreter_finalizing() {
207+
#if PY_VERSION_HEX < 0x030D0000
208+
return _Py_IsFinalizing() != 0;
209+
#else
210+
return Py_IsFinalizing() != 0;
211+
#endif
212+
}
213+
206214
#ifdef Py_GIL_DISABLED
207215
// Wrapper around PyMutex to provide BasicLockable semantics
208216
class pymutex {
@@ -707,6 +715,26 @@ class internals_pp_manager {
707715
return internals_singleton_pp_;
708716
}
709717

718+
/// Get the current pointer-to-pointer if it already exists, without creating it.
719+
std::unique_ptr<InternalsType> *get_pp_if_exists() {
720+
#ifdef PYBIND11_HAS_SUBINTERPRETER_SUPPORT
721+
if (has_seen_non_main_interpreter()) {
722+
auto *tstate = get_thread_state_unchecked();
723+
if (tstate && tstate->interp != last_istate_tls()) {
724+
gil_scoped_acquire_simple gil;
725+
last_istate_tls() = tstate->interp;
726+
internals_p_tls() = get_pp_from_state_dict_if_exists();
727+
}
728+
return internals_p_tls();
729+
}
730+
#endif
731+
if (!internals_singleton_pp_) {
732+
gil_scoped_acquire_simple gil;
733+
internals_singleton_pp_ = get_pp_from_state_dict_if_exists();
734+
}
735+
return internals_singleton_pp_;
736+
}
737+
710738
/// Drop all the references we're currently holding.
711739
void unref() {
712740
#ifdef PYBIND11_HAS_SUBINTERPRETER_SUPPORT
@@ -728,23 +756,10 @@ class internals_pp_manager {
728756
gil_scoped_acquire_simple gil;
729757
auto *tstate = get_thread_state_unchecked();
730758
if (tstate) {
731-
try {
732-
// Get the capsule directly from the state dict and reset the unique_ptr
733-
auto state_dict = reinterpret_borrow<dict>(get_python_state_dict());
734-
PyObject *capsule_obj = dict_getitemstring(state_dict.ptr(), holder_id_);
735-
if (capsule_obj) {
736-
void *raw_ptr = PyCapsule_GetPointer(capsule_obj, /*name=*/nullptr);
737-
if (raw_ptr) {
738-
auto *pp = static_cast<std::unique_ptr<InternalsType> *>(raw_ptr);
739-
if (pp && pp->get() != nullptr) {
740-
// Only reset if the unique_ptr actually contains an object
741-
pp->reset();
742-
}
743-
}
744-
}
745-
} catch (...) {
746-
// If we can't get the state dict or capsule, silently continue
747-
// The capsule destructor will clean up eventually
759+
auto *pp = get_pp_from_state_dict_if_exists();
760+
if (pp && pp->get() != nullptr) {
761+
// Only reset if the unique_ptr actually contains an object
762+
pp->reset();
748763
}
749764
}
750765
return;
@@ -810,6 +825,30 @@ class internals_pp_manager {
810825
return pp;
811826
}
812827

828+
std::unique_ptr<InternalsType> *get_pp_from_state_dict_if_exists() {
829+
error_scope err_scope; // preserve any existing Python error states
830+
try {
831+
auto state_dict = reinterpret_borrow<dict>(get_python_state_dict());
832+
PyObject *capsule_obj = dict_getitemstring(state_dict.ptr(), holder_id_);
833+
if (capsule_obj == nullptr) {
834+
if (PyErr_Occurred()) {
835+
PyErr_Clear();
836+
}
837+
return nullptr;
838+
}
839+
void *raw_ptr = PyCapsule_GetPointer(capsule_obj, /*name=*/nullptr);
840+
if (!raw_ptr) {
841+
if (PyErr_Occurred()) {
842+
PyErr_Clear();
843+
}
844+
return nullptr;
845+
}
846+
return static_cast<std::unique_ptr<InternalsType> *>(raw_ptr);
847+
} catch (const error_already_set &) {
848+
return nullptr;
849+
}
850+
}
851+
813852
#ifdef PYBIND11_HAS_SUBINTERPRETER_SUPPORT
814853
static PyInterpreterState *&last_istate_tls() {
815854
static thread_local PyInterpreterState *last_istate = nullptr;

0 commit comments

Comments
 (0)