gh-5991: Fix segfault during finalization related to function_record (#6010)

* gh-5991: Fix segfault during finalization related to function_record

This patch was developed with assistance from  Claude Code Opus 4.6

Here's Claude's explanation of the crash mechanism and some reasoning for the difficulty to repro:

`tp_dealloc_impl` calls `cpp_function::destruct` which:
1. Calls `std::free()` on function_record string members (`name`, `doc`, `signature`)
2. Calls `arg.value.dec_ref()` on default argument values
3. Calls `delete rec` on the function_record

But it never calls `PyObject_Free(self)` or `Py_DECREF(Py_TYPE(self))`, which are
required for heap types.

During `_Py_Finalize`, final GC collects the heap types (which survive module dict
clearing via `tp_mro` self-references). This triggers a massive cascade:
`type_dealloc → property_dealloc → meth_dealloc → tp_dealloc_impl → destruct`.

At scale (~1,200+ function_records), the volume of `delete`/`free` calls corrupts
heap metadata, causing subsequent `std::free()` to receive garbage pointers → SEGV.

* Add detail::py_is_finalizing() wrapper to deduplicate version-guarded #ifdef blocks

Also fixes clang-tidy readability-implicit-bool-conversion warnings.

Made-with: Cursor

---------

Co-authored-by: Ralf W. Grosse-Kunstleve <rgrossekunst@nvidia.com>
This commit is contained in:
Itamar Oren
2026-03-23 20:59:26 -07:00
committed by GitHub
parent 1c72409f7f
commit 0a45af2531
2 changed files with 26 additions and 2 deletions

View File

@@ -601,6 +601,15 @@ enum class return_value_policy : uint8_t {
PYBIND11_NAMESPACE_BEGIN(detail)
// Py_IsFinalizing() is a public API since 3.13; before that use _Py_IsFinalizing().
inline bool py_is_finalizing() {
#if PY_VERSION_HEX >= 0x030D0000
return Py_IsFinalizing() != 0;
#else
return _Py_IsFinalizing() != 0;
#endif
}
static constexpr int log2(size_t n, int k = 0) { return (n <= 1) ? k : log2(n >> 1, k + 1); }
// Returns the size as a multiple of sizeof(void *), rounded up.

View File

@@ -846,8 +846,11 @@ protected:
std::free(const_cast<char *>(arg.descr));
}
}
for (auto &arg : rec->args) {
arg.value.dec_ref();
// During finalization, default arg values may already be freed by GC.
if (!detail::py_is_finalizing()) {
for (auto &arg : rec->args) {
arg.value.dec_ref();
}
}
if (rec->def) {
std::free(const_cast<char *>(rec->def->ml_doc));
@@ -1342,9 +1345,21 @@ PYBIND11_NAMESPACE_BEGIN(function_record_PyTypeObject_methods)
// This implementation needs the definition of `class cpp_function`.
inline void tp_dealloc_impl(PyObject *self) {
// Skip dealloc during finalization — GC may have already freed objects
// reachable from the function record (e.g. default arg values), causing
// use-after-free in destruct().
if (detail::py_is_finalizing()) {
return;
}
// Save type before PyObject_Free invalidates self.
auto *type = Py_TYPE(self);
auto *py_func_rec = reinterpret_cast<function_record_PyObject *>(self);
cpp_function::destruct(py_func_rec->cpp_func_rec);
py_func_rec->cpp_func_rec = nullptr;
// PyObject_New increments the heap type refcount and allocates via
// PyObject_Malloc; balance both here
PyObject_Free(self);
Py_DECREF(type);
}
PYBIND11_NAMESPACE_END(function_record_PyTypeObject_methods)