From 40b9586249f8defa23b4e5895fd049bc19fe56e4 Mon Sep 17 00:00:00 2001 From: Xuehai Pan Date: Mon, 19 May 2025 01:29:04 +0800 Subject: [PATCH 1/4] Add semi-public API: `pybind11::detail::is_holder_constructed` --- include/pybind11/detail/value_and_holder.h | 12 ++++++++++++ 1 file changed, 12 insertions(+) diff --git a/include/pybind11/detail/value_and_holder.h b/include/pybind11/detail/value_and_holder.h index 64c55cc595..87c92f8e49 100644 --- a/include/pybind11/detail/value_and_holder.h +++ b/include/pybind11/detail/value_and_holder.h @@ -74,5 +74,17 @@ struct value_and_holder { } }; +// This is a semi-public API to check if the corresponding instance has been constructed with a +// holder. That is, if the instance has been constructed with a holder, the `__init__` method is +// called and the C++ object is valid. Otherwise, the C++ object might only be allocated, but not +// initialized. This will lead to **SEGMENTATION FAULTS** if the C++ object is used in any way. +// Example usage: https://pybind11.readthedocs.io/en/stable/advanced/classes.html#custom-type-setup +// for `tp_traverse` and `tp_clear` implementations. +// WARNING: The caller is responsible for ensuring that the `reinterpret_cast` is valid. +inline bool is_holder_constructed(PyObject *obj) { + auto *const instance = reinterpret_cast(obj); + return instance->get_value_and_holder().holder_constructed(); +} + PYBIND11_NAMESPACE_END(detail) PYBIND11_NAMESPACE_END(PYBIND11_NAMESPACE) From ad587ebf412995f18f0347b892797ca221d49f1f Mon Sep 17 00:00:00 2001 From: Xuehai Pan Date: Mon, 19 May 2025 01:29:42 +0800 Subject: [PATCH 2/4] Update docs for `pybind11::custom_type_setup` --- docs/advanced/classes.rst | 16 ++++++++++++---- 1 file changed, 12 insertions(+), 4 deletions(-) diff --git a/docs/advanced/classes.rst b/docs/advanced/classes.rst index 5f11215d06..d494a8a616 100644 --- a/docs/advanced/classes.rst +++ b/docs/advanced/classes.rst @@ -1384,13 +1384,21 @@ You can do that using ``py::custom_type_setup``: auto *type = &heap_type->ht_type; type->tp_flags |= Py_TPFLAGS_HAVE_GC; type->tp_traverse = [](PyObject *self_base, visitproc visit, void *arg) { - auto &self = py::cast(py::handle(self_base)); - Py_VISIT(self.value.ptr()); + // https://docs.python.org/3/c-api/typeobj.html#c.PyTypeObject.tp_traverse + #if PY_VERSION_HEX >= 0x03090000 // Python 3.9 + Py_VISIT(Py_TYPE(self_base)); + #endif + if (py::detail::is_holder_constructed(self_base)) { + auto &self = py::cast(py::handle(self_base)); + Py_VISIT(self.value.ptr()); + } return 0; }; type->tp_clear = [](PyObject *self_base) { - auto &self = py::cast(py::handle(self_base)); - self.value = py::none(); + if (py::detail::is_holder_constructed(self_base)) { + auto &self = py::cast(py::handle(self_base)); + self.value = py::none(); + } return 0; }; })); From 015be881dfc8590ac41c7626f62cc296a3dcaa6c Mon Sep 17 00:00:00 2001 From: Xuehai Pan Date: Mon, 19 May 2025 02:02:30 +0800 Subject: [PATCH 3/4] Update tests in `test_custom_type_setup.cpp` --- tests/test_custom_type_setup.cpp | 16 ++++++++++++---- 1 file changed, 12 insertions(+), 4 deletions(-) diff --git a/tests/test_custom_type_setup.cpp b/tests/test_custom_type_setup.cpp index 42fae05d5d..96ca6b0d57 100644 --- a/tests/test_custom_type_setup.cpp +++ b/tests/test_custom_type_setup.cpp @@ -26,13 +26,21 @@ TEST_SUBMODULE(custom_type_setup, m) { auto *type = &heap_type->ht_type; type->tp_flags |= Py_TPFLAGS_HAVE_GC; type->tp_traverse = [](PyObject *self_base, visitproc visit, void *arg) { - auto &self = py::cast(py::handle(self_base)); - Py_VISIT(self.value.ptr()); + // https://docs.python.org/3/c-api/typeobj.html#c.PyTypeObject.tp_traverse +#if PY_VERSION_HEX >= 0x03090000 // Python 3.9 + Py_VISIT(Py_TYPE(self_base)); +#endif + if (py::detail::is_holder_constructed(self_base)) { + auto &self = py::cast(py::handle(self_base)); + Py_VISIT(self.value.ptr()); + } return 0; }; type->tp_clear = [](PyObject *self_base) { - auto &self = py::cast(py::handle(self_base)); - self.value = py::none(); + if (py::detail::is_holder_constructed(self_base)) { + auto &self = py::cast(py::handle(self_base)); + self.value = py::none(); + } return 0; }; })); From 965110fb2bb8bec6b33e91d9faa4d328cf60669e Mon Sep 17 00:00:00 2001 From: Xuehai Pan Date: Mon, 19 May 2025 02:16:47 +0800 Subject: [PATCH 4/4] Apply suggestions from code review --- docs/advanced/classes.rst | 2 +- tests/test_custom_type_setup.cpp | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/docs/advanced/classes.rst b/docs/advanced/classes.rst index d494a8a616..a8c2da17b2 100644 --- a/docs/advanced/classes.rst +++ b/docs/advanced/classes.rst @@ -1385,7 +1385,7 @@ You can do that using ``py::custom_type_setup``: type->tp_flags |= Py_TPFLAGS_HAVE_GC; type->tp_traverse = [](PyObject *self_base, visitproc visit, void *arg) { // https://docs.python.org/3/c-api/typeobj.html#c.PyTypeObject.tp_traverse - #if PY_VERSION_HEX >= 0x03090000 // Python 3.9 + #if PY_VERSION_HEX >= 0x03090000 Py_VISIT(Py_TYPE(self_base)); #endif if (py::detail::is_holder_constructed(self_base)) { diff --git a/tests/test_custom_type_setup.cpp b/tests/test_custom_type_setup.cpp index 96ca6b0d57..ef87ed484d 100644 --- a/tests/test_custom_type_setup.cpp +++ b/tests/test_custom_type_setup.cpp @@ -27,7 +27,7 @@ TEST_SUBMODULE(custom_type_setup, m) { type->tp_flags |= Py_TPFLAGS_HAVE_GC; type->tp_traverse = [](PyObject *self_base, visitproc visit, void *arg) { // https://docs.python.org/3/c-api/typeobj.html#c.PyTypeObject.tp_traverse -#if PY_VERSION_HEX >= 0x03090000 // Python 3.9 +#if PY_VERSION_HEX >= 0x03090000 Py_VISIT(Py_TYPE(self_base)); #endif if (py::detail::is_holder_constructed(self_base)) {