Skip to content

Commit c2e6b38

Browse files
committed
Modified version of PR pybind#4293 by @wjakob
Modifications are: * Backward compatibility (no ABI break), as originally under PR pybind#4307. * Naming: `get_python_state_dict()`, `has_pybind11_internals_capsule()` * Report error retrieving `internals**` from capsule instead of clearing it. Locally tested with ASAN, MSAN, TSAN, UBSAN (Google-internal toolchain).
1 parent a0f43c9 commit c2e6b38

File tree

2 files changed

+47
-16
lines changed

2 files changed

+47
-16
lines changed

include/pybind11/detail/internals.h

Lines changed: 38 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -424,9 +424,30 @@ inline void translate_local_exception(std::exception_ptr p) {
424424
}
425425
#endif
426426

427+
inline object get_python_state_dict() {
428+
object state_dict;
429+
#if (PYBIND11_INTERNALS_VERSION <= 4 && PY_VERSION_HEX < 0x030C0000) \
430+
|| PY_VERSION_HEX < 0x03080000 || defined(PYPY_VERSION)
431+
state_dict = reinterpret_borrow<object>(PyEval_GetBuiltins());
432+
#else
433+
# if PY_VERSION_HEX < 0x03090000
434+
PyInterpreterState *istate = _PyInterpreterState_Get();
435+
# else
436+
PyInterpreterState *istate = PyInterpreterState_Get();
437+
# endif
438+
if (istate) {
439+
state_dict = reinterpret_borrow<object>(PyInterpreterState_GetDict(istate));
440+
}
441+
#endif
442+
if (!state_dict) {
443+
raise_from(PyExc_SystemError, "pybind11::detail::get_python_state_dict() FAILED");
444+
}
445+
return state_dict;
446+
}
447+
427448
/// Return a reference to the current `internals` data
428449
PYBIND11_NOINLINE internals &get_internals() {
429-
auto **&internals_pp = get_internals_pp();
450+
internals **&internals_pp = get_internals_pp();
430451
if (internals_pp && *internals_pp) {
431452
return **internals_pp;
432453
}
@@ -448,11 +469,22 @@ PYBIND11_NOINLINE internals &get_internals() {
448469
#endif
449470
error_scope err_scope;
450471

451-
PYBIND11_STR_TYPE id(PYBIND11_INTERNALS_ID);
452-
auto builtins = handle(PyEval_GetBuiltins());
453-
if (builtins.contains(id) && isinstance<capsule>(builtins[id])) {
454-
internals_pp = static_cast<internals **>(capsule(builtins[id]));
472+
constexpr const char *id_cstr = PYBIND11_INTERNALS_ID;
473+
str id(id_cstr);
455474

475+
dict state_dict = get_python_state_dict();
476+
477+
if (state_dict.contains(id_cstr)) {
478+
void *raw_ptr = PyCapsule_GetPointer(state_dict[id].ptr(), id_cstr);
479+
if (raw_ptr == nullptr) {
480+
raise_from(
481+
PyExc_SystemError,
482+
"pybind11::detail::get_internals(): Retrieve internals** from capsule FAILED");
483+
}
484+
internals_pp = static_cast<internals **>(raw_ptr);
485+
}
486+
487+
if (internals_pp && *internals_pp) {
456488
// We loaded builtins through python's builtins, which means that our `error_already_set`
457489
// and `builtin_exception` may be different local classes than the ones set up in the
458490
// initial exception translator, below, so add another for our local exception classes.
@@ -488,7 +520,7 @@ PYBIND11_NOINLINE internals &get_internals() {
488520
# endif
489521
internals_ptr->istate = tstate->interp;
490522
#endif
491-
builtins[id] = capsule(internals_pp);
523+
state_dict[id] = capsule(internals_pp, id_cstr);
492524
internals_ptr->registered_exception_translators.push_front(&translate_exception);
493525
internals_ptr->static_property_type = make_static_property_type();
494526
internals_ptr->default_metaclass = make_default_metaclass();

tests/test_embed/test_interpreter.cpp

Lines changed: 9 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -168,9 +168,8 @@ TEST_CASE("There can be only one interpreter") {
168168
py::initialize_interpreter();
169169
}
170170

171-
bool has_pybind11_internals_builtin() {
172-
auto builtins = py::handle(PyEval_GetBuiltins());
173-
return builtins.contains(PYBIND11_INTERNALS_ID);
171+
bool has_pybind11_internals_capsule() {
172+
return py::detail::get_python_state_dict().contains(PYBIND11_INTERNALS_ID);
174173
};
175174

176175
bool has_pybind11_internals_static() {
@@ -181,7 +180,7 @@ bool has_pybind11_internals_static() {
181180
TEST_CASE("Restart the interpreter") {
182181
// Verify pre-restart state.
183182
REQUIRE(py::module_::import("widget_module").attr("add")(1, 2).cast<int>() == 3);
184-
REQUIRE(has_pybind11_internals_builtin());
183+
REQUIRE(has_pybind11_internals_capsule());
185184
REQUIRE(has_pybind11_internals_static());
186185
REQUIRE(py::module_::import("external_module").attr("A")(123).attr("value").cast<int>()
187186
== 123);
@@ -198,10 +197,10 @@ TEST_CASE("Restart the interpreter") {
198197
REQUIRE(Py_IsInitialized() == 1);
199198

200199
// Internals are deleted after a restart.
201-
REQUIRE_FALSE(has_pybind11_internals_builtin());
200+
REQUIRE_FALSE(has_pybind11_internals_capsule());
202201
REQUIRE_FALSE(has_pybind11_internals_static());
203202
pybind11::detail::get_internals();
204-
REQUIRE(has_pybind11_internals_builtin());
203+
REQUIRE(has_pybind11_internals_capsule());
205204
REQUIRE(has_pybind11_internals_static());
206205
REQUIRE(reinterpret_cast<uintptr_t>(*py::detail::get_internals_pp())
207206
== py::module_::import("external_module").attr("internals_at")().cast<uintptr_t>());
@@ -216,13 +215,13 @@ TEST_CASE("Restart the interpreter") {
216215
py::detail::get_internals();
217216
*static_cast<bool *>(ran) = true;
218217
});
219-
REQUIRE_FALSE(has_pybind11_internals_builtin());
218+
REQUIRE_FALSE(has_pybind11_internals_capsule());
220219
REQUIRE_FALSE(has_pybind11_internals_static());
221220
REQUIRE_FALSE(ran);
222221
py::finalize_interpreter();
223222
REQUIRE(ran);
224223
py::initialize_interpreter();
225-
REQUIRE_FALSE(has_pybind11_internals_builtin());
224+
REQUIRE_FALSE(has_pybind11_internals_capsule());
226225
REQUIRE_FALSE(has_pybind11_internals_static());
227226

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

245244
REQUIRE(m.attr("add")(1, 2).cast<int>() == 3);
246245
}
247-
REQUIRE(has_pybind11_internals_builtin());
246+
REQUIRE(has_pybind11_internals_capsule());
248247
REQUIRE(has_pybind11_internals_static());
249248

250249
/// Create and switch to a subinterpreter.
@@ -254,7 +253,7 @@ TEST_CASE("Subinterpreter") {
254253
// Subinterpreters get their own copy of builtins. detail::get_internals() still
255254
// works by returning from the static variable, i.e. all interpreters share a single
256255
// global pybind11::internals;
257-
REQUIRE_FALSE(has_pybind11_internals_builtin());
256+
REQUIRE_FALSE(has_pybind11_internals_capsule());
258257
REQUIRE(has_pybind11_internals_static());
259258

260259
// Modules tags should be gone.

0 commit comments

Comments
 (0)