Skip to content

Scratch PR: clone of #4293 that could be merged immediately without disrupting anything #4307

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
wants to merge 12 commits into from
Closed
10 changes: 5 additions & 5 deletions .github/workflows/upstream.yml
Original file line number Diff line number Diff line change
@@ -1,4 +1,3 @@

name: Upstream

on:
Expand All @@ -7,24 +6,25 @@ on:

concurrency:
group: upstream-${{ github.ref }}
cancel-in-progress: true
cancel-in-progress: false

env:
PIP_ONLY_BINARY: numpy
PYTEST_TIMEOUT: 300

jobs:
standard:
name: "🐍 3.11 latest internals • ubuntu-latest • x64"
name: "🐍 3.12 latest internals • ubuntu-latest • x64"
runs-on: ubuntu-latest
if: "contains(github.event.pull_request.labels.*.name, 'python dev')"

steps:
- uses: actions/checkout@v3

- name: Setup Python 3.11
- name: Setup Python 3.12
uses: actions/setup-python@v4
with:
python-version: "3.11-dev"
python-version: "3.12-dev"

- name: Setup Boost (Linux)
if: runner.os == 'Linux'
Expand Down
45 changes: 39 additions & 6 deletions include/pybind11/detail/internals.h
Original file line number Diff line number Diff line change
Expand Up @@ -421,9 +421,31 @@ inline void translate_local_exception(std::exception_ptr p) {
}
#endif

inline object get_internals_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<object>(PyEval_GetBuiltins());
#else
# if PY_VERSION_HEX < 0x03090000
PyInterpreterState *istate = _PyInterpreterState_Get();
# else
PyInterpreterState *istate = PyInterpreterState_Get();
# endif
if (istate) {
state_dict = reinterpret_borrow<object>(PyInterpreterState_GetDict(istate));
}
#endif
if (!state_dict) {
raise_from(PyExc_SystemError, "get_internals(): could not acquire state dictionary!");
}

return state_dict;
}

/// Return a reference to the current `internals` data
PYBIND11_NOINLINE internals &get_internals() {
auto **&internals_pp = get_internals_pp();
internals **&internals_pp = get_internals_pp();
if (internals_pp && *internals_pp) {
return **internals_pp;
}
Expand All @@ -445,11 +467,22 @@ 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<capsule>(builtins[id])) {
internals_pp = static_cast<internals **>(capsule(builtins[id]));
constexpr const char *id_cstr = PYBIND11_INTERNALS_ID;
str id(id_cstr);

dict state_dict = get_internals_state_dict();

if (state_dict.contains(id_cstr)) {
object o = state_dict[id];
// May fail if 'capsule_obj' is not a capsule, or if it has a different
// name. We clear the error status below in that case
internals_pp = static_cast<internals **>(PyCapsule_GetPointer(o.ptr(), id_cstr));
if (!internals_pp) {
PyErr_Clear();
}
}

if (internals_pp && *internals_pp) {
// We loaded builtins through python's builtins, 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.
Expand Down Expand Up @@ -485,7 +518,7 @@ PYBIND11_NOINLINE internals &get_internals() {
# endif
internals_ptr->istate = tstate->interp;
#endif
builtins[id] = capsule(internals_pp);
state_dict[id] = capsule(internals_pp, id_cstr);
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();
Expand Down
19 changes: 9 additions & 10 deletions tests/test_embed/test_interpreter.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -168,9 +168,8 @@ TEST_CASE("There can be only one interpreter") {
py::initialize_interpreter();
}

bool has_pybind11_internals_builtin() {
auto builtins = py::handle(PyEval_GetBuiltins());
return builtins.contains(PYBIND11_INTERNALS_ID);
bool has_pybind11_internals_state_dict() {
return py::detail::get_internals_state_dict().contains(PYBIND11_INTERNALS_ID);
};

bool has_pybind11_internals_static() {
Expand All @@ -181,7 +180,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<int>() == 3);
REQUIRE(has_pybind11_internals_builtin());
REQUIRE(has_pybind11_internals_state_dict());
REQUIRE(has_pybind11_internals_static());
REQUIRE(py::module_::import("external_module").attr("A")(123).attr("value").cast<int>()
== 123);
Expand All @@ -198,10 +197,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_pybind11_internals_state_dict());
REQUIRE_FALSE(has_pybind11_internals_static());
pybind11::detail::get_internals();
REQUIRE(has_pybind11_internals_builtin());
REQUIRE(has_pybind11_internals_state_dict());
REQUIRE(has_pybind11_internals_static());
REQUIRE(reinterpret_cast<uintptr_t>(*py::detail::get_internals_pp())
== py::module_::import("external_module").attr("internals_at")().cast<uintptr_t>());
Expand All @@ -216,13 +215,13 @@ TEST_CASE("Restart the interpreter") {
py::detail::get_internals();
*static_cast<bool *>(ran) = true;
});
REQUIRE_FALSE(has_pybind11_internals_builtin());
REQUIRE_FALSE(has_pybind11_internals_state_dict());
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_pybind11_internals_state_dict());
REQUIRE_FALSE(has_pybind11_internals_static());

// C++ modules can be reloaded.
Expand All @@ -244,7 +243,7 @@ TEST_CASE("Subinterpreter") {

REQUIRE(m.attr("add")(1, 2).cast<int>() == 3);
}
REQUIRE(has_pybind11_internals_builtin());
REQUIRE(has_pybind11_internals_state_dict());
REQUIRE(has_pybind11_internals_static());

/// Create and switch to a subinterpreter.
Expand All @@ -254,7 +253,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_pybind11_internals_state_dict());
REQUIRE(has_pybind11_internals_static());

// Modules tags should be gone.
Expand Down