Make sure `detail::get_internals` acquires the GIL before making Python calls. (#1836)
This is only necessary if `get_internals` is called for the first time in a given module when the running thread is in a GIL-released state.
Fixes #1364
diff --git a/include/pybind11/detail/internals.h b/include/pybind11/detail/internals.h
index f1dd387..952f801 100644
--- a/include/pybind11/detail/internals.h
+++ b/include/pybind11/detail/internals.h
@@ -171,6 +171,14 @@
if (internals_pp && *internals_pp)
return **internals_pp;
+ // Ensure that the GIL is held since we will need to make Python calls.
+ // Cannot use py::gil_scoped_acquire here since that constructor calls get_internals.
+ struct gil_scoped_acquire {
+ gil_scoped_acquire() : state (PyGILState_Ensure()) {}
+ ~gil_scoped_acquire() { PyGILState_Release(state); }
+ const PyGILState_STATE state;
+ } gil;
+
constexpr auto *id = PYBIND11_INTERNALS_ID;
auto builtins = handle(PyEval_GetBuiltins());
if (builtins.contains(id) && isinstance<capsule>(builtins[id])) {
diff --git a/tests/CMakeLists.txt b/tests/CMakeLists.txt
index 9a70110..fb6776f 100644
--- a/tests/CMakeLists.txt
+++ b/tests/CMakeLists.txt
@@ -83,6 +83,10 @@
test_stl_binders.py
)
+set(PYBIND11_CROSS_MODULE_GIL_TESTS
+ test_gil_scoped.py
+)
+
# Check if Eigen is available; if not, remove from PYBIND11_TEST_FILES (but
# keep it in PYBIND11_PYTEST_FILES, so that we get the "eigen is not installed"
# skip message).
@@ -150,6 +154,14 @@
endif()
endforeach()
+foreach(t ${PYBIND11_CROSS_MODULE_GIL_TESTS})
+ list(FIND PYBIND11_PYTEST_FILES ${t} i)
+ if (i GREATER -1)
+ list(APPEND test_targets cross_module_gil_utils)
+ break()
+ endif()
+endforeach()
+
set(testdir ${CMAKE_CURRENT_SOURCE_DIR})
foreach(target ${test_targets})
set(test_files ${PYBIND11_TEST_FILES})
diff --git a/tests/cross_module_gil_utils.cpp b/tests/cross_module_gil_utils.cpp
new file mode 100644
index 0000000..07db9f6
--- /dev/null
+++ b/tests/cross_module_gil_utils.cpp
@@ -0,0 +1,73 @@
+/*
+ tests/cross_module_gil_utils.cpp -- tools for acquiring GIL from a different module
+
+ Copyright (c) 2019 Google LLC
+
+ All rights reserved. Use of this source code is governed by a
+ BSD-style license that can be found in the LICENSE file.
+*/
+#include <pybind11/pybind11.h>
+#include <cstdint>
+
+// This file mimics a DSO that makes pybind11 calls but does not define a
+// PYBIND11_MODULE. The purpose is to test that such a DSO can create a
+// py::gil_scoped_acquire when the running thread is in a GIL-released state.
+//
+// Note that we define a Python module here for convenience, but in general
+// this need not be the case. The typical scenario would be a DSO that implements
+// shared logic used internally by multiple pybind11 modules.
+
+namespace {
+
+namespace py = pybind11;
+void gil_acquire() { py::gil_scoped_acquire gil; }
+
+constexpr char kModuleName[] = "cross_module_gil_utils";
+
+#if PY_MAJOR_VERSION >= 3
+struct PyModuleDef moduledef = {
+ PyModuleDef_HEAD_INIT,
+ kModuleName,
+ NULL,
+ 0,
+ NULL,
+ NULL,
+ NULL,
+ NULL,
+ NULL
+};
+#else
+PyMethodDef module_methods[] = {
+ {NULL, NULL, 0, NULL}
+};
+#endif
+
+} // namespace
+
+extern "C" PYBIND11_EXPORT
+#if PY_MAJOR_VERSION >= 3
+PyObject* PyInit_cross_module_gil_utils()
+#else
+void initcross_module_gil_utils()
+#endif
+{
+
+ PyObject* m =
+#if PY_MAJOR_VERSION >= 3
+ PyModule_Create(&moduledef);
+#else
+ Py_InitModule(kModuleName, module_methods);
+#endif
+
+ if (m != NULL) {
+ static_assert(
+ sizeof(&gil_acquire) == sizeof(void*),
+ "Function pointer must have the same size as void*");
+ PyModule_AddObject(m, "gil_acquire_funcaddr",
+ PyLong_FromVoidPtr(reinterpret_cast<void*>(&gil_acquire)));
+ }
+
+#if PY_MAJOR_VERSION >= 3
+ return m;
+#endif
+}
diff --git a/tests/test_gil_scoped.cpp b/tests/test_gil_scoped.cpp
index cb0010e..76c17fd 100644
--- a/tests/test_gil_scoped.cpp
+++ b/tests/test_gil_scoped.cpp
@@ -41,4 +41,12 @@
[](VirtClass &virt) { virt.virtual_func(); });
m.def("test_callback_pure_virtual_func",
[](VirtClass &virt) { virt.pure_virtual_func(); });
+ m.def("test_cross_module_gil",
+ []() {
+ auto cm = py::module::import("cross_module_gil_utils");
+ auto gil_acquire = reinterpret_cast<void (*)()>(
+ PyLong_AsVoidPtr(cm.attr("gil_acquire_funcaddr").ptr()));
+ py::gil_scoped_release gil_release;
+ gil_acquire();
+ });
}
diff --git a/tests/test_gil_scoped.py b/tests/test_gil_scoped.py
index 2c72fc6..1548337 100644
--- a/tests/test_gil_scoped.py
+++ b/tests/test_gil_scoped.py
@@ -78,3 +78,8 @@
This test is for completion, but it was never an issue.
"""
assert _run_in_process(_python_to_cpp_to_python) == 0
+
+
+def test_cross_module_gil():
+ """Makes sure that the GIL can be acquired by another module from a GIL-released state."""
+ m.test_cross_module_gil() # Should not raise a SIGSEGV