fix!: modify the internals pointer-to-pointer implementation to not use thread_local (#5709)

* Refactor internals to use a holder that manages the PP

* Refactor internals to use a holder that manages the PP

* Fix cleanup/destruction issues.

* Fix one more destruction issue

Should now just be able to delete the internals PP on destruction

* Make clang-tidy happy

* Try to fix exception translators issue on certain platforms

Also fix a couple more pedantic warings

* Fix test, after internals is free'd it can come back at the same address

So instead, just make sure it was zero'd and don't try to compare the addresses.

Also a little code cleanup

* Comment tweak [skip ci]

* Switch to ifdef instead of if

* Re-enable subinterpreters in iOS

* style: pre-commit fixes

* Oops, this snuck in on merge

* fix: bump ABI version to 10

Signed-off-by: Henry Schreiner <henryschreineriii@gmail.com>

---------

Signed-off-by: Henry Schreiner <henryschreineriii@gmail.com>
Co-authored-by: pre-commit-ci[bot] <66853113+pre-commit-ci[bot]@users.noreply.github.com>
Co-authored-by: Henry Schreiner <henryschreineriii@gmail.com>
This commit is contained in:
b-pass
2025-06-03 16:02:44 -04:00
committed by GitHub
parent b19489145b
commit c7026d0d1c
9 changed files with 270 additions and 236 deletions

View File

@@ -33,4 +33,3 @@ pyodide.test-groups = ["numpy", "scipy"]
ios.test-groups = ["numpy"]
ios.xbuild-tools = ["cmake", "ninja"]
ios.environment.PIP_EXTRA_INDEX_URL = "https://pypi.anaconda.org/beeware/simple"
ios.config-settings."cmake.define.CMAKE_CXX_FLAGS" = "-DPYBIND11_HAS_SUBINTERPRETER_SUPPORT=0"

View File

@@ -25,14 +25,8 @@ bool has_state_dict_internals_obj() {
return state.contains(PYBIND11_INTERNALS_ID);
}
bool has_pybind11_internals_static() {
auto *&ipp = py::detail::get_internals_pp<py::detail::internals>();
return (ipp != nullptr) && *ipp;
}
uintptr_t get_details_as_uintptr() {
return reinterpret_cast<uintptr_t>(
py::detail::get_internals_pp<py::detail::internals>()->get());
return reinterpret_cast<uintptr_t>(py::detail::get_internals_pp_manager().get_pp()->get());
}
class Widget {
@@ -278,7 +272,6 @@ TEST_CASE("Restart the interpreter") {
// Verify pre-restart state.
REQUIRE(py::module_::import("widget_module").attr("add")(1, 2).cast<int>() == 3);
REQUIRE(has_state_dict_internals_obj());
REQUIRE(has_pybind11_internals_static());
REQUIRE(py::module_::import("external_module").attr("A")(123).attr("value").cast<int>()
== 123);
@@ -295,10 +288,10 @@ TEST_CASE("Restart the interpreter") {
// Internals are deleted after a restart.
REQUIRE_FALSE(has_state_dict_internals_obj());
REQUIRE_FALSE(has_pybind11_internals_static());
REQUIRE(get_details_as_uintptr() == 0);
pybind11::detail::get_internals();
REQUIRE(has_state_dict_internals_obj());
REQUIRE(has_pybind11_internals_static());
REQUIRE(get_details_as_uintptr() != 0);
REQUIRE(get_details_as_uintptr()
== py::module_::import("external_module").attr("internals_at")().cast<uintptr_t>());
@@ -311,18 +304,15 @@ TEST_CASE("Restart the interpreter") {
= py::capsule(&ran, [](void *ran) {
py::detail::get_internals();
REQUIRE(has_state_dict_internals_obj());
REQUIRE(has_pybind11_internals_static());
*static_cast<bool *>(ran) = true;
});
REQUIRE_FALSE(has_state_dict_internals_obj());
REQUIRE_FALSE(has_pybind11_internals_static());
REQUIRE_FALSE(ran);
py::finalize_interpreter();
REQUIRE(ran);
REQUIRE_FALSE(has_pybind11_internals_static());
py::initialize_interpreter();
REQUIRE_FALSE(has_state_dict_internals_obj());
REQUIRE_FALSE(has_pybind11_internals_static());
REQUIRE(get_details_as_uintptr() == 0);
// C++ modules can be reloaded.
auto cpp_module = py::module_::import("widget_module");
@@ -348,7 +338,6 @@ TEST_CASE("Threads") {
// Restart interpreter to ensure threads are not initialized
py::finalize_interpreter();
py::initialize_interpreter();
REQUIRE_FALSE(has_pybind11_internals_static());
constexpr auto num_threads = 10;
auto locals = py::dict("count"_a = 0);

View File

@@ -17,20 +17,21 @@ namespace py = pybind11;
using namespace py::literals;
bool has_state_dict_internals_obj();
bool has_pybind11_internals_static();
uintptr_t get_details_as_uintptr();
void unsafe_reset_internals_for_single_interpreter() {
// unsafe normally, but for subsequent tests, put this back.. we know there are no threads
// running and only 1 interpreter
py::detail::get_internals_pp_manager().unref();
py::detail::get_local_internals_pp_manager().unref();
py::detail::get_num_interpreters_seen() = 1;
py::detail::get_internals_pp<py::detail::internals>() = nullptr;
py::detail::get_internals();
py::detail::get_internals_pp<py::detail::local_internals>() = nullptr;
py::detail::get_local_internals();
}
TEST_CASE("Single Subinterpreter") {
unsafe_reset_internals_for_single_interpreter();
py::module_::import("external_module"); // in the main interpreter
// Add tags to the modules in the main interpreter and test the basics.
@@ -42,7 +43,6 @@ TEST_CASE("Single Subinterpreter") {
REQUIRE(m.attr("add")(1, 2).cast<int>() == 3);
}
REQUIRE(has_state_dict_internals_obj());
REQUIRE(has_pybind11_internals_static());
auto main_int
= py::module_::import("external_module").attr("internals_at")().cast<uintptr_t>();
@@ -52,14 +52,13 @@ TEST_CASE("Single Subinterpreter") {
py::scoped_subinterpreter ssi;
// The subinterpreter has internals populated
REQUIRE(has_pybind11_internals_static());
REQUIRE(has_state_dict_internals_obj());
py::list(py::module_::import("sys").attr("path")).append(py::str("."));
auto ext_int
= py::module_::import("external_module").attr("internals_at")().cast<uintptr_t>();
py::detail::get_internals();
REQUIRE(has_pybind11_internals_static());
REQUIRE(get_details_as_uintptr() == ext_int);
REQUIRE(ext_int != main_int);
@@ -205,6 +204,8 @@ TEST_CASE("GIL Subinterpreter") {
}
TEST_CASE("Multiple Subinterpreters") {
unsafe_reset_internals_for_single_interpreter();
// Make sure the module is in the main interpreter and save its pointer
auto *main_ext = py::module_::import("external_module").ptr();
auto main_int