From 0d473b1e76adb2316c22839bf37d47653864839f Mon Sep 17 00:00:00 2001 From: BREW <81044362+b-re-w@users.noreply.github.com> Date: Sun, 25 Oct 2026 22:31:00 +0900 Subject: [PATCH] Feat: withGIL pins a JVM virtual thread to its carrier instead of refusing it (#161 option B) withGIL's and withPython's blocks become crossinline (a public API change, user-approved) so the outermost scope on a virtual thread can run inside a native frame: C's bsearch over one element calls its comparator, an FFM upcall that runs the scope, so the virtual thread cannot unmount between PyGILState_Ensure and Release. The result or exception is carried out of the upcall. Platform threads, Android, native and wasm run the block directly. Panama gains the (long, long) -> int upcall shape; its reachability descriptor is generated from the @UpcallShape marker. --- docs/SPEC.md | 15 +-- .../ffi/GILCarrierPin.android.kt | 8 ++ .../ffi/GILThreadCheck.android.kt | 5 - .../python/multiplatform/ffi/GILScope.kt | 31 ++++-- .../python/multiplatform/ffi/PyObject.kt | 2 +- .../python/multiplatform/ffi/Python3.kt | 2 +- .../ffi/GILCarrierPin.desktop.kt | 87 +++++++++++++++++ .../ffi/GILThreadCheck.desktop.kt | 31 ------ .../kotlin/python/native/ffi/Panama.kt | 32 ++++++- .../multiplatform/ffi/VirtualThreadGILTest.kt | 95 +++++++++++++------ .../native/ffi/ReachabilityMetadataTest.kt | 3 + .../multiplatform/ffi/GILCarrierPin.native.kt | 8 ++ .../ffi/GILThreadCheck.native.kt | 5 - .../multiplatform/ffi/GILCarrierPin.wasmJs.kt | 8 ++ .../ffi/GILThreadCheck.wasmJs.kt | 5 - 15 files changed, 247 insertions(+), 90 deletions(-) create mode 100644 python-multiplatform/src/androidMain/kotlin/python/multiplatform/ffi/GILCarrierPin.android.kt delete mode 100644 python-multiplatform/src/androidMain/kotlin/python/multiplatform/ffi/GILThreadCheck.android.kt create mode 100644 python-multiplatform/src/desktopMain/kotlin/python/multiplatform/ffi/GILCarrierPin.desktop.kt delete mode 100644 python-multiplatform/src/desktopMain/kotlin/python/multiplatform/ffi/GILThreadCheck.desktop.kt create mode 100644 python-multiplatform/src/nativeMain/kotlin/python/multiplatform/ffi/GILCarrierPin.native.kt delete mode 100644 python-multiplatform/src/nativeMain/kotlin/python/multiplatform/ffi/GILThreadCheck.native.kt create mode 100644 python-multiplatform/src/wasmJsMain/kotlin/python/multiplatform/ffi/GILCarrierPin.wasmJs.kt delete mode 100644 python-multiplatform/src/wasmJsMain/kotlin/python/multiplatform/ffi/GILThreadCheck.wasmJs.kt diff --git a/docs/SPEC.md b/docs/SPEC.md index 6889cfddd..1ba12bbc1 100644 --- a/docs/SPEC.md +++ 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`. diff --git a/python-multiplatform/src/androidMain/kotlin/python/multiplatform/ffi/GILCarrierPin.android.kt b/python-multiplatform/src/androidMain/kotlin/python/multiplatform/ffi/GILCarrierPin.android.kt new file mode 100644 index 000000000..f10f43c6a --- /dev/null +++ b/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 runOnPinnedCarrier(body: () -> R): R = body() 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 deleted file mode 100644 index 71490d22d..000000000 --- a/python-multiplatform/src/androidMain/kotlin/python/multiplatform/ffi/GILThreadCheck.android.kt +++ /dev/null @@ -1,5 +0,0 @@ -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 0ea134c50..4476f2670 100644 --- a/python-multiplatform/src/commonMain/kotlin/python/multiplatform/ffi/GILScope.kt +++ b/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 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 withGIL(block: () -> T): T { +public inline fun 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 gilScope(crossinline 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/commonMain/kotlin/python/multiplatform/ffi/PyObject.kt b/python-multiplatform/src/commonMain/kotlin/python/multiplatform/ffi/PyObject.kt index 51b019301..1de9a243f 100644 --- a/python-multiplatform/src/commonMain/kotlin/python/multiplatform/ffi/PyObject.kt +++ b/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 { diff --git a/python-multiplatform/src/commonMain/kotlin/python/multiplatform/ffi/Python3.kt b/python-multiplatform/src/commonMain/kotlin/python/multiplatform/ffi/Python3.kt index ca637c399..14d153326 100644 --- a/python-multiplatform/src/commonMain/kotlin/python/multiplatform/ffi/Python3.kt +++ b/python-multiplatform/src/commonMain/kotlin/python/multiplatform/ffi/Python3.kt @@ -20,7 +20,7 @@ object Python3 { /** * Ensure Python is initialized */ - inline fun withPython(block: () -> T): T { + inline fun withPython(crossinline block: () -> T): T { if (!isInitialized) throw IllegalStateException("Python is not initialized") return withGIL { memScoped { diff --git a/python-multiplatform/src/desktopMain/kotlin/python/multiplatform/ffi/GILCarrierPin.desktop.kt b/python-multiplatform/src/desktopMain/kotlin/python/multiplatform/ffi/GILCarrierPin.desktop.kt new file mode 100644 index 000000000..086cb1ed4 --- /dev/null +++ b/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 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 = 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 + } +} 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 deleted file mode 100644 index 0886fee58..000000000 --- a/python-multiplatform/src/desktopMain/kotlin/python/multiplatform/ffi/GILThreadCheck.desktop.kt +++ /dev/null @@ -1,31 +0,0 @@ -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/desktopMain/kotlin/python/native/ffi/Panama.kt b/python-multiplatform/src/desktopMain/kotlin/python/native/ffi/Panama.kt index 21054441e..8ee385c50 100644 --- a/python-multiplatform/src/desktopMain/kotlin/python/native/ffi/Panama.kt +++ b/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( diff --git a/python-multiplatform/src/desktopTest/kotlin/python/multiplatform/ffi/VirtualThreadGILTest.kt b/python-multiplatform/src/desktopTest/kotlin/python/multiplatform/ffi/VirtualThreadGILTest.kt index 7e4982a04..c57eb16d9 100644 --- a/python-multiplatform/src/desktopTest/kotlin/python/multiplatform/ffi/VirtualThreadGILTest.kt +++ b/python-multiplatform/src/desktopTest/kotlin/python/multiplatform/ffi/VirtualThreadGILTest.kt @@ -1,54 +1,93 @@ package python.multiplatform.ffi -import java.util.concurrent.atomic.AtomicReference +import python.native.ffi.Panama +import java.lang.invoke.MethodHandle +import java.util.concurrent.ConcurrentLinkedQueue +import java.util.concurrent.TimeUnit +import java.util.concurrent.locks.LockSupport 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. + * Issue #161 option B: on a JVM virtual thread the outermost [withGIL] runs pinned to its OS carrier, + * so a virtual thread that parks inside the scope resumes on the same OS thread that called + * `PyGILState_Ensure`, and `Release` runs there too. + * + * Red without the pin (`runOnPinnedCarrier` calling its body directly): 16 virtual threads that each + * park several times inside the scope are rescheduled onto other carriers, so the OS thread recorded + * after a park differs from the one recorded at entry. */ class VirtualThreadGILTest { private fun virtualThreadsAvailable() = Runtime.version().feature() >= 21 + /** The OS thread the caller runs on right now (`pthread_self`), through the same Panama path. */ + private val pthreadSelf: MethodHandle by lazy { + val long = Long::class.javaPrimitiveType!! + Panama.findSymbol("pthread_self", long, arrayOf()) + } + + private fun osThread(): Long = pthreadSelf.invokeExact() as Long + + @Test + fun aVirtualThreadParkingInsideWithGilStaysOnOneOsThread() = PythonTestFixture.withInterpreter { + if (!virtualThreadsAvailable() || System.getProperty("os.name").startsWith("Windows")) return@withInterpreter + val moved = ConcurrentLinkedQueue() + val failures = ConcurrentLinkedQueue() + val threads = List(16) { k -> + Thread.ofVirtual().unstarted { + try { + withGIL { + val entry = osThread() + repeat(5) { round -> + if (round % 2 == 0) Thread.sleep(2) else LockSupport.parkNanos(TimeUnit.MILLISECONDS.toNanos(2)) + val now = osThread() + if (now != entry) moved += "thread $k round $round: $entry -> $now" + } + // still attached and usable on the same carrier + check(PythonTestFixture.eval("6 * 7").toString() == "42") + } + } catch (t: Throwable) { + failures += t + } + } + } + threads.forEach { it.start() } + threads.forEach { it.join(60_000) } + assertTrue(threads.none { it.isAlive }, "a virtual thread did not finish") + assertTrue(failures.isEmpty(), "failures: ${failures.map { it.toString() }}") + assertTrue(moved.isEmpty(), "a virtual thread changed OS thread inside withGIL: ${moved.take(5)}") + } + @Test - fun virtualThreadIsRefusedAndGilIsNotLeaked() = PythonTestFixture.withInterpreter { + fun anExceptionInsideAPinnedScopeReachesTheCaller() = PythonTestFixture.withInterpreter { if (!virtualThreadsAvailable()) return@withInterpreter - val entered = java.util.concurrent.atomic.AtomicBoolean(false) - val failure = AtomicReference(null) + var seen: Throwable? = null val t = Thread.ofVirtual().start { try { - withGIL { entered.set(true) } - } catch (e: Throwable) { - failure.set(e) + withGIL { throw IllegalArgumentException("from inside") } + } catch (e: IllegalArgumentException) { + seen = 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 }) } + assertEquals("from inside", seen?.message) + // the GIL was released on the way out: a platform thread can take it at once + val p = Thread { withGIL { PythonTestFixture.eval("1 + 1") } } p.start() p.join(30_000) - assertEquals(42, result.get(), "platform thread withGIL must complete (GIL not leaked)") + assertTrue(!p.isAlive, "the GIL was leaked by the failed pinned scope") } @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 } }) + fun nestedScopesAndPlatformThreadsAreUnchanged() = PythonTestFixture.withInterpreter { + val inner = withGIL { withGIL { PythonTestFixture.eval("2 + 3").toString() } } + assertEquals("5", inner) + if (!virtualThreadsAvailable()) return@withInterpreter + var nested = "" + val t = Thread.ofVirtual().start { nested = withGIL { withGIL { PythonTestFixture.eval("3 * 3").toString() } } } + t.join(30_000) + assertEquals("9", nested) } } diff --git a/python-multiplatform/src/desktopTest/kotlin/python/native/ffi/ReachabilityMetadataTest.kt b/python-multiplatform/src/desktopTest/kotlin/python/native/ffi/ReachabilityMetadataTest.kt index b608e41b1..27a43f9e6 100644 --- a/python-multiplatform/src/desktopTest/kotlin/python/native/ffi/ReachabilityMetadataTest.kt +++ b/python-multiplatform/src/desktopTest/kotlin/python/native/ffi/ReachabilityMetadataTest.kt @@ -192,6 +192,7 @@ class ReachabilityMetadataTest { "createUpcallStubI_V", "createUpcallStubII_L", "createUpcallStubIII_I", + "createUpcallStubII_I", // #161: the pinning trampoline's comparator "createUpcallStubLongToLong", ), entryPoints.toSet(), @@ -209,6 +210,8 @@ class ReachabilityMetadataTest { Descriptor("long", listOf("long", "long")), Descriptor("int", listOf("long")), Descriptor("int", listOf("long", "long", "long")), + // #161: C's `int (*)(const void *, const void *)` comparator, the virtual-thread pin + Descriptor("int", listOf("long", "long")), // tp_dealloc: destructor returns void, so this shape exists as of the // handle-leak fix. The int shape only survived that mismatch by ABI accident. Descriptor("void", listOf("long")), diff --git a/python-multiplatform/src/nativeMain/kotlin/python/multiplatform/ffi/GILCarrierPin.native.kt b/python-multiplatform/src/nativeMain/kotlin/python/multiplatform/ffi/GILCarrierPin.native.kt new file mode 100644 index 000000000..f10f43c6a --- /dev/null +++ b/python-multiplatform/src/nativeMain/kotlin/python/multiplatform/ffi/GILCarrierPin.native.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 runOnPinnedCarrier(body: () -> R): R = body() 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 deleted file mode 100644 index 71490d22d..000000000 --- a/python-multiplatform/src/nativeMain/kotlin/python/multiplatform/ffi/GILThreadCheck.native.kt +++ /dev/null @@ -1,5 +0,0 @@ -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/GILCarrierPin.wasmJs.kt b/python-multiplatform/src/wasmJsMain/kotlin/python/multiplatform/ffi/GILCarrierPin.wasmJs.kt new file mode 100644 index 000000000..f10f43c6a --- /dev/null +++ b/python-multiplatform/src/wasmJsMain/kotlin/python/multiplatform/ffi/GILCarrierPin.wasmJs.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 runOnPinnedCarrier(body: () -> R): R = body() 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 deleted file mode 100644 index 71490d22d..000000000 --- a/python-multiplatform/src/wasmJsMain/kotlin/python/multiplatform/ffi/GILThreadCheck.wasmJs.kt +++ /dev/null @@ -1,5 +0,0 @@ -package python.multiplatform.ffi - -/** No virtual threads on this target; nothing to check (see the common declaration). */ -@PublishedApi -internal actual fun checkGILThread() {}