From 44e39e0de7ad011de9862a66666632401d66cdf3 Mon Sep 17 00:00:00 2001 From: Wenzel Jakob Date: Tue, 11 Sep 2018 09:32:45 +0200 Subject: [PATCH] fix regression reported by @cyfdecyf in #1454 (#1517) --- include/pybind11/detail/internals.h | 32 ++++++++++++++++++----------- tests/test_virtual_functions.cpp | 28 ++++++++++++++++++++++++- tests/test_virtual_functions.py | 6 ++++++ 3 files changed, 53 insertions(+), 13 deletions(-) diff --git a/include/pybind11/detail/internals.h b/include/pybind11/detail/internals.h index e6f851abb..78d4afed0 100644 --- a/include/pybind11/detail/internals.h +++ b/include/pybind11/detail/internals.h @@ -21,20 +21,28 @@ inline PyObject *make_object_base_type(PyTypeObject *metaclass); // The old Python Thread Local Storage (TLS) API is deprecated in Python 3.7 in favor of the new // Thread Specific Storage (TSS) API. #if PY_VERSION_HEX >= 0x03070000 - #define PYBIND11_TLS_KEY_INIT(var) Py_tss_t *var = nullptr - #define PYBIND11_TLS_GET_VALUE(key) PyThread_tss_get((key)) - #define PYBIND11_TLS_REPLACE_VALUE(key, value) PyThread_tss_set((key), (tstate)) - #define PYBIND11_TLS_DELETE_VALUE(key) PyThread_tss_set((key), nullptr) +# define PYBIND11_TLS_KEY_INIT(var) Py_tss_t *var = nullptr +# define PYBIND11_TLS_GET_VALUE(key) PyThread_tss_get((key)) +# define PYBIND11_TLS_REPLACE_VALUE(key, value) PyThread_tss_set((key), (tstate)) +# define PYBIND11_TLS_DELETE_VALUE(key) PyThread_tss_set((key), nullptr) #else // Usually an int but a long on Cygwin64 with Python 3.x - #define PYBIND11_TLS_KEY_INIT(var) decltype(PyThread_create_key()) var = 0 - #define PYBIND11_TLS_GET_VALUE(key) PyThread_get_key_value((key)) - #if PY_MAJOR_VERSION < 3 - #define PYBIND11_TLS_REPLACE_VALUE(key, value) do { PyThread_delete_key_value((key)); PyThread_set_key_value((key), (value)); } while (false) - #else - #define PYBIND11_TLS_REPLACE_VALUE(key, value) PyThread_set_key_value((key), (value)) - #endif - #define PYBIND11_TLS_DELETE_VALUE(key) PyThread_set_key_value((key), nullptr) +# define PYBIND11_TLS_KEY_INIT(var) decltype(PyThread_create_key()) var = 0 +# define PYBIND11_TLS_GET_VALUE(key) PyThread_get_key_value((key)) +# if PY_MAJOR_VERSION < 3 +# define PYBIND11_TLS_DELETE_VALUE(key) \ + PyThread_delete_key_value(key) +# define PYBIND11_TLS_REPLACE_VALUE(key, value) \ + do { \ + PyThread_delete_key_value((key)); \ + PyThread_set_key_value((key), (value)); \ + } while (false) +# else +# define PYBIND11_TLS_DELETE_VALUE(key) \ + PyThread_set_key_value((key), nullptr) +# define PYBIND11_TLS_REPLACE_VALUE(key, value) \ + PyThread_set_key_value((key), (value)) +# endif #endif // Python loads modules by default with dlopen with the RTLD_LOCAL flag; under libc++ and possibly diff --git a/tests/test_virtual_functions.cpp b/tests/test_virtual_functions.cpp index 6ffdf33af..c9a561c09 100644 --- a/tests/test_virtual_functions.cpp +++ b/tests/test_virtual_functions.cpp @@ -10,6 +10,7 @@ #include "pybind11_tests.h" #include "constructor_stats.h" #include +#include /* This is an example class that we'll want to be able to extend from Python */ class ExampleVirt { @@ -157,6 +158,28 @@ struct DispatchIssue : Base { } }; +static void test_gil() { + { + py::gil_scoped_acquire lock; + py::print("1st lock acquired"); + + } + + { + py::gil_scoped_acquire lock; + py::print("2nd lock acquired"); + } + +} + +static void test_gil_from_thread() { + py::gil_scoped_release release; + + std::thread t(test_gil); + t.join(); +} + + // Forward declaration (so that we can put the main tests here; the inherited virtual approaches are // rather long). void initialize_inherited_virtuals(py::module &m); @@ -416,7 +439,6 @@ public: }; */ - void initialize_inherited_virtuals(py::module &m) { // test_inherited_virtuals @@ -449,4 +471,8 @@ void initialize_inherited_virtuals(py::module &m) { py::class_>(m, "D_Tpl") .def(py::init<>()); + + // Fix issue #1454 (crash when acquiring/releasing GIL on another thread in Python 2.7) + m.def("test_gil", &test_gil); + m.def("test_gil_from_thread", &test_gil_from_thread); }; diff --git a/tests/test_virtual_functions.py b/tests/test_virtual_functions.py index 2a92476f9..5ce9abd35 100644 --- a/tests/test_virtual_functions.py +++ b/tests/test_virtual_functions.py @@ -369,3 +369,9 @@ def test_inherited_virtuals(): assert obj.unlucky_number() == -7 assert obj.lucky_number() == -1.375 assert obj.say_everything() == "BT -7" + + +def test_issue_1454(): + # Fix issue #1454 (crash when acquiring/releasing GIL on another thread in Python 2.7) + m.test_gil() + m.test_gil_from_thread()