Binder: pin virtual threads in withGIL; crossinline block (#161 option B)

docs/SPEC.md

@@ -644,12 +644,15 @@ 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; the release after initialisation is tested by `GilReleaseTest` / `GilReleaseNativeTest`).
-- **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-1a** On desktop, Python may be entered from a JVM virtual thread (#161 option B, user decision):
+  the outermost `withGIL` runs pinned to its OS carrier from `PyGILState_Ensure` to `Release`, so a
+  virtual thread that parks inside the scope (sleep, lock, blocking I/O) stays on the OS thread that
+  holds the thread state. The pin is a native frame (the block runs as the comparator of C's `bsearch`
+  over one element, an FFM upcall); its result or exception is passed through. `withGIL`'s and
+  `Python3.withPython`'s blocks are therefore `crossinline` (no non-local `return` from them).
+  Platform threads, Android, native and wasm run the block directly. A pinned virtual thread holds its
+  carrier while it waits inside the scope, as any pinned virtual thread does. `Status: implemented`,
+  `PM/desktopTest/.../VirtualThreadGILTest.kt` (without the pin the JVM aborts in `PyGILState_Release`).
 - **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/GILCarrierPin.android.kt

@@ -0,0 +1,8 @@
+package python.multiplatform.ffi
+
+/** No virtual threads on this target (#161): nothing to pin. */
+@PublishedApi
+internal actual fun needsCarrierPin(): Boolean = false
+
+@PublishedApi
+internal actual fun <R> runOnPinnedCarrier(body: () -> R): R = body()

python-multiplatform/src/commonMain/kotlin/python/multiplatform/ffi/GILScope.kt

@@ -51,12 +51,22 @@ 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.
+ * Whether this thread must be pinned to its OS carrier for the outermost [withGIL] (#161): true only
+ * for a JVM virtual thread on desktop. `PyGILState_Ensure` binds the CPython thread state to the OS
+ * thread, while [withGIL]'s depth lives in the (virtual) thread's own state; a virtual thread that
+ * parked inside the scope could resume on another carrier and Release there.
  */
 @PublishedApi
-internal expect fun checkGILThread()
+internal expect fun needsCarrierPin(): Boolean
+
+/**
+ * Runs [body] with the calling (virtual) thread pinned to its current carrier, so it cannot unmount
+ * until [body] returns; [body]'s result or exception is passed through. Only called when
+ * [needsCarrierPin] is true; every other target runs [body] directly.
+ */
+@PublishedApi
+internal expect fun <R> runOnPinnedCarrier(body: () -> R): R
+
 
 public expect fun getThreadGILState(): ThreadGILState
 public expect fun setThreadGILState(state: ThreadGILState?)
@@ -79,11 +89,20 @@ public expect fun setThreadGILState(state: ThreadGILState?)
  * that penalty. The nesting counter here already makes inner scopes cheap; on a free-threaded
  * build the outermost one stops being a bottleneck as well.
  */
-public inline fun <T> withGIL(block: () -> T): T {
+public inline fun <T> withGIL(crossinline block: () -> T): T =
+    // #161 option B: on a JVM virtual thread the outermost scope runs pinned to its carrier, from
+    // PyGILState_Ensure to Release, so a park inside it cannot move it to another OS thread. That is
+    // why `block` is crossinline: it is called from the pinned trampoline, not inlined into a frame
+    // that could return from the caller.
+    if (getThreadGILState().depth == 0 && needsCarrierPin()) runOnPinnedCarrier { gilScope(block) }
+    else gilScope(block)
+
+/** [withGIL]'s scope itself: attach on the outermost entry, count the nesting, release on the last exit. */
+@PublishedApi
+internal inline fun <T> gilScope(crossinline block: () -> T): T {
     val tState = getThreadGILState()
     val isOutermost = tState.depth == 0
     if (isOutermost) {
-        checkGILThread()
         tState.state = PyGILState_Ensure()
     }
     tState.depth++

python-multiplatform/src/commonMain/kotlin/python/multiplatform/ffi/PyObject.kt

@@ -356,7 +356,7 @@ open class PyObject(val pointer: NativePointer, borrowed: Boolean): PyAutoClosea
      * Builds an argument tuple of [size], lets [fill] populate it, and calls this object with it.
      * The whole sequence runs under a single GIL acquisition rather than one per C API call.
      */
-    private inline fun callWithTuple(size: Int, fill: (NativePointer) -> Unit): PyObject =
+    private inline fun callWithTuple(size: Int, crossinline fill: (NativePointer) -> Unit): PyObject =
         python.multiplatform.ffi.Python3.withPython {
             val argTuple = PyTuple_New(size.toLong()) ?: throw pyErrorOrGeneric("Failed to build argument tuple")
             try {

python-multiplatform/src/commonMain/kotlin/python/multiplatform/ffi/Python3.kt

@@ -20,7 +20,7 @@ object Python3 {
     /**
      * Ensure Python is initialized
      */
-    inline fun <T> withPython(block: () -> T): T {
+    inline fun <T> withPython(crossinline block: () -> T): T {
         if (!isInitialized) throw IllegalStateException("Python is not initialized")
         return withGIL {
             memScoped {

python-multiplatform/src/desktopMain/kotlin/python/multiplatform/ffi/GILCarrierPin.desktop.kt

@@ -0,0 +1,87 @@
+package python.multiplatform.ffi
+
+import python.native.ffi.Panama
+import java.lang.invoke.MethodHandle
+import java.lang.invoke.MethodHandles
+
+/** `Thread.isVirtual` exists from JDK 21. Computed once. */
+private val runtimeHasVirtualThreads: Boolean = Runtime.version().feature() >= 21
+
+/**
+ * True on a JVM virtual thread (#161). `Thread.isVirtual` is a JDK 21 API while the module targets
+ * JVM 17; the JVM links a call site only when it first runs, so the version flag is tested first and
+ * a JDK 17 to 20 runtime never reaches the call. A direct call: no lookup by name, no reflection.
+ */
+@PublishedApi
+internal actual fun needsCarrierPin(): Boolean = runtimeHasVirtualThreads && Thread.currentThread().isVirtual
+
+/**
+ * Pins the calling virtual thread for [body] (#161 option B).
+ *
+ * A virtual thread cannot unmount while a native frame is on its stack (JDK 21 to 25: "pinned"). So
+ * [body] runs inside a foreign call: C's standard `bsearch` over a one-element array calls its
+ * comparator exactly once, and the comparator is an upcall stub that runs [body]. The thread stays on
+ * this carrier from the downcall to its return, which covers [withGIL]'s `PyGILState_Ensure` to
+ * `Release`. An upcall must not throw (the JVM would terminate), so the comparator stores [body]'s
+ * result or exception in [Trampoline.pending] and returns normally; it is rethrown here.
+ */
+@PublishedApi
+internal actual fun <R> runOnPinnedCarrier(body: () -> R): R {
+    val slot = Trampoline.Pending(body)
+    val previous = Trampoline.pending.get()
+    Trampoline.pending.set(slot)
+    try {
+        // invokeExact: (long key, long base, long nmemb, long size, long compar) -> long
+        @Suppress("UNUSED_VARIABLE")
+        val found = Trampoline.bsearch.invokeExact(
+            Trampoline.element, Trampoline.element, 1L, 8L, Trampoline.comparator,
+        ) as Long
+    } finally {
+        Trampoline.pending.set(previous)
+    }
+    slot.failure?.let { throw it }
+    check(slot.ran) { "the pinned trampoline did not run its body (bsearch never called the comparator)" }
+    @Suppress("UNCHECKED_CAST")
+    return slot.result as R
+}
+
+/** The pinning trampoline's fixed parts, created once per process. */
+internal object Trampoline {
+    class Pending(val body: () -> Any?) {
+        var result: Any? = null
+        var failure: Throwable? = null
+        var ran = false
+    }
+
+    /** The body for the comparator to run, per (virtual) thread; a nested pin saves and restores it. */
+    val pending: ThreadLocal<Pending?> = ThreadLocal()
+
+    /** `bsearch(const void *key, const void *base, size_t nmemb, size_t size, int (*compar)(...))`. */
+    val bsearch: MethodHandle by lazy {
+        val long = Long::class.javaPrimitiveType!!
+        Panama.findSymbol("bsearch", long, arrayOf(long, long, long, long, long))
+    }
+
+    /** One 8-byte native element, used as both the key and the one-element array. */
+    val element: Long by lazy { Panama.allocatePointerSlot() }
+
+    /** The comparator's upcall stub address: [compare] as `int (*)(const void *, const void *)`. */
+    val comparator: Long by lazy {
+        val method = Trampoline::class.java.methods.first { it.name == "compare" }
+        Panama.createUpcallStubII_I(MethodHandles.lookup().unreflect(method))
+    }
+
+    /** Runs the pending body; never throws (an upcall must not). Returns 0: "equal", found. */
+    @JvmStatic
+    fun compare(@Suppress("UNUSED_PARAMETER") a: Long, @Suppress("UNUSED_PARAMETER") b: Long): Int {
+        val slot = pending.get() ?: return 0
+        if (slot.ran) return 0
+        slot.ran = true
+        try {
+            slot.result = slot.body()
+        } catch (t: Throwable) {
+            slot.failure = t
+        }
+        return 0
+    }
+}

python-multiplatform/src/desktopMain/kotlin/python/native/ffi/Panama.kt

@@ -128,6 +128,10 @@ internal object Panama {
 
     /** Create a Panama upcall stub for a (Long, Long, Long) -> Int MethodHandle */
     val createUpcallStubIII_I: (MethodHandle) -> Long
+
+    /** Create a Panama upcall stub for a (Long, Long) -> Int MethodHandle: a C `int (*)(const void *,
+     *  const void *)`, a comparator (#161: the pinning trampoline in GILScope.desktop.kt). */
+    val createUpcallStubII_I: (MethodHandle) -> Long
 
     /** Create a Panama upcall stub for a (Long) -> Int MethodHandle */
     val createUpcallStubI_I: (MethodHandle) -> Long
@@ -165,6 +169,7 @@ internal object Panama {
                 createUpcallStubLongToLong = data.createUpcallStubLongToLong
                 createUpcallStubII_L = data.createUpcallStubII_L
                 createUpcallStubIII_I = data.createUpcallStubIII_I
+                createUpcallStubII_I = data.createUpcallStubII_I
                 createUpcallStubI_I = data.createUpcallStubI_I
                 createUpcallStubI_V = data.createUpcallStubI_V
             }
@@ -185,6 +190,7 @@ internal object Panama {
                 createUpcallStubLongToLong = data.createUpcallStubLongToLong
                 createUpcallStubII_L = data.createUpcallStubII_L
                 createUpcallStubIII_I = data.createUpcallStubIII_I
+                createUpcallStubII_I = data.createUpcallStubII_I
                 createUpcallStubI_I = data.createUpcallStubI_I
                 createUpcallStubI_V = data.createUpcallStubI_V
             }
@@ -209,6 +215,7 @@ internal object Panama {
         val createUpcallStubLongToLong: (MethodHandle) -> Long,
         val createUpcallStubII_L: (MethodHandle) -> Long,
         val createUpcallStubIII_I: (MethodHandle) -> Long,
+        val createUpcallStubII_I: (MethodHandle) -> Long,
         val createUpcallStubI_I: (MethodHandle) -> Long,
         val createUpcallStubI_V: (MethodHandle) -> Long
     )
@@ -592,6 +599,16 @@ internal object Panama {
             segmentAddressExact.invokeExact(stub) as Long
         }
 
+        // @UpcallShape(returnType = "int", parameterTypes = ["long", "long"])
+        val buildUpcallStubII_I: (MethodHandle) -> Long = { handle ->
+            val layoutParams = java.lang.reflect.Array.newInstance(memoryLayoutClass, 2)
+            java.lang.reflect.Array.set(layoutParams, 0, javaLong)
+            java.lang.reflect.Array.set(layoutParams, 1, javaLong)
+            val fd = fdOfMethod.invoke(null, javaInt, layoutParams)
+            val stub: Any = upcallStubMethod.invoke(linker, handle, fd, globalArena, emptyOptions)
+            segmentAddressExact.invokeExact(stub) as Long
+        }
+
         // @UpcallShape(returnType = "int", parameterTypes = ["long"])
         val buildUpcallStubI_I: (MethodHandle) -> Long = { handle ->
             val layoutParams = java.lang.reflect.Array.newInstance(memoryLayoutClass, 1)
@@ -610,7 +627,7 @@ internal object Panama {
             segmentAddressExact.invokeExact(stub) as Long
         }
 
-        return ModernData(allocStr, readStr, findSym, findAddr, allocFreeable, freeAddr, allocSlot, readSlot, writeSlot, freeSlot, allocBytes, buildShape, buildUpcallStub, buildUpcallStubII_L, buildUpcallStubIII_I, buildUpcallStubI_I, buildUpcallStubI_V)
+        return ModernData(allocStr, readStr, findSym, findAddr, allocFreeable, freeAddr, allocSlot, readSlot, writeSlot, freeSlot, allocBytes, buildShape, buildUpcallStub, buildUpcallStubII_L, buildUpcallStubIII_I, buildUpcallStubII_I, buildUpcallStubI_I, buildUpcallStubI_V)
     }
 
     private fun adaptModernHandle(
@@ -688,6 +705,7 @@ internal object Panama {
         val createUpcallStubLongToLong: (MethodHandle) -> Long,
         val createUpcallStubII_L: (MethodHandle) -> Long,
         val createUpcallStubIII_I: (MethodHandle) -> Long,
+        val createUpcallStubII_I: (MethodHandle) -> Long,
         val createUpcallStubI_I: (MethodHandle) -> Long,
         val createUpcallStubI_V: (MethodHandle) -> Long
     )
@@ -954,6 +972,16 @@ internal object Panama {
             toRawLongMethod.invoke(segAddressMethod.invoke(stub)) as Long
         }
 
+        // @UpcallShape(returnType = "int", parameterTypes = ["long", "long"])
+        val buildUpcallStubII_I: (MethodHandle) -> Long = { handle ->
+            val layoutParams = java.lang.reflect.Array.newInstance(memoryLayoutClass, 2)
+            java.lang.reflect.Array.set(layoutParams, 0, cLongLong)
+            java.lang.reflect.Array.set(layoutParams, 1, cLongLong)
+            val fd = fdOfMethod.invoke(null, cInt, layoutParams)
+            val stub = upcallStubMethod.invoke(clinker, handle, fd)
+            toRawLongMethod.invoke(segAddressMethod.invoke(stub)) as Long
+        }
+
         // @UpcallShape(returnType = "int", parameterTypes = ["long"])
         val buildUpcallStubI_I: (MethodHandle) -> Long = { handle ->
             val layoutParams = java.lang.reflect.Array.newInstance(memoryLayoutClass, 1)
@@ -972,7 +1000,7 @@ internal object Panama {
             toRawLongMethod.invoke(segAddressMethod.invoke(stub)) as Long
         }
 
-        return IncubatorData(allocStr, readStr, findSym, findAddr, allocFreeable, freeAddr, allocSlot, readSlot, writeSlot, freeSlot, allocBytes, buildShape, buildUpcallStub, buildUpcallStubII_L, buildUpcallStubIII_I, buildUpcallStubI_I, buildUpcallStubI_V)
+        return IncubatorData(allocStr, readStr, findSym, findAddr, allocFreeable, freeAddr, allocSlot, readSlot, writeSlot, freeSlot, allocBytes, buildShape, buildUpcallStub, buildUpcallStubII_L, buildUpcallStubIII_I, buildUpcallStubII_I, buildUpcallStubI_I, buildUpcallStubI_V)
     }
 
     private fun adaptIncubatorHandle(