diff --git a/graalpython/com.oracle.graal.python.test/src/tests/cpyext/test_thread.py b/graalpython/com.oracle.graal.python.test/src/tests/cpyext/test_thread.py index f64b179663..4752645d7d 100644 --- a/graalpython/com.oracle.graal.python.test/src/tests/cpyext/test_thread.py +++ b/graalpython/com.oracle.graal.python.test/src/tests/cpyext/test_thread.py @@ -98,6 +98,48 @@ class TestPyThread(CPyExtTestCase): @unittest.skipIf(sys.platform == 'win32', "Needs pthread") class TestNativeThread(unittest.TestCase): + def test_save_thread_in_native_thread(self): + NativeThread = CPyExtType( + name="SaveThreadInNativeThread", + includes="#include ", + code=r''' + static void* worker(void* arg) { + for (int i = 0; i < 3; i++) { + PyGILState_STATE state = PyGILState_Ensure(); + /* Do not call PyThreadState_Get first: it can initialize + the native thread state and hide the failure. */ + PyThreadState *saved = PyEval_SaveThread(); + PyEval_RestoreThread(saved); + PyGILState_STATE nested = PyGILState_Ensure(); + PyGILState_Release(nested); + if (PyThreadState_Get() != saved) { + Py_FatalError("Nested GIL state acquisition changed the thread state"); + } + PyGILState_Release(state); + } + return NULL; + } + + static PyObject* run(PyObject* self, PyObject* ignored) { + pthread_t thread; + int error; + Py_BEGIN_ALLOW_THREADS + error = pthread_create(&thread, NULL, worker, NULL); + if (!error) { + error = pthread_join(thread, NULL); + } + Py_END_ALLOW_THREADS + if (error) { + PyErr_SetString(PyExc_RuntimeError, "pthread_create/join failed"); + return NULL; + } + Py_RETURN_NONE; + } + ''', + tp_methods='{"run", (PyCFunction)run, METH_NOARGS | METH_STATIC, ""}' + ) + NativeThread.run() + def test_register_new_thread(self): TestThread = CPyExtType( name="TestThread", diff --git a/graalpython/com.oracle.graal.python/src/com/oracle/graal/python/builtins/objects/cext/capi/CApiContext.java b/graalpython/com.oracle.graal.python/src/com/oracle/graal/python/builtins/objects/cext/capi/CApiContext.java index b1b56d238c..922b244680 100644 --- a/graalpython/com.oracle.graal.python/src/com/oracle/graal/python/builtins/objects/cext/capi/CApiContext.java +++ b/graalpython/com.oracle.graal.python/src/com/oracle/graal/python/builtins/objects/cext/capi/CApiContext.java @@ -929,14 +929,25 @@ public void dispose() { */ private int attachNativeThread() { CompilerAsserts.neverPartOfCompilation(); - TruffleContext truffleContext = getContext().getEnv().getContext(); + PythonContext context = getContext(); + TruffleContext truffleContext = context.getEnv().getContext(); if (truffleContext.isActive()) { return GRAALPY_ATTACH_NATIVE_FOREIGN; } try { Object previousContext = truffleContext.enter(null); - attachedThreadPreviousContext.set(previousContext); - return GRAALPY_ATTACH_NATIVE_OWNED; + try { + // Re-entering a still-alive thread reuses its ContextThreadLocal without calling + // PythonLanguage.initializeThread again. Restore state disposed by the last detach. + if (context.getThreadState(context.getLanguage()).isUnused()) { + context.attachThread(Thread.currentThread(), context.getLanguage().getThreadStateLocal()); + } + attachedThreadPreviousContext.set(previousContext); + return GRAALPY_ATTACH_NATIVE_OWNED; + } catch (Throwable t) { + truffleContext.leave(null, previousContext); + throw t; + } } catch (Throwable t) { LOGGER.severe("could not attach native thread to polyglot context: " + t.getMessage()); return GRAALPY_ATTACH_NATIVE_FAILED; diff --git a/graalpython/com.oracle.graal.python/src/com/oracle/graal/python/runtime/PythonContext.java b/graalpython/com.oracle.graal.python/src/com/oracle/graal/python/runtime/PythonContext.java index a1da746666..6df48c68be 100644 --- a/graalpython/com.oracle.graal.python/src/com/oracle/graal/python/runtime/PythonContext.java +++ b/graalpython/com.oracle.graal.python/src/com/oracle/graal/python/runtime/PythonContext.java @@ -360,6 +360,9 @@ public int[] getThresholds() { public static final class PythonThreadState { private volatile boolean shuttingDown = false; + /* A temporary native detach leaves this object cached in Truffle's ContextThreadLocal. */ + private boolean unused; + /* * The reference to the last top frame on the Python stack during interop calls. Initially, * this is EMPTY representing the top frame. @@ -450,6 +453,10 @@ public boolean isShuttingDown() { return shuttingDown; } + public boolean isUnused() { + return unused; + } + @TruffleBoundary boolean reprEnter(Object item) { if (reprObjectSet == null) { @@ -568,6 +575,9 @@ public void dispose(boolean clearNativeThreadLocalVarPointer, boolean markShutti NativeMemory.writePtr(nativeThreadLocalVarPointer, NULLPTR); } nativeThreadLocalVarPointer = NULLPTR; + if (!markShuttingDown) { + unused = true; + } } public Object getTraceFun() { @@ -2459,6 +2469,7 @@ public void attachThread(Thread thread, ContextThreadLocal th // initialize this thread's native TLS slot eagerly instead of on first use initializeNativeThreadState(pythonThreadState); } + pythonThreadState.unused = false; } catch (PException e) { synchronized (this) { if (previousThreadState == null) { @@ -2505,7 +2516,8 @@ public void disposeThread(Thread thread, boolean canRunGuestCode) { * for a still-alive thread, so a native thread that calls {@code PyGILState_Ensure} again after * {@code PyGILState_Release} may receive the same Java {@link PythonThreadState} object. Such a * detach must remove the thread from {@link #threadStateMapping} and free the native - * {@code PyThreadState}, but it must not mark the Java thread state as shutting down. + * {@code PyThreadState}, but it must not mark the Java thread state as shutting down. Instead, + * it marks the Java state as unused so that the next native attach can initialize it again. */ public void disposeThread(Thread thread, boolean canRunGuestCode, boolean markShuttingDown) { CompilerAsserts.neverPartOfCompilation();