Binder: refuse withGIL on JVM virtual threads (#161)

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`.

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() {}

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 <T> withGIL(block: () -> T): T {
     val tState = getThreadGILState()
     val isOutermost = tState.depth == 0
     if (isOutermost) {
+        checkGILThread()
         tState.state = PyGILState_Ensure()
     }
     tState.depth++

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."
+        )
+    }
+}

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<Throwable?>(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<Int?>(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<Int?>(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 } })
+    }
+}

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() {}

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() {}