Fix using PyGILState_Check in unattached thread

Author: msimacek

graalpython/com.oracle.graal.python.cext/include/pystate.h

@@ -1,4 +1,4 @@
-/* Copyright (c) 2018, 2023, Oracle and/or its affiliates.
+/* Copyright (c) 2018, 2024, Oracle and/or its affiliates.
  * Copyright (C) 1996-2020 Python Software Foundation
  *
  * Licensed under the PYTHON SOFTWARE FOUNDATION LICENSE VERSION 2
@@ -39,6 +39,7 @@ PyAPI_FUNC(PyObject *) PyInterpreterState_GetDict(PyInterpreterState *);
 #if !defined(Py_LIMITED_API) || Py_LIMITED_API+0 >= 0x03070000
 /* New in 3.7 */
 PyAPI_FUNC(int64_t) PyInterpreterState_GetID(PyInterpreterState *);
+// GraalPy-specific
 PyAPI_FUNC(int64_t) PyInterpreterState_GetIDFromThreadState(PyThreadState *);
 #endif
 #if !defined(Py_LIMITED_API) || Py_LIMITED_API+0 >= 0x03030000
@@ -79,8 +80,9 @@ PyAPI_FUNC(PyFrameObject*) PyThreadState_GetFrame(PyThreadState *tstate);
 PyAPI_FUNC(uint64_t) PyThreadState_GetID(PyThreadState *tstate);
 #endif
 
-/* GraalVM change: we need more state bits */
-typedef int PyGILState_STATE;
+typedef
+    enum {PyGILState_LOCKED, PyGILState_UNLOCKED}
+        PyGILState_STATE;
 
 
 /* Ensure that the current thread is ready to call the Python

graalpython/com.oracle.graal.python.cext/src/pystate.c

@@ -137,29 +137,56 @@ int PyState_RemoveModule(struct PyModuleDef* def) {
     return 0;
 }
 
-#define _PYGILSTATE_LOCKED   0x1
-#define _PYGILSTATE_ATTACHED 0x2
-
-PyAPI_FUNC(PyGILState_STATE) PyGILState_Ensure() {
-    int result = 0;
+int
+PyGILState_Check()
+{
+    int attached = 0;
+    /*
+     * PyGILState_Check is allowed to be called from a new thread that didn't yet setup the GIL.
+     * If we don't attach the thread ourselves, the upcall will work because NFI will attach
+     * the thread automatically, but it won't create the context which would then break
+     * subsequent PyGILState_Ensure.
+     */
     if (TRUFFLE_CONTEXT) {
         if ((*TRUFFLE_CONTEXT)->getTruffleEnv(TRUFFLE_CONTEXT) == NULL) {
             (*TRUFFLE_CONTEXT)->attachCurrentThread(TRUFFLE_CONTEXT);
-            result |= _PYGILSTATE_ATTACHED;
+            attached = 1;
         }
     }
-    int locked = GraalPyTruffleGILState_Ensure();
-    if (locked) {
-        result |= _PYGILSTATE_LOCKED;
+    int ret = GraalPyTruffleGILState_Check();
+    if (attached) {
+        (*TRUFFLE_CONTEXT)->detachCurrentThread(TRUFFLE_CONTEXT);
+    }
+    return ret;
+}
+
+static THREAD_LOCAL int graalpy_attached_thread = 0;
+static THREAD_LOCAL int graalpy_gilstate_counter = 0;
+
+PyGILState_STATE
+PyGILState_Ensure(void)
+{
+    if (TRUFFLE_CONTEXT) {
+        if ((*TRUFFLE_CONTEXT)->getTruffleEnv(TRUFFLE_CONTEXT) == NULL) {
+            (*TRUFFLE_CONTEXT)->attachCurrentThread(TRUFFLE_CONTEXT);
+            graalpy_attached_thread = 1;
+        }
+        graalpy_gilstate_counter++;
     }
-    return result;
+    return GraalPyTruffleGILState_Ensure() ? PyGILState_UNLOCKED : PyGILState_LOCKED;
 }
 
-PyAPI_FUNC(void) PyGILState_Release(PyGILState_STATE state) {
-    if (state & _PYGILSTATE_LOCKED) {
+void
+PyGILState_Release(PyGILState_STATE oldstate)
+{
+    if (oldstate == PyGILState_UNLOCKED) {
         GraalPyTruffleGILState_Release();
     }
-    if (TRUFFLE_CONTEXT && (state & _PYGILSTATE_ATTACHED)) {
-        (*TRUFFLE_CONTEXT)->detachCurrentThread(TRUFFLE_CONTEXT);
+    if (TRUFFLE_CONTEXT) {
+        graalpy_gilstate_counter--;
+        if (graalpy_gilstate_counter == 0 && graalpy_attached_thread) {
+            (*TRUFFLE_CONTEXT)->detachCurrentThread(TRUFFLE_CONTEXT);
+            graalpy_attached_thread = 0;
+        }
     }
 }

graalpython/com.oracle.graal.python/src/com/oracle/graal/python/builtins/modules/cext/PythonCextPyStateBuiltins.java

@@ -71,7 +71,7 @@
 public final class PythonCextPyStateBuiltins {
 
     @CApiBuiltin(ret = Int, args = {}, acquiresGIL = false, call = Direct)
-    abstract static class PyGILState_Check extends CApiNullaryBuiltinNode {
+    abstract static class PyTruffleGILState_Check extends CApiNullaryBuiltinNode {
 
         @Specialization
         Object check() {

graalpython/com.oracle.graal.python/src/com/oracle/graal/python/builtins/objects/cext/capi/CApiFunction.java

@@ -186,6 +186,7 @@ public final class CApiFunction {
     /*
      * Functions that are implemented as C code that can be executed both in native and in Sulong:
      */
+    @CApiBuiltin(name = "PyGILState_Check", ret = Int, args = {}, acquiresGIL = false, call = CImpl)
     @CApiBuiltin(name = "PyArg_Parse", ret = Int, args = {PyObject, ConstCharPtrAsTruffleString, VARARGS}, call = CImpl)
     @CApiBuiltin(name = "PyArg_ParseTuple", ret = Int, args = {PyObject, ConstCharPtrAsTruffleString, VARARGS}, call = CImpl)
     @CApiBuiltin(name = "PyArg_ParseTupleAndKeywords", ret = Int, args = {PyObject, PyObject, ConstCharPtrAsTruffleString, CHAR_PTR_LIST, VARARGS}, call = CImpl)