Fix how thread states are created (#1276)

include/pybind11/detail/internals.h

@@ -79,7 +79,6 @@ struct internals {
     PyTypeObject *default_metaclass;
     PyObject *instance_base;
 #if defined(WITH_THREAD)
-    decltype(PyThread_create_key()) tstate = 0; // Usually an int but a long on Cygwin64 with Python 3.x
     PyInterpreterState *istate = nullptr;
 #endif
 };
@@ -166,8 +165,6 @@ PYBIND11_NOINLINE inline internals &get_internals() {
 #if defined(WITH_THREAD)
         PyEval_InitThreads();
         PyThreadState *tstate = PyThreadState_Get();
-        internals_ptr->tstate = PyThread_create_key();
-        PyThread_set_key_value(internals_ptr->tstate, tstate);
         internals_ptr->istate = tstate->interp;
 #endif
         builtins[id] = capsule(internals_pp);

include/pybind11/pybind11.h

@@ -1755,7 +1755,7 @@ class gil_scoped_acquire {
 public:
     PYBIND11_NOINLINE gil_scoped_acquire() {
         auto const &internals = detail::get_internals();
-        tstate = (PyThreadState *) PyThread_get_key_value(internals.tstate);
+        tstate = PyGILState_GetThisThreadState();
 
         if (!tstate) {
             tstate = PyThreadState_New(internals.istate);
@@ -1764,10 +1764,6 @@ class gil_scoped_acquire {
                     pybind11_fail("scoped_acquire: could not create thread state!");
             #endif
             tstate->gilstate_counter = 0;
-            #if PY_MAJOR_VERSION < 3
-                PyThread_delete_key_value(internals.tstate);
-            #endif
-            PyThread_set_key_value(internals.tstate, tstate);
         } else {
             release = detail::get_thread_state_unchecked() != tstate;
         }
@@ -1806,7 +1802,6 @@ class gil_scoped_acquire {
             #endif
             PyThreadState_Clear(tstate);
             PyThreadState_DeleteCurrent();
-            PyThread_delete_key_value(detail::get_internals().tstate);
             release = false;
         }
     }
@@ -1825,30 +1820,15 @@ class gil_scoped_release {
 public:
     explicit gil_scoped_release(bool disassoc = false) : disassoc(disassoc) {
         // `get_internals()` must be called here unconditionally in order to initialize
-        // `internals.tstate` for subsequent `gil_scoped_acquire` calls. Otherwise, an
+        // `internals.istate` for subsequent `gil_scoped_acquire` calls. Otherwise, an
         // initialization race could occur as multiple threads try `gil_scoped_acquire`.
-        const auto &internals = detail::get_internals();
+        detail::get_internals();
         tstate = PyEval_SaveThread();
-        if (disassoc) {
-            auto key = internals.tstate;
-            #if PY_MAJOR_VERSION < 3
-                PyThread_delete_key_value(key);
-            #else
-                PyThread_set_key_value(key, nullptr);
-            #endif
-        }
     }
     ~gil_scoped_release() {
         if (!tstate)
             return;
         PyEval_RestoreThread(tstate);
-        if (disassoc) {
-            auto key = detail::get_internals().tstate;
-            #if PY_MAJOR_VERSION < 3
-                PyThread_delete_key_value(key);
-            #endif
-            PyThread_set_key_value(key, tstate);
-        }
     }
 private:
     PyThreadState *tstate;

tests/CMakeLists.txt

@@ -57,6 +57,7 @@ set(PYBIND11_TEST_FILES
   test_smart_ptr.cpp
   test_stl.cpp
   test_stl_binders.cpp
+  test_threads.cpp
   test_virtual_functions.cpp
 )
 

tests/conftest.py

@@ -199,6 +199,10 @@ def pytest_namespace():
         from pybind11_tests.eigen import have_eigen
     except ImportError:
         have_eigen = False
+    try:
+        import threading
+    except ImportError:
+        threading = None
     pypy = platform.python_implementation() == "PyPy"
 
     skipif = pytest.mark.skipif
@@ -210,6 +214,7 @@ def pytest_namespace():
                                            reason="eigen and/or numpy are not installed"),
         'requires_eigen_and_scipy': skipif(not have_eigen or not scipy,
                                            reason="eigen and/or scipy are not installed"),
+        'requires_threading': skipif(not threading, reason="no threading"),
         'unsupported_on_pypy': skipif(pypy, reason="unsupported on PyPy"),
         'unsupported_on_py2': skipif(sys.version_info.major < 3,
                                      reason="unsupported on Python 2.x"),

tests/test_threads.cpp

@@ -0,0 +1,27 @@
+#include "pybind11_tests.h"
+
+#if defined(WITH_THREAD)
+
+#include <thread>
+
+static bool check_threadstate() {
+    return PyGILState_GetThisThreadState() == PyThreadState_Get();
+}
+
+TEST_SUBMODULE(threads, m) {
+    m.def("check_pythread", []() -> bool {
+        py::gil_scoped_acquire acquire1;
+        return check_threadstate();
+    }, py::call_guard<py::gil_scoped_release>())
+    .def("check_cthread", []() -> bool {
+        bool result = false;
+        std::thread thread([&result]() {
+            py::gil_scoped_acquire acquire;
+            result = check_threadstate();
+        });
+        thread.join();
+        return result;
+    }, py::call_guard<py::gil_scoped_release>());
+}
+
+#endif

tests/test_threads.py

@@ -0,0 +1,19 @@
+import pytest
+
+pytestmark = pytest.requires_threading
+
+with pytest.suppress(ImportError):
+    import threading
+    from pybind11_tests import threads as t
+
+
+def test_threads():
+    def pythread_routine():
+        threading.current_thread()._return = t.check_pythread()
+
+    thread = threading.Thread(target=pythread_routine)
+    thread.start()
+    thread.join()
+    assert thread._return
+
+    assert t.check_cthread()