Zengtudor/pybind11
1
0
Fork 0
mirror of https://github.com/pybind/pybind11.git synced 2024-11-14 09:34:46 +00:00
Code Issues Packages Projects Releases Wiki Activity

Destroy internals if created during Py_Finalize()

Browse Source 
Py_Finalize could potentially invoke code that calls `get_internals()`,
which could create a new internals object if one didn't exist.
`finalize_interpreter()` didn't catch this because it only used the
pre-finalize interpreter pointer status; if this happens, it results in
the internals pointer not being properly destroyed with the interpreter,
which leaks, and also causes a `get_internals()` under a future
interpreter to return an internals object that is wrong in various ways.
This commit is contained in:
Jason Rhinelander 2017-06-07 12:35:39 -03:00
parent ef2dfd47ba
commit 4edb1ce20c
2 changed files with 36 additions and 7 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

6
include/pybind11/embed.h
View File

@ -143,7 +143,11 @@ inline void finalize_interpreter() {
handle builtins(PyEval_GetBuiltins()); handle builtins(PyEval_GetBuiltins());
const char *id = PYBIND11_INTERNALS_ID; const char *id = PYBIND11_INTERNALS_ID;
detail::internals **internals_ptr_ptr = nullptr; // 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_ptr();
// It could also be stashed in builtins, so look there too:
if (builtins.contains(id) && isinstance<capsule>(builtins[id])) if (builtins.contains(id) && isinstance<capsule>(builtins[id]))
internals_ptr_ptr = capsule(builtins[id]); internals_ptr_ptr = capsule(builtins[id]);

37
tests/test_embed/test_interpreter.cpp
View File

@ -84,15 +84,20 @@ TEST_CASE("There can be only one interpreter") {
py::initialize_interpreter(); py::initialize_interpreter();
} }
bool has_pybind11_internals() { bool has_pybind11_internals_builtin() {
auto builtins = py::handle(PyEval_GetBuiltins()); auto builtins = py::handle(PyEval_GetBuiltins());
return builtins.contains(PYBIND11_INTERNALS_ID); return builtins.contains(PYBIND11_INTERNALS_ID);
}; };
bool has_pybind11_internals_static() {
return py::detail::get_internals_ptr() != nullptr;
}
TEST_CASE("Restart the interpreter") { TEST_CASE("Restart the interpreter") {
// Verify pre-restart state. // Verify pre-restart state.
REQUIRE(py::module::import("widget_module").attr("add")(1, 2).cast<int>() == 3); REQUIRE(py::module::import("widget_module").attr("add")(1, 2).cast<int>() == 3);
REQUIRE(has_pybind11_internals()); REQUIRE(has_pybind11_internals_builtin());
REQUIRE(has_pybind11_internals_static());
// Restart the interpreter. // Restart the interpreter.
py::finalize_interpreter(); py::finalize_interpreter();
@ -102,9 +107,27 @@ TEST_CASE("Restart the interpreter") {
REQUIRE(Py_IsInitialized() == 1); REQUIRE(Py_IsInitialized() == 1);
// Internals are deleted after a restart. // Internals are deleted after a restart.
REQUIRE_FALSE(has_pybind11_internals()); REQUIRE_FALSE(has_pybind11_internals_builtin());
REQUIRE_FALSE(has_pybind11_internals_static());
pybind11::detail::get_internals(); pybind11::detail::get_internals();
REQUIRE(has_pybind11_internals()); REQUIRE(has_pybind11_internals_builtin());
REQUIRE(has_pybind11_internals_static());
// Make sure that an interpreter with no get_internals() created until finalize still gets the
// internals destroyed
py::finalize_interpreter();
py::initialize_interpreter();
bool ran = false;
py::module::import("__main__").attr("internals_destroy_test") =
py::capsule(&ran, [](void *ran) { py::detail::get_internals(); *static_cast<bool *>(ran) = true; });
REQUIRE_FALSE(has_pybind11_internals_builtin());
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_static());
// C++ modules can be reloaded. // C++ modules can be reloaded.
auto cpp_module = py::module::import("widget_module"); auto cpp_module = py::module::import("widget_module");
@ -125,7 +148,8 @@ TEST_CASE("Subinterpreter") {
REQUIRE(m.attr("add")(1, 2).cast<int>() == 3); REQUIRE(m.attr("add")(1, 2).cast<int>() == 3);
} }
REQUIRE(has_pybind11_internals()); REQUIRE(has_pybind11_internals_builtin());
REQUIRE(has_pybind11_internals_static());
/// Create and switch to a subinterpreter. /// Create and switch to a subinterpreter.
auto main_tstate = PyThreadState_Get(); auto main_tstate = PyThreadState_Get();
@ -134,7 +158,8 @@ TEST_CASE("Subinterpreter") {
// Subinterpreters get their own copy of builtins. detail::get_internals() still // 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 // works by returning from the static variable, i.e. all interpreters share a single
// global pybind11::internals; // global pybind11::internals;
REQUIRE_FALSE(has_pybind11_internals()); REQUIRE_FALSE(has_pybind11_internals_builtin());
REQUIRE(has_pybind11_internals_static());
// Modules tags should be gone. // Modules tags should be gone.
REQUIRE_FALSE(py::hasattr(py::module::import("__main__"), "tag")); REQUIRE_FALSE(py::hasattr(py::module::import("__main__"), "tag"));