From 4da59edf8ebd560f99fbf32e04a65f45e92e7773 Mon Sep 17 00:00:00 2001 From: BREW <81044362+b-re-w@users.noreply.github.com> Date: Sun, 25 Oct 2026 19:03:10 +0900 Subject: [PATCH] Fix: the outermost withGIL on a JVM virtual thread raises IllegalStateException (#161) PyGILState binds the thread state to the OS carrier while withGIL's depth lives in the virtual thread's ThreadLocal; a virtual thread parking inside the scope could resume on another carrier and release the GIL there. Desktop refuses the entry (option A); Android, native and wasm are no-ops. Pinning instead (option B) changes the public withGIL signature and awaits a decision. --- docs/SPEC.md | 6 +++ .../ffi/GILThreadCheck.android.kt | 5 ++ .../python/multiplatform/ffi/GILScope.kt | 9 ++++ .../ffi/GILThreadCheck.desktop.kt | 31 +++++++++++ .../multiplatform/ffi/VirtualThreadGILTest.kt | 54 +++++++++++++++++++ .../ffi/GILThreadCheck.native.kt | 5 ++ .../ffi/GILThreadCheck.wasmJs.kt | 5 ++ 7 files changed, 115 insertions(+) create mode 100644 python-multiplatform/src/androidMain/kotlin/python/multiplatform/ffi/GILThreadCheck.android.kt create mode 100644 python-multiplatform/src/desktopMain/kotlin/python/multiplatform/ffi/GILThreadCheck.desktop.kt create mode 100644 python-multiplatform/src/desktopTest/kotlin/python/multiplatform/ffi/VirtualThreadGILTest.kt create mode 100644 python-multiplatform/src/nativeMain/kotlin/python/multiplatform/ffi/GILThreadCheck.native.kt create mode 100644 python-multiplatform/src/wasmJsMain/kotlin/python/multiplatform/ffi/GILThreadCheck.wasmJs.kt diff --git a/docs/SPEC.md b/docs/SPEC.md index f86497e78..da5b21446 100644 --- a/docs/SPEC.md +++ b/docs/SPEC.md @@ -587,6 +587,12 @@ All in `PM/commonTest`, so they run wherever the interpreter loads. - **T-1** GIL builds: Kotlin threads attach a thread state and serialise on the GIL. `Status: partial` (see C-4). +- **T-1a** On desktop, entering Python from a JVM virtual thread (the outermost `withGIL`) raises + `IllegalStateException` before `PyGILState_Ensure`: the GIL thread state is bound to the OS carrier + thread, and a virtual thread can unmount and resume on another carrier inside the scope. Run Python on + platform threads. Android, native and wasm are unaffected. Pinning the virtual thread instead (issue #161 + option B) needs a public API change and is pending a decision. `Status: implemented`, + `PM/desktopTest/.../VirtualThreadGILTest.kt`. - **T-2** Free-threaded builds selected with `-PpythonFreeThreaded=true` download a different CPython asset. `Status: partial` (desktop only, opt-in; default is `pythonFreeThreaded=false` in `gradle.properties`) , `GP/PythonHomeStagingTest.kt`, `PM/desktopTest/.../FreeThreadedGCGateTest.kt`, `VersionsTest.kt`. diff --git a/python-multiplatform/src/androidMain/kotlin/python/multiplatform/ffi/GILThreadCheck.android.kt b/python-multiplatform/src/androidMain/kotlin/python/multiplatform/ffi/GILThreadCheck.android.kt new file mode 100644 index 000000000..71490d22d --- /dev/null +++ b/python-multiplatform/src/androidMain/kotlin/python/multiplatform/ffi/GILThreadCheck.android.kt @@ -0,0 +1,5 @@ +package python.multiplatform.ffi + +/** No virtual threads on this target; nothing to check (see the common declaration). */ +@PublishedApi +internal actual fun checkGILThread() {} diff --git a/python-multiplatform/src/commonMain/kotlin/python/multiplatform/ffi/GILScope.kt b/python-multiplatform/src/commonMain/kotlin/python/multiplatform/ffi/GILScope.kt index f58002b10..0ea134c50 100644 --- a/python-multiplatform/src/commonMain/kotlin/python/multiplatform/ffi/GILScope.kt +++ b/python-multiplatform/src/commonMain/kotlin/python/multiplatform/ffi/GILScope.kt @@ -50,6 +50,14 @@ internal fun maybeReachEvalCheckpoint(tState: ThreadGILState) { } } +/** + * Called by the outermost [withGIL] before `PyGILState_Ensure`. Desktop throws + * [IllegalStateException] on a JVM virtual thread (issue #161); every other target does nothing. + * Not inline, so that [withGIL], which is, can call it. + */ +@PublishedApi +internal expect fun checkGILThread() + public expect fun getThreadGILState(): ThreadGILState public expect fun setThreadGILState(state: ThreadGILState?) @@ -75,6 +83,7 @@ public inline fun withGIL(block: () -> T): T { val tState = getThreadGILState() val isOutermost = tState.depth == 0 if (isOutermost) { + checkGILThread() tState.state = PyGILState_Ensure() } tState.depth++ diff --git a/python-multiplatform/src/desktopMain/kotlin/python/multiplatform/ffi/GILThreadCheck.desktop.kt b/python-multiplatform/src/desktopMain/kotlin/python/multiplatform/ffi/GILThreadCheck.desktop.kt new file mode 100644 index 000000000..0886fee58 --- /dev/null +++ b/python-multiplatform/src/desktopMain/kotlin/python/multiplatform/ffi/GILThreadCheck.desktop.kt @@ -0,0 +1,31 @@ +package python.multiplatform.ffi + +/** `Thread.isVirtual` exists from JDK 21. Computed once; see [checkGILThread]. */ +private val runtimeHasVirtualThreads: Boolean = Runtime.version().feature() >= 21 + +/** + * Refuses to attach a JVM virtual thread (issue #161). + * + * `PyGILState_Ensure` binds the CPython thread state to the OS carrier thread, while the nesting + * depth of [withGIL] lives in a `java.lang.ThreadLocal`, which belongs to the virtual thread. A + * virtual thread that parks inside the scope unmounts, another virtual thread then runs on the same + * carrier and sees depth 0 (so it calls Ensure again), and the first one may resume and Release on a + * different carrier. None of that can be made correct without pinning, so the outermost scope fails + * before Ensure instead. + * + * `Thread.isVirtual` is a JDK 21 API while the module targets JVM 17. The JVM links a call site only + * when it is first executed, so the version flag is tested first and a JDK 17 to 20 runtime never + * reaches the call. It is a direct call on purpose: no lookup by name, no reflection. + */ +@PublishedApi +internal actual fun checkGILThread() { + if (runtimeHasVirtualThreads && Thread.currentThread().isVirtual) { + throw IllegalStateException( + "withGIL cannot be entered from a virtual thread: the CPython GIL thread state is bound to " + + "the OS carrier thread, and a virtual thread can unmount and resume on another carrier " + + "inside the scope. Run Python from a platform thread (a fixed thread pool, or " + + "Dispatchers.IO-backed platform threads), not Executors.newVirtualThreadPerTaskExecutor " + + "or Thread.ofVirtual." + ) + } +} diff --git a/python-multiplatform/src/desktopTest/kotlin/python/multiplatform/ffi/VirtualThreadGILTest.kt b/python-multiplatform/src/desktopTest/kotlin/python/multiplatform/ffi/VirtualThreadGILTest.kt new file mode 100644 index 000000000..7e4982a04 --- /dev/null +++ b/python-multiplatform/src/desktopTest/kotlin/python/multiplatform/ffi/VirtualThreadGILTest.kt @@ -0,0 +1,54 @@ +package python.multiplatform.ffi + +import java.util.concurrent.atomic.AtomicReference +import kotlin.test.Test +import kotlin.test.assertEquals +import kotlin.test.assertNotNull +import kotlin.test.assertTrue + +/** + * Issue #161: the outermost [withGIL] on a JVM virtual thread throws before PyGILState_Ensure. + * Red before the change: withGIL simply ran on the virtual thread. + */ +class VirtualThreadGILTest { + + private fun virtualThreadsAvailable() = Runtime.version().feature() >= 21 + + @Test + fun virtualThreadIsRefusedAndGilIsNotLeaked() = PythonTestFixture.withInterpreter { + if (!virtualThreadsAvailable()) return@withInterpreter + val entered = java.util.concurrent.atomic.AtomicBoolean(false) + val failure = AtomicReference(null) + val t = Thread.ofVirtual().start { + try { + withGIL { entered.set(true) } + } catch (e: Throwable) { + failure.set(e) + } + } + t.join(30_000) + val e = failure.get() + assertNotNull(e, "withGIL on a virtual thread must throw") + assertTrue(e is IllegalStateException, "got $e") + assertTrue(e.message!!.contains("virtual thread"), e.message) + assertTrue(e.message!!.contains("platform thread"), e.message) + assertTrue(!entered.get(), "block must not run") + + // Ensure was never called, so the GIL is free: a platform thread completes promptly. + val result = AtomicReference(null) + val p = Thread { result.set(withGIL { 42 }) } + p.start() + p.join(30_000) + assertEquals(42, result.get(), "platform thread withGIL must complete (GIL not leaked)") + } + + @Test + fun platformThreadAndNestedScopesStillWork() = PythonTestFixture.withInterpreter { + val result = AtomicReference(null) + val p = Thread { result.set(withGIL { withGIL { 1 } + withGIL { 2 } }) } + p.start() + p.join(30_000) + assertEquals(3, result.get()) + assertEquals(7, withGIL { withGIL { 7 } }) + } +} diff --git a/python-multiplatform/src/nativeMain/kotlin/python/multiplatform/ffi/GILThreadCheck.native.kt b/python-multiplatform/src/nativeMain/kotlin/python/multiplatform/ffi/GILThreadCheck.native.kt new file mode 100644 index 000000000..71490d22d --- /dev/null +++ b/python-multiplatform/src/nativeMain/kotlin/python/multiplatform/ffi/GILThreadCheck.native.kt @@ -0,0 +1,5 @@ +package python.multiplatform.ffi + +/** No virtual threads on this target; nothing to check (see the common declaration). */ +@PublishedApi +internal actual fun checkGILThread() {} diff --git a/python-multiplatform/src/wasmJsMain/kotlin/python/multiplatform/ffi/GILThreadCheck.wasmJs.kt b/python-multiplatform/src/wasmJsMain/kotlin/python/multiplatform/ffi/GILThreadCheck.wasmJs.kt new file mode 100644 index 000000000..71490d22d --- /dev/null +++ b/python-multiplatform/src/wasmJsMain/kotlin/python/multiplatform/ffi/GILThreadCheck.wasmJs.kt @@ -0,0 +1,5 @@ +package python.multiplatform.ffi + +/** No virtual threads on this target; nothing to check (see the common declaration). */ +@PublishedApi +internal actual fun checkGILThread() {}