fix regression reported by @cyfdecyf in #1454 (#1517)

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
 // The old Python Thread Local Storage (TLS) API is deprecated in Python 3.7 in favor of the new
 // Thread Specific Storage (TSS) API.
 // Thread Specific Storage (TSS) API.
 #if PY_VERSION_HEX >= 0x03070000
 #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
 #else
     // Usually an int but a long on Cygwin64 with Python 3.x
     // 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
 #endif
 
 
 // Python loads modules by default with dlopen with the RTLD_LOCAL flag; under libc++ and possibly
 // Python loads modules by default with dlopen with the RTLD_LOCAL flag; under libc++ and possibly

tests/test_virtual_functions.cpp

@@ -10,6 +10,7 @@
 #include "pybind11_tests.h"
 #include "pybind11_tests.h"
 #include "constructor_stats.h"
 #include "constructor_stats.h"
 #include <pybind11/functional.h>
 #include <pybind11/functional.h>
+#include <thread>
 
 
 /* This is an example class that we'll want to be able to extend from Python */
 /* This is an example class that we'll want to be able to extend from Python */
 class ExampleVirt  {
 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
 // Forward declaration (so that we can put the main tests here; the inherited virtual approaches are
 // rather long).
 // rather long).
 void initialize_inherited_virtuals(py::module &m);
 void initialize_inherited_virtuals(py::module &m);
@@ -416,7 +439,6 @@ public:
 };
 };
 */
 */
 
 
-
 void initialize_inherited_virtuals(py::module &m) {
 void initialize_inherited_virtuals(py::module &m) {
     // test_inherited_virtuals
     // test_inherited_virtuals
 
 
@@ -449,4 +471,8 @@ void initialize_inherited_virtuals(py::module &m) {
     py::class_<D_Tpl, C_Tpl, PyB_Tpl<D_Tpl>>(m, "D_Tpl")
     py::class_<D_Tpl, C_Tpl, PyB_Tpl<D_Tpl>>(m, "D_Tpl")
         .def(py::init<>());
         .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);
 };
 };

tests/test_virtual_functions.py

@@ -369,3 +369,9 @@ def test_inherited_virtuals():
     assert obj.unlucky_number() == -7
     assert obj.unlucky_number() == -7
     assert obj.lucky_number() == -1.375
     assert obj.lucky_number() == -1.375
     assert obj.say_everything() == "BT -7"
     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()