From 12ef7766f848181b7008902fb7c78c75aa1900a3 Mon Sep 17 00:00:00 2001 From: "Ralf W. Grosse-Kunstleve" Date: Tue, 14 Mar 2023 07:41:36 -0700 Subject: [PATCH 1/6] Introduce `get_python_state_dict()` --- include/pybind11/detail/internals.h | 47 +++++++++++++++++++++++---- include/pybind11/embed.h | 10 +++--- tests/test_embed/test_interpreter.cpp | 22 ++++++------- 3 files changed, 55 insertions(+), 24 deletions(-) diff --git a/include/pybind11/detail/internals.h b/include/pybind11/detail/internals.h index 05e36ad18b..ed37adddda 100644 --- a/include/pybind11/detail/internals.h +++ b/include/pybind11/detail/internals.h @@ -421,6 +421,39 @@ inline void translate_local_exception(std::exception_ptr p) { } #endif +inline object get_python_state_dict() { + object state_dict; +#if (PYBIND11_INTERNALS_VERSION <= 4 && PY_VERSION_HEX < 0x030C0000) \ + || PY_VERSION_HEX < 0x03080000 || defined(PYPY_VERSION) + state_dict = reinterpret_borrow(PyEval_GetBuiltins()); +#else +# if PY_VERSION_HEX < 0x03090000 + PyInterpreterState *istate = _PyInterpreterState_Get(); +# else + PyInterpreterState *istate = PyInterpreterState_Get(); +# endif + if (istate) { + state_dict = reinterpret_borrow(PyInterpreterState_GetDict(istate)); + } +#endif + if (!state_dict) { + raise_from(PyExc_SystemError, "pybind11::detail::get_python_state_dict() FAILED"); + } + return state_dict; +} + +inline object get_internals_obj_from_state_dict(handle state_dict) { + return reinterpret_borrow(dict_getitemstring(state_dict.ptr(), PYBIND11_INTERNALS_ID)); +} + +inline internals **get_internals_pp_from_capsule(handle obj) { + void *raw_ptr = PyCapsule_GetPointer(obj.ptr(), /*name=*/nullptr); + if (raw_ptr == nullptr) { + raise_from(PyExc_SystemError, "pybind11::detail::get_internals_pp_from_capsule() FAILED"); + } + return static_cast(raw_ptr); +} + /// Return a reference to the current `internals` data PYBIND11_NOINLINE internals &get_internals() { auto **&internals_pp = get_internals_pp(); @@ -445,12 +478,12 @@ PYBIND11_NOINLINE internals &get_internals() { #endif error_scope err_scope; - PYBIND11_STR_TYPE id(PYBIND11_INTERNALS_ID); - auto builtins = handle(PyEval_GetBuiltins()); - if (builtins.contains(id) && isinstance(builtins[id])) { - internals_pp = static_cast(capsule(builtins[id])); - - // We loaded builtins through python's builtins, which means that our `error_already_set` + dict state_dict = get_python_state_dict(); + if (object internals_obj = get_internals_obj_from_state_dict(state_dict)) { + internals_pp = get_internals_pp_from_capsule(internals_obj); + } + if (internals_pp && *internals_pp) { + // We loaded the internals through `state_dict`, which means that our `error_already_set` // and `builtin_exception` may be different local classes than the ones set up in the // initial exception translator, below, so add another for our local exception classes. // @@ -484,7 +517,7 @@ PYBIND11_NOINLINE internals &get_internals() { # endif internals_ptr->istate = tstate->interp; #endif - builtins[id] = capsule(internals_pp); + state_dict[PYBIND11_INTERNALS_ID] = capsule(internals_pp); internals_ptr->registered_exception_translators.push_front(&translate_exception); internals_ptr->static_property_type = make_static_property_type(); internals_ptr->default_metaclass = make_default_metaclass(); diff --git a/include/pybind11/embed.h b/include/pybind11/embed.h index 5a175b1341..caa14f4a05 100644 --- a/include/pybind11/embed.h +++ b/include/pybind11/embed.h @@ -243,16 +243,14 @@ inline void initialize_interpreter(bool init_signal_handlers = true, \endrst */ inline void finalize_interpreter() { - handle builtins(PyEval_GetBuiltins()); - const char *id = PYBIND11_INTERNALS_ID; - // Get the internals pointer (without creating it if it doesn't exist). It's possible for the // internals to be created during Py_Finalize() (e.g. if a py::capsule calls `get_internals()` // during destruction), so we get the pointer-pointer here and check it after Py_Finalize(). detail::internals **internals_ptr_ptr = detail::get_internals_pp(); - // It could also be stashed in builtins, so look there too: - if (builtins.contains(id) && isinstance(builtins[id])) { - internals_ptr_ptr = capsule(builtins[id]); + // It could also be stashed in state_dict, so look there too: + if (object internals_obj + = get_internals_obj_from_state_dict(detail::get_python_state_dict())) { + internals_ptr_ptr = detail::get_internals_pp_from_capsule(internals_obj); } // Local internals contains data managed by the current interpreter, so we must clear them to // avoid undefined behaviors when initializing another interpreter diff --git a/tests/test_embed/test_interpreter.cpp b/tests/test_embed/test_interpreter.cpp index e54e822708..c6c8a22d98 100644 --- a/tests/test_embed/test_interpreter.cpp +++ b/tests/test_embed/test_interpreter.cpp @@ -255,10 +255,10 @@ TEST_CASE("Add program dir to path using PyConfig") { } #endif -bool has_pybind11_internals_builtin() { - auto builtins = py::handle(PyEval_GetBuiltins()); - return builtins.contains(PYBIND11_INTERNALS_ID); -}; +bool has_state_dict_internals_obj() { + return bool( + py::detail::get_internals_obj_from_state_dict(py::detail::get_python_state_dict())); +} bool has_pybind11_internals_static() { auto **&ipp = py::detail::get_internals_pp(); @@ -268,7 +268,7 @@ bool has_pybind11_internals_static() { TEST_CASE("Restart the interpreter") { // Verify pre-restart state. REQUIRE(py::module_::import("widget_module").attr("add")(1, 2).cast() == 3); - REQUIRE(has_pybind11_internals_builtin()); + REQUIRE(has_state_dict_internals_obj()); REQUIRE(has_pybind11_internals_static()); REQUIRE(py::module_::import("external_module").attr("A")(123).attr("value").cast() == 123); @@ -285,10 +285,10 @@ TEST_CASE("Restart the interpreter") { REQUIRE(Py_IsInitialized() == 1); // Internals are deleted after a restart. - REQUIRE_FALSE(has_pybind11_internals_builtin()); + REQUIRE_FALSE(has_state_dict_internals_obj()); REQUIRE_FALSE(has_pybind11_internals_static()); pybind11::detail::get_internals(); - REQUIRE(has_pybind11_internals_builtin()); + REQUIRE(has_state_dict_internals_obj()); REQUIRE(has_pybind11_internals_static()); REQUIRE(reinterpret_cast(*py::detail::get_internals_pp()) == py::module_::import("external_module").attr("internals_at")().cast()); @@ -303,13 +303,13 @@ TEST_CASE("Restart the interpreter") { py::detail::get_internals(); *static_cast(ran) = true; }); - REQUIRE_FALSE(has_pybind11_internals_builtin()); + REQUIRE_FALSE(has_state_dict_internals_obj()); REQUIRE_FALSE(has_pybind11_internals_static()); REQUIRE_FALSE(ran); py::finalize_interpreter(); REQUIRE(ran); py::initialize_interpreter(); - REQUIRE_FALSE(has_pybind11_internals_builtin()); + REQUIRE_FALSE(has_state_dict_internals_obj()); REQUIRE_FALSE(has_pybind11_internals_static()); // C++ modules can be reloaded. @@ -331,7 +331,7 @@ TEST_CASE("Subinterpreter") { REQUIRE(m.attr("add")(1, 2).cast() == 3); } - REQUIRE(has_pybind11_internals_builtin()); + REQUIRE(has_state_dict_internals_obj()); REQUIRE(has_pybind11_internals_static()); /// Create and switch to a subinterpreter. @@ -341,7 +341,7 @@ TEST_CASE("Subinterpreter") { // Subinterpreters get their own copy of builtins. detail::get_internals() still // works by returning from the static variable, i.e. all interpreters share a single // global pybind11::internals; - REQUIRE_FALSE(has_pybind11_internals_builtin()); + REQUIRE_FALSE(has_state_dict_internals_obj()); REQUIRE(has_pybind11_internals_static()); // Modules tags should be gone. From 1c5bc8538551112d5625bd3bc8b172c7536474e3 Mon Sep 17 00:00:00 2001 From: "Ralf W. Grosse-Kunstleve" Date: Sun, 26 Mar 2023 10:07:12 -0700 Subject: [PATCH 2/6] Conditional version bump for Python 3.12+ --- include/pybind11/detail/internals.h | 7 ++++++- 1 file changed, 6 insertions(+), 1 deletion(-) diff --git a/include/pybind11/detail/internals.h b/include/pybind11/detail/internals.h index ed37adddda..91b26ac25a 100644 --- a/include/pybind11/detail/internals.h +++ b/include/pybind11/detail/internals.h @@ -34,7 +34,12 @@ /// further ABI-incompatible changes may be made before the ABI is officially /// changed to the new version. #ifndef PYBIND11_INTERNALS_VERSION -# define PYBIND11_INTERNALS_VERSION 4 +# if PY_VERSION_HEX >= 0x030C0000 +// Version bump for Python 3.12+, before first 3.12 beta release. +# define PYBIND11_INTERNALS_VERSION 5 +# else +# define PYBIND11_INTERNALS_VERSION 4 +# endif #endif PYBIND11_NAMESPACE_BEGIN(PYBIND11_NAMESPACE) From e8b34b473546c469f11cd2c2a0cb098ca12134e5 Mon Sep 17 00:00:00 2001 From: "Ralf W. Grosse-Kunstleve" Date: Sun, 26 Mar 2023 11:15:09 -0700 Subject: [PATCH 3/6] Shuffle subexpressions to make the condition easier to understand (no change to logic). --- include/pybind11/detail/internals.h | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/include/pybind11/detail/internals.h b/include/pybind11/detail/internals.h index 91b26ac25a..648df019b6 100644 --- a/include/pybind11/detail/internals.h +++ b/include/pybind11/detail/internals.h @@ -428,8 +428,8 @@ inline void translate_local_exception(std::exception_ptr p) { inline object get_python_state_dict() { object state_dict; -#if (PYBIND11_INTERNALS_VERSION <= 4 && PY_VERSION_HEX < 0x030C0000) \ - || PY_VERSION_HEX < 0x03080000 || defined(PYPY_VERSION) +#if PY_VERSION_HEX < 0x03080000 \ + || (PYBIND11_INTERNALS_VERSION <= 4 && PY_VERSION_HEX < 0x030C0000) || defined(PYPY_VERSION) state_dict = reinterpret_borrow(PyEval_GetBuiltins()); #else # if PY_VERSION_HEX < 0x03090000 From e23ddd83b0e17ed3c1c089713d693bb0e003ee60 Mon Sep 17 00:00:00 2001 From: "Ralf W. Grosse-Kunstleve" Date: Mon, 27 Mar 2023 09:32:24 -0700 Subject: [PATCH 4/6] Make pybind11 ABI version 5 the minimum for Python 3.12+ (as suggested by @Lalaland) --- include/pybind11/detail/internals.h | 7 +++++-- 1 file changed, 5 insertions(+), 2 deletions(-) diff --git a/include/pybind11/detail/internals.h b/include/pybind11/detail/internals.h index 648df019b6..fa4cea7e4b 100644 --- a/include/pybind11/detail/internals.h +++ b/include/pybind11/detail/internals.h @@ -42,6 +42,10 @@ # endif #endif +// This requirement is mainly to reduce the support burden (see PR #4570). +static_assert(PY_VERSION_HEX < 0x030C0000 || PYBIND11_INTERNALS_VERSION >= 5, + "pybind11 ABI version 5 is the minimum for Python 3.12+"); + PYBIND11_NAMESPACE_BEGIN(PYBIND11_NAMESPACE) using ExceptionTranslator = void (*)(std::exception_ptr); @@ -428,8 +432,7 @@ inline void translate_local_exception(std::exception_ptr p) { inline object get_python_state_dict() { object state_dict; -#if PY_VERSION_HEX < 0x03080000 \ - || (PYBIND11_INTERNALS_VERSION <= 4 && PY_VERSION_HEX < 0x030C0000) || defined(PYPY_VERSION) +#if PYBIND11_INTERNALS_VERSION <= 4 || PY_VERSION_HEX < 0x03080000 state_dict = reinterpret_borrow(PyEval_GetBuiltins()); #else # if PY_VERSION_HEX < 0x03090000 From 5d4716a620ba7f65ae791823ecce050ff6b7b472 Mon Sep 17 00:00:00 2001 From: "Ralf W. Grosse-Kunstleve" Date: Mon, 27 Mar 2023 10:11:27 -0700 Subject: [PATCH 5/6] Add back condition for PYPY_VERSION, but keep it open for future PyPy versions. --- include/pybind11/detail/internals.h | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/include/pybind11/detail/internals.h b/include/pybind11/detail/internals.h index fa4cea7e4b..d2ea1543bc 100644 --- a/include/pybind11/detail/internals.h +++ b/include/pybind11/detail/internals.h @@ -432,7 +432,8 @@ inline void translate_local_exception(std::exception_ptr p) { inline object get_python_state_dict() { object state_dict; -#if PYBIND11_INTERNALS_VERSION <= 4 || PY_VERSION_HEX < 0x03080000 +#if PYBIND11_INTERNALS_VERSION <= 4 || PY_VERSION_HEX < 0x03080000 \ + || (defined(PYPY_VERSION) && PY_VERSION_HEX <= 0x03090000) state_dict = reinterpret_borrow(PyEval_GetBuiltins()); #else # if PY_VERSION_HEX < 0x03090000 From 772961211bfc28f79c8e975ec230b7c035137552 Mon Sep 17 00:00:00 2001 From: "Ralf W. Grosse-Kunstleve" Date: Mon, 27 Mar 2023 10:41:17 -0700 Subject: [PATCH 6/6] Fall back to simple `|| defined(PYPY_VERSION)`. `PY_VERSION_HEX` does not appear to be meaningful with PyPy. --- include/pybind11/detail/internals.h | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/include/pybind11/detail/internals.h b/include/pybind11/detail/internals.h index d2ea1543bc..8364ebd1e0 100644 --- a/include/pybind11/detail/internals.h +++ b/include/pybind11/detail/internals.h @@ -432,8 +432,7 @@ inline void translate_local_exception(std::exception_ptr p) { inline object get_python_state_dict() { object state_dict; -#if PYBIND11_INTERNALS_VERSION <= 4 || PY_VERSION_HEX < 0x03080000 \ - || (defined(PYPY_VERSION) && PY_VERSION_HEX <= 0x03090000) +#if PYBIND11_INTERNALS_VERSION <= 4 || PY_VERSION_HEX < 0x03080000 || defined(PYPY_VERSION) state_dict = reinterpret_borrow(PyEval_GetBuiltins()); #else # if PY_VERSION_HEX < 0x03090000