From a0eb05b4e336d83b396ed667667be36e0e06ad61 Mon Sep 17 00:00:00 2001 From: Aaron Gokaslan Date: Mon, 7 Feb 2022 11:20:40 -0500 Subject: [PATCH 01/18] Test out Python 3.11 migration --- include/pybind11/pybind11.h | 9 +++++---- 1 file changed, 5 insertions(+), 4 deletions(-) diff --git a/include/pybind11/pybind11.h b/include/pybind11/pybind11.h index 7aa93bb5aa..d4395e0edb 100644 --- a/include/pybind11/pybind11.h +++ b/include/pybind11/pybind11.h @@ -2384,7 +2384,7 @@ inline function get_type_override(const void *this_ptr, const type_info *this_ty /* Don't call dispatch code if invoked from overridden function. Unfortunately this doesn't work on PyPy. */ -#if !defined(PYPY_VERSION) && PY_VERSION_HEX < 0x030B0000 +#if !defined(PYPY_VERSION) // TODO: Remove PyPy workaround for Python 3.11. // Current API fails on 3.11 since co_varnames can be null. #if PY_VERSION_HEX >= 0x03090000 @@ -2394,10 +2394,11 @@ inline function get_type_override(const void *this_ptr, const type_info *this_ty // f_code is guaranteed to not be NULL if ((std::string) str(f_code->co_name) == name && f_code->co_argcount > 0) { PyObject* locals = PyEval_GetLocals(); - if (locals != nullptr && f_code->co_varnames != nullptr) { + if (locals != nullptr) { PyObject *self_caller = dict_getitem( - locals, PyTuple_GET_ITEM(f_code->co_varnames, 0) - ); + locals, + PyTuple_GET_ITEM(PyObject_GetAttrString((PyObject *) f_code, "co_varnames"), + 0)); if (self_caller == self.ptr()) { Py_DECREF(f_code); Py_DECREF(frame); From 6a0a4d07d59787e0f8e26536f7031782430551d5 Mon Sep 17 00:00:00 2001 From: Aaron Gokaslan Date: Mon, 7 Feb 2022 11:37:42 -0500 Subject: [PATCH 02/18] Clean up a bit --- include/pybind11/pybind11.h | 7 +++---- 1 file changed, 3 insertions(+), 4 deletions(-) diff --git a/include/pybind11/pybind11.h b/include/pybind11/pybind11.h index d4395e0edb..b20997e047 100644 --- a/include/pybind11/pybind11.h +++ b/include/pybind11/pybind11.h @@ -2395,10 +2395,9 @@ inline function get_type_override(const void *this_ptr, const type_info *this_ty if ((std::string) str(f_code->co_name) == name && f_code->co_argcount > 0) { PyObject* locals = PyEval_GetLocals(); if (locals != nullptr) { - PyObject *self_caller = dict_getitem( - locals, - PyTuple_GET_ITEM(PyObject_GetAttrString((PyObject *) f_code, "co_varnames"), - 0)); + PyObject *self_arg = PyTuple_GET_ITEM( + PyObject_GetAttrString((PyObject *) f_code, "co_varnames"), 0); + PyObject *self_caller = dict_getitem(locals, self_arg); if (self_caller == self.ptr()) { Py_DECREF(f_code); Py_DECREF(frame); From e3f3644b70ef6083e37890fccccb91a471c50e2a Mon Sep 17 00:00:00 2001 From: Aaron Gokaslan Date: Mon, 7 Feb 2022 11:42:38 -0500 Subject: [PATCH 03/18] Remove todo --- include/pybind11/pybind11.h | 2 -- 1 file changed, 2 deletions(-) diff --git a/include/pybind11/pybind11.h b/include/pybind11/pybind11.h index b20997e047..87d93f760c 100644 --- a/include/pybind11/pybind11.h +++ b/include/pybind11/pybind11.h @@ -2385,8 +2385,6 @@ inline function get_type_override(const void *this_ptr, const type_info *this_ty /* Don't call dispatch code if invoked from overridden function. Unfortunately this doesn't work on PyPy. */ #if !defined(PYPY_VERSION) - // TODO: Remove PyPy workaround for Python 3.11. - // Current API fails on 3.11 since co_varnames can be null. #if PY_VERSION_HEX >= 0x03090000 PyFrameObject *frame = PyThreadState_GetFrame(PyThreadState_Get()); if (frame != nullptr) { From c0a1306168c93c6487b40fdabbbb614a6b5d7813 Mon Sep 17 00:00:00 2001 From: Aaron Gokaslan Date: Mon, 7 Feb 2022 11:55:37 -0500 Subject: [PATCH 04/18] Test workaround --- tests/test_multiple_inheritance.cpp | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/tests/test_multiple_inheritance.cpp b/tests/test_multiple_inheritance.cpp index 4689df4e46..0a3c59ede9 100644 --- a/tests/test_multiple_inheritance.cpp +++ b/tests/test_multiple_inheritance.cpp @@ -8,6 +8,7 @@ BSD-style license that can be found in the LICENSE file. */ +#include "pybind11/attr.h" #include "pybind11_tests.h" #include "constructor_stats.h" @@ -212,8 +213,8 @@ TEST_SUBMODULE(multiple_inheritance, m) { struct VanillaDictMix1 : Vanilla, WithDict { }; struct VanillaDictMix2 : WithDict, Vanilla { }; py::class_(m, "WithDict", py::dynamic_attr()).def(py::init<>()); - py::class_(m, "VanillaDictMix1").def(py::init<>()); - py::class_(m, "VanillaDictMix2").def(py::init<>()); + py::class_(m, "VanillaDictMix1", py::dynamic_attr()).def(py::init<>()); + py::class_(m, "VanillaDictMix2", py::dynamic_attr()).def(py::init<>()); // test_diamond_inheritance // Issue #959: segfault when constructing diamond inheritance instance From a731769515f153fda41cd3e460faebda16355929 Mon Sep 17 00:00:00 2001 From: Aaron Gokaslan Date: Mon, 7 Feb 2022 12:12:00 -0500 Subject: [PATCH 05/18] Fix potential bug uncovered in 3.11 --- include/pybind11/detail/class.h | 5 +++++ tests/test_multiple_inheritance.cpp | 5 ++--- 2 files changed, 7 insertions(+), 3 deletions(-) diff --git a/include/pybind11/detail/class.h b/include/pybind11/detail/class.h index cc1e40ce7a..536e1cdce5 100644 --- a/include/pybind11/detail/class.h +++ b/include/pybind11/detail/class.h @@ -686,6 +686,11 @@ inline PyObject* make_new_python_type(const type_record &rec) { if (rec.custom_type_setup_callback) rec.custom_type_setup_callback(heap_type); + if (PyType_IS_GC(type) && type->tp_traverse == nullptr) { + // cludge for MI check on 3.11. + type->tp_traverse = pybind11_traverse; + } + if (PyType_Ready(type) < 0) pybind11_fail(std::string(rec.name) + ": PyType_Ready failed (" + error_string() + ")!"); diff --git a/tests/test_multiple_inheritance.cpp b/tests/test_multiple_inheritance.cpp index 0a3c59ede9..4689df4e46 100644 --- a/tests/test_multiple_inheritance.cpp +++ b/tests/test_multiple_inheritance.cpp @@ -8,7 +8,6 @@ BSD-style license that can be found in the LICENSE file. */ -#include "pybind11/attr.h" #include "pybind11_tests.h" #include "constructor_stats.h" @@ -213,8 +212,8 @@ TEST_SUBMODULE(multiple_inheritance, m) { struct VanillaDictMix1 : Vanilla, WithDict { }; struct VanillaDictMix2 : WithDict, Vanilla { }; py::class_(m, "WithDict", py::dynamic_attr()).def(py::init<>()); - py::class_(m, "VanillaDictMix1", py::dynamic_attr()).def(py::init<>()); - py::class_(m, "VanillaDictMix2", py::dynamic_attr()).def(py::init<>()); + py::class_(m, "VanillaDictMix1").def(py::init<>()); + py::class_(m, "VanillaDictMix2").def(py::init<>()); // test_diamond_inheritance // Issue #959: segfault when constructing diamond inheritance instance From be9561196dc75997b152319c37d06eacfef24c29 Mon Sep 17 00:00:00 2001 From: Aaron Gokaslan Date: Mon, 7 Feb 2022 12:35:35 -0500 Subject: [PATCH 06/18] Try to fix it more --- include/pybind11/detail/class.h | 12 +++++++++++- 1 file changed, 11 insertions(+), 1 deletion(-) diff --git a/include/pybind11/detail/class.h b/include/pybind11/detail/class.h index 536e1cdce5..16fd2fa65e 100644 --- a/include/pybind11/detail/class.h +++ b/include/pybind11/detail/class.h @@ -634,6 +634,15 @@ inline PyObject* make_new_python_type(const type_record &rec) { auto base = (bases.empty()) ? internals.instance_base : bases[0].ptr(); + traverseproc parent_tp_traverse = nullptr; + for (auto b : bases) { + auto *obj = (PyTypeObject *) b.ptr(); + if (PyType_IS_GC(obj) != 0 && obj->tp_traverse != nullptr) { + parent_tp_traverse = obj->tp_traverse; + break; // Should I merge them somehow? + } + } + /* Danger zone: from now (and until PyType_Ready), make sure to issue no Python C API calls which could potentially invoke the garbage collector (the GC will call type_traverse(), which will in @@ -688,7 +697,8 @@ inline PyObject* make_new_python_type(const type_record &rec) { if (PyType_IS_GC(type) && type->tp_traverse == nullptr) { // cludge for MI check on 3.11. - type->tp_traverse = pybind11_traverse; + type->tp_traverse + = (parent_tp_traverse != nullptr) ? parent_tp_traverse : pybind11_traverse; } if (PyType_Ready(type) < 0) From 5864a2a606f65c076b0e103bd30c6180c26ba5b0 Mon Sep 17 00:00:00 2001 From: Aaron Gokaslan Date: Mon, 7 Feb 2022 12:44:55 -0500 Subject: [PATCH 07/18] last ditch fix --- include/pybind11/detail/class.h | 14 +++++++++++--- 1 file changed, 11 insertions(+), 3 deletions(-) diff --git a/include/pybind11/detail/class.h b/include/pybind11/detail/class.h index 16fd2fa65e..234befa6f1 100644 --- a/include/pybind11/detail/class.h +++ b/include/pybind11/detail/class.h @@ -635,11 +635,18 @@ inline PyObject* make_new_python_type(const type_record &rec) { : bases[0].ptr(); traverseproc parent_tp_traverse = nullptr; + auto parent_tp_clear = nullptr; for (auto b : bases) { auto *obj = (PyTypeObject *) b.ptr(); - if (PyType_IS_GC(obj) != 0 && obj->tp_traverse != nullptr) { - parent_tp_traverse = obj->tp_traverse; - break; // Should I merge them somehow? + if (PyType_IS_GC(obj) != 0) { + if (obj->tp_traverse != nullptr) { + parent_tp_traverse = obj->tp_traverse; + } + if (obj->tp_clear != nullptr) { + parent_tp_clear = obj->tp_clear; + } + // This needs to improved and should probably combine funcs + break; } } @@ -699,6 +706,7 @@ inline PyObject* make_new_python_type(const type_record &rec) { // cludge for MI check on 3.11. type->tp_traverse = (parent_tp_traverse != nullptr) ? parent_tp_traverse : pybind11_traverse; + type->tp_clear = (parent_tp_clear != nullptr) ? parent_tp_clear : pybind11_clear; } if (PyType_Ready(type) < 0) From b292dd839183d245318a57cdabb4f3de834efff1 Mon Sep 17 00:00:00 2001 From: Aaron Gokaslan Date: Sat, 12 Feb 2022 12:12:11 -0500 Subject: [PATCH 08/18] Revert. Tp-traverse isn't the problem --- include/pybind11/detail/class.h | 17 +---------------- 1 file changed, 1 insertion(+), 16 deletions(-) diff --git a/include/pybind11/detail/class.h b/include/pybind11/detail/class.h index 453152ca91..a94a395d32 100644 --- a/include/pybind11/detail/class.h +++ b/include/pybind11/detail/class.h @@ -653,22 +653,6 @@ inline PyObject *make_new_python_type(const type_record &rec) { auto bases = tuple(rec.bases); auto *base = (bases.empty()) ? internals.instance_base : bases[0].ptr(); - traverseproc parent_tp_traverse = nullptr; - auto parent_tp_clear = nullptr; - for (auto b : bases) { - auto *obj = (PyTypeObject *) b.ptr(); - if (PyType_IS_GC(obj) != 0) { - if (obj->tp_traverse != nullptr) { - parent_tp_traverse = obj->tp_traverse; - } - if (obj->tp_clear != nullptr) { - parent_tp_clear = obj->tp_clear; - } - // This needs to improved and should probably combine funcs - break; - } - } - /* Danger zone: from now (and until PyType_Ready), make sure to issue no Python C API calls which could potentially invoke the garbage collector (the GC will call type_traverse(), which will in @@ -726,6 +710,7 @@ inline PyObject *make_new_python_type(const type_record &rec) { pybind11_fail(std::string(rec.name) + ": PyType_Ready failed (" + error_string() + ")!"); } + assert(type->tp_traverse != nullptr || !PyType_HasFeature(type, Py_TPFLAGS_HAVE_GC)); assert(!rec.dynamic_attr || PyType_HasFeature(type, Py_TPFLAGS_HAVE_GC)); /* Register type with the parent scope */ From 729bb4f75052e4d557b2789fa4996312312275b5 Mon Sep 17 00:00:00 2001 From: Aaron Gokaslan Date: Sat, 12 Feb 2022 12:20:42 -0500 Subject: [PATCH 09/18] Test workaround --- tests/test_multiple_inheritance.cpp | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/tests/test_multiple_inheritance.cpp b/tests/test_multiple_inheritance.cpp index 5916ae9010..a89e87c31d 100644 --- a/tests/test_multiple_inheritance.cpp +++ b/tests/test_multiple_inheritance.cpp @@ -204,7 +204,9 @@ TEST_SUBMODULE(multiple_inheritance, m) { m.def("i801e_b2", []() -> I801B2 * { return new I801E(); }); // test_mi_static_properties - py::class_(m, "Vanilla").def(py::init<>()).def("vanilla", &Vanilla::vanilla); + py::class_(m, "Vanilla", py::dynamic_attr()) + .def(py::init<>()) + .def("vanilla", &Vanilla::vanilla); py::class_(m, "WithStatic1") .def(py::init<>()) From 8bd9214d0229b5946dcbfdda3f008b77e2f5adea Mon Sep 17 00:00:00 2001 From: Aaron Gokaslan Date: Sat, 12 Feb 2022 12:39:25 -0500 Subject: [PATCH 10/18] Try this hack --- include/pybind11/detail/class.h | 14 ++++++++++++-- 1 file changed, 12 insertions(+), 2 deletions(-) diff --git a/include/pybind11/detail/class.h b/include/pybind11/detail/class.h index a94a395d32..98c1b80113 100644 --- a/include/pybind11/detail/class.h +++ b/include/pybind11/detail/class.h @@ -653,6 +653,16 @@ inline PyObject *make_new_python_type(const type_record &rec) { auto bases = tuple(rec.bases); auto *base = (bases.empty()) ? internals.instance_base : bases[0].ptr(); + bool has_dynamic_base = false; + for (auto b : bases) { + auto *obj = (PyTypeObject *) b.ptr(); + // hack to see if enable_dynamic_attrs if any of the bases do. + if (obj->tp_traverse == pybind11_traverse) { + has_dynamic_base = true; + break; + } + } + /* Danger zone: from now (and until PyType_Ready), make sure to issue no Python C API calls which could potentially invoke the garbage collector (the GC will call type_traverse(), which will in @@ -694,7 +704,7 @@ inline PyObject *make_new_python_type(const type_record &rec) { type->tp_flags |= Py_TPFLAGS_BASETYPE; } - if (rec.dynamic_attr) { + if (rec.dynamic_attr || has_dynamic_base) { enable_dynamic_attributes(heap_type); } @@ -711,7 +721,7 @@ inline PyObject *make_new_python_type(const type_record &rec) { } assert(type->tp_traverse != nullptr || !PyType_HasFeature(type, Py_TPFLAGS_HAVE_GC)); - assert(!rec.dynamic_attr || PyType_HasFeature(type, Py_TPFLAGS_HAVE_GC)); + // assert(!rec.dynamic_attr || PyType_HasFeature(type, Py_TPFLAGS_HAVE_GC)); /* Register type with the parent scope */ if (rec.scope) { From b37559120abdfc78514a5df41a50017f5453b8d2 Mon Sep 17 00:00:00 2001 From: Aaron Gokaslan Date: Sun, 27 Feb 2022 13:02:03 -0500 Subject: [PATCH 11/18] Revert MRO changes --- include/pybind11/detail/class.h | 15 ++------------- tests/test_multiple_inheritance.cpp | 4 +--- 2 files changed, 3 insertions(+), 16 deletions(-) diff --git a/include/pybind11/detail/class.h b/include/pybind11/detail/class.h index 98c1b80113..ebdc07271b 100644 --- a/include/pybind11/detail/class.h +++ b/include/pybind11/detail/class.h @@ -653,16 +653,6 @@ inline PyObject *make_new_python_type(const type_record &rec) { auto bases = tuple(rec.bases); auto *base = (bases.empty()) ? internals.instance_base : bases[0].ptr(); - bool has_dynamic_base = false; - for (auto b : bases) { - auto *obj = (PyTypeObject *) b.ptr(); - // hack to see if enable_dynamic_attrs if any of the bases do. - if (obj->tp_traverse == pybind11_traverse) { - has_dynamic_base = true; - break; - } - } - /* Danger zone: from now (and until PyType_Ready), make sure to issue no Python C API calls which could potentially invoke the garbage collector (the GC will call type_traverse(), which will in @@ -704,7 +694,7 @@ inline PyObject *make_new_python_type(const type_record &rec) { type->tp_flags |= Py_TPFLAGS_BASETYPE; } - if (rec.dynamic_attr || has_dynamic_base) { + if (rec.dynamic_attr) { enable_dynamic_attributes(heap_type); } @@ -720,8 +710,7 @@ inline PyObject *make_new_python_type(const type_record &rec) { pybind11_fail(std::string(rec.name) + ": PyType_Ready failed (" + error_string() + ")!"); } - assert(type->tp_traverse != nullptr || !PyType_HasFeature(type, Py_TPFLAGS_HAVE_GC)); - // assert(!rec.dynamic_attr || PyType_HasFeature(type, Py_TPFLAGS_HAVE_GC)); + assert(!rec.dynamic_attr || PyType_HasFeature(type, Py_TPFLAGS_HAVE_GC)); /* Register type with the parent scope */ if (rec.scope) { diff --git a/tests/test_multiple_inheritance.cpp b/tests/test_multiple_inheritance.cpp index a89e87c31d..5916ae9010 100644 --- a/tests/test_multiple_inheritance.cpp +++ b/tests/test_multiple_inheritance.cpp @@ -204,9 +204,7 @@ TEST_SUBMODULE(multiple_inheritance, m) { m.def("i801e_b2", []() -> I801B2 * { return new I801E(); }); // test_mi_static_properties - py::class_(m, "Vanilla", py::dynamic_attr()) - .def(py::init<>()) - .def("vanilla", &Vanilla::vanilla); + py::class_(m, "Vanilla").def(py::init<>()).def("vanilla", &Vanilla::vanilla); py::class_(m, "WithStatic1") .def(py::init<>()) From 6fa3fc5062d27d800726ad1751f49cccba10b3fb Mon Sep 17 00:00:00 2001 From: Aaron Gokaslan Date: Mon, 28 Feb 2022 13:04:47 -0500 Subject: [PATCH 12/18] Use f_back properly --- include/pybind11/detail/type_caster_base.h | 12 ++++++++++-- 1 file changed, 10 insertions(+), 2 deletions(-) diff --git a/include/pybind11/detail/type_caster_base.h b/include/pybind11/detail/type_caster_base.h index ff7d7cf8c2..e8a58b9aee 100644 --- a/include/pybind11/detail/type_caster_base.h +++ b/include/pybind11/detail/type_caster_base.h @@ -502,9 +502,10 @@ PYBIND11_NOINLINE std::string error_string() { } PyFrameObject *frame = trace->tb_frame; + Py_XINCREF(frame); errorString += "\n\nAt:\n"; while (frame) { -# if PY_VERSION_HEX >= 0x03090000 +# if PY_VERSION_HEX >= 0x030900B1 PyCodeObject *f_code = PyFrame_GetCode(frame); # else PyCodeObject *f_code = frame->f_code; @@ -514,7 +515,14 @@ PYBIND11_NOINLINE std::string error_string() { errorString += " " + handle(f_code->co_filename).cast() + "(" + std::to_string(lineno) + "): " + handle(f_code->co_name).cast() + "\n"; - frame = frame->f_back; +# if PY_VERSION_HEX >= 0x030900B1 + auto b_frame = PyFrame_GetBack(frame); +# else + auto b_frame = frame->f_back; + Py_XINCREF(b_frame); +# endif + Py_DECREF(frame); + frame = b_frame; Py_DECREF(f_code); } } From 38f9096d47b40a2b45bfd865d4562e13eca05e84 Mon Sep 17 00:00:00 2001 From: Aaron Gokaslan Date: Mon, 28 Feb 2022 13:19:33 -0500 Subject: [PATCH 13/18] Qualify auto --- include/pybind11/detail/type_caster_base.h | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/include/pybind11/detail/type_caster_base.h b/include/pybind11/detail/type_caster_base.h index e8a58b9aee..9636bcaf5f 100644 --- a/include/pybind11/detail/type_caster_base.h +++ b/include/pybind11/detail/type_caster_base.h @@ -516,9 +516,9 @@ PYBIND11_NOINLINE std::string error_string() { + std::to_string(lineno) + "): " + handle(f_code->co_name).cast() + "\n"; # if PY_VERSION_HEX >= 0x030900B1 - auto b_frame = PyFrame_GetBack(frame); + auto *b_frame = PyFrame_GetBack(frame); # else - auto b_frame = frame->f_back; + auto *b_frame = frame->f_back; Py_XINCREF(b_frame); # endif Py_DECREF(frame); From 9b9df3d588ac18a95184c3488f6c483beb3bff13 Mon Sep 17 00:00:00 2001 From: Henry Schreiner Date: Tue, 1 Mar 2022 14:39:08 -0500 Subject: [PATCH 14/18] Update include/pybind11/pybind11.h --- include/pybind11/pybind11.h | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/include/pybind11/pybind11.h b/include/pybind11/pybind11.h index 24d979778c..d31be89db6 100644 --- a/include/pybind11/pybind11.h +++ b/include/pybind11/pybind11.h @@ -2645,9 +2645,11 @@ get_type_override(const void *this_ptr, const type_info *this_type, const char * if ((std::string) str(f_code->co_name) == name && f_code->co_argcount > 0) { PyObject *locals = PyEval_GetLocals(); if (locals != nullptr) { - PyObject *self_arg = PyTuple_GET_ITEM( - PyObject_GetAttrString((PyObject *) f_code, "co_varnames"), 0); + PyObject *co_varnames = + PyObject_GetAttrString((PyObject *) f_code, "co_varnames"); + PyObject *self_arg = PyTuple_GET_ITEM(co_varnames, 0); PyObject *self_caller = dict_getitem(locals, self_arg); + Py_DECREF(co_varnames); if (self_caller == self.ptr()) { Py_DECREF(f_code); Py_DECREF(frame); From 6868334c9485593fdffb4f7c128b8b68ebd4f16e Mon Sep 17 00:00:00 2001 From: "pre-commit-ci[bot]" <66853113+pre-commit-ci[bot]@users.noreply.github.com> Date: Tue, 1 Mar 2022 19:40:24 +0000 Subject: [PATCH 15/18] [pre-commit.ci] auto fixes from pre-commit.com hooks for more information, see https://pre-commit.ci --- include/pybind11/pybind11.h | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/include/pybind11/pybind11.h b/include/pybind11/pybind11.h index d31be89db6..9bbdc246fd 100644 --- a/include/pybind11/pybind11.h +++ b/include/pybind11/pybind11.h @@ -2645,8 +2645,7 @@ get_type_override(const void *this_ptr, const type_info *this_type, const char * if ((std::string) str(f_code->co_name) == name && f_code->co_argcount > 0) { PyObject *locals = PyEval_GetLocals(); if (locals != nullptr) { - PyObject *co_varnames = - PyObject_GetAttrString((PyObject *) f_code, "co_varnames"); + PyObject *co_varnames = PyObject_GetAttrString((PyObject *) f_code, "co_varnames"); PyObject *self_arg = PyTuple_GET_ITEM(co_varnames, 0); PyObject *self_caller = dict_getitem(locals, self_arg); Py_DECREF(co_varnames); From 1eccd5dcf25ce0a6711e8914ab9812f193221d7f Mon Sep 17 00:00:00 2001 From: Aaron Gokaslan Date: Tue, 1 Mar 2022 15:46:15 -0500 Subject: [PATCH 16/18] Simplify code slightly --- include/pybind11/pybind11.h | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/include/pybind11/pybind11.h b/include/pybind11/pybind11.h index 9bbdc246fd..52a6376aae 100644 --- a/include/pybind11/pybind11.h +++ b/include/pybind11/pybind11.h @@ -2646,8 +2646,7 @@ get_type_override(const void *this_ptr, const type_info *this_type, const char * PyObject *locals = PyEval_GetLocals(); if (locals != nullptr) { PyObject *co_varnames = PyObject_GetAttrString((PyObject *) f_code, "co_varnames"); - PyObject *self_arg = PyTuple_GET_ITEM(co_varnames, 0); - PyObject *self_caller = dict_getitem(locals, self_arg); + PyObject *self_caller = dict_getitem(locals, PyTuple_GET_ITEM(co_varnames, 0)); Py_DECREF(co_varnames); if (self_caller == self.ptr()) { Py_DECREF(f_code); From eadcb873d2ed0d098ac75b3130f308399372230c Mon Sep 17 00:00:00 2001 From: Aaron Gokaslan Date: Wed, 2 Mar 2022 12:09:46 -0500 Subject: [PATCH 17/18] Ensure co_varnames decref if dict_getitem throws --- include/pybind11/pybind11.h | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/include/pybind11/pybind11.h b/include/pybind11/pybind11.h index 38873d7dc1..4f6b2cdf41 100644 --- a/include/pybind11/pybind11.h +++ b/include/pybind11/pybind11.h @@ -2646,8 +2646,9 @@ get_type_override(const void *this_ptr, const type_info *this_type, const char * PyObject *locals = PyEval_GetLocals(); if (locals != nullptr) { PyObject *co_varnames = PyObject_GetAttrString((PyObject *) f_code, "co_varnames"); - PyObject *self_caller = dict_getitem(locals, PyTuple_GET_ITEM(co_varnames, 0)); + PyObject *self_arg = PyTuple_GET_ITEM(co_varnames, 0); Py_DECREF(co_varnames); + PyObject *self_caller = dict_getitem(locals, self_arg); if (self_caller == self.ptr()) { Py_DECREF(f_code); Py_DECREF(frame); From 4add4c230a5974eb47fa4faf2cfd265504a64f4d Mon Sep 17 00:00:00 2001 From: Aaron Gokaslan Date: Wed, 2 Mar 2022 12:32:21 -0500 Subject: [PATCH 18/18] Eager decref f_code --- include/pybind11/detail/type_caster_base.h | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/include/pybind11/detail/type_caster_base.h b/include/pybind11/detail/type_caster_base.h index 9636bcaf5f..a4154136a6 100644 --- a/include/pybind11/detail/type_caster_base.h +++ b/include/pybind11/detail/type_caster_base.h @@ -515,6 +515,7 @@ PYBIND11_NOINLINE std::string error_string() { errorString += " " + handle(f_code->co_filename).cast() + "(" + std::to_string(lineno) + "): " + handle(f_code->co_name).cast() + "\n"; + Py_DECREF(f_code); # if PY_VERSION_HEX >= 0x030900B1 auto *b_frame = PyFrame_GetBack(frame); # else @@ -523,7 +524,6 @@ PYBIND11_NOINLINE std::string error_string() { # endif Py_DECREF(frame); frame = b_frame; - Py_DECREF(f_code); } } #endif