Binder: GIL release tests beyond 'does not crash' (#162)

docs/SPEC.md

@@ -176,9 +176,13 @@ fail, then implement.
 - **C-3** Android binds through `RegisterNatives` with the calling convention chosen per function;
   re-entrant functions are never `@CriticalNative`. `Status: implemented`,
   `PM/desktopTest/.../ffi/JniCallConventionClassificationTest.kt`, `PM/androidInstrumentedTest/.../JniWiringTest.kt`.
-- **C-4** Every C API call holds the GIL. `Status: partial`, enforced by convention and README rules;
-  the only direct test (`PM/commonTest/.../GilParkingTest.kt`) asserts only "does not crash". Releasing
-  the GIL after initialisation is not enabled.
+- **C-4** Every C API call holds the GIL. `Status: partial`, enforced by convention and README rules.
+  The GIL is released once `Python3.initialize` returns (it saves the main thread state with
+  `PyEval_SaveThread`), so every entry into Python goes through `withGIL`, and `withoutGIL` gives it
+  up while waiting in Kotlin. Tests: `PM/jvmTest/.../ffi/GilReleaseTest.kt` (desktop and Android
+  instrumented), `PM/nativeTest/.../ffi/GilReleaseNativeTest.kt` (iOS simulator, androidNative); a
+  second thread runs Python while the first waits in Kotlin, with bounded waits. The "every call holds
+  the GIL" half has only `PM/commonTest/.../GilParkingTest.kt` (asserts "does not crash").
 - **C-5** A thread CPython creates can upcall into ART: it is attached once per thread and detached when
   it dies. `Status: implemented` on Android, `PM/androidInstrumentedTest/.../UpcallThreadAttachTest.kt`.
 - **C-6** wasm: `Py_ssize_t` boundaries and the `-1` error return are handled at the 32-bit ABI.
@@ -586,7 +590,7 @@ All in `PM/commonTest`, so they run wherever the interpreter loads.
 ## 7. Threading and builds
 
 - **T-1** GIL builds: Kotlin threads attach a thread state and serialise on the GIL. `Status: partial`
-  (see C-4).
+  (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

python-multiplatform/src/jvmTest/kotlin/python/multiplatform/ffi/GilReleaseTest.kt

@@ -0,0 +1,73 @@
+package python.multiplatform.ffi
+
+import java.util.concurrent.CountDownLatch
+import java.util.concurrent.TimeUnit
+import java.util.concurrent.atomic.AtomicReference
+import kotlin.test.Test
+import kotlin.test.assertEquals
+import kotlin.test.assertNull
+import kotlin.test.assertTrue
+
+/**
+ * SPEC C-4 / T-1: the GIL is released once [Python3.initialize] returns (it saves the main thread
+ * state), so a second Kotlin thread can run Python while the first waits in Kotlin.
+ *
+ * `jvmTest` is shared by `desktopTest` and `androidInstrumentedTest`, so one file covers both.
+ * Modelled on `PyTypeCacheConcurrencyTest` (desktopTest) for the thread and latch shape and on
+ * `PythonTestFixture.withInterpreter` for the interpreter set-up.
+ *
+ * Every wait is bounded. If the GIL were still held, the second thread would block in
+ * `PyGILState_Ensure`, the bounded wait would time out and the assertion would fail; nothing
+ * waits forever on the thread that is blocked.
+ *
+ * Negative control (deliberately not a test, it would hang on a GIL build): thread A waiting inside
+ * `withGIL` WITHOUT `withoutGIL` keeps the GIL, so B could never get in.
+ */
+class GilReleaseTest {
+
+    private val timeoutSeconds = 20L
+
+    /** Runs `6 * 7` under the GIL on a new thread; the answer or failure lands in [result]/[error]. */
+    private fun pythonThread(done: CountDownLatch, result: AtomicReference<String?>, error: AtomicReference<Throwable?>) =
+        Thread {
+            try {
+                result.set(withGIL { PythonTestFixture.eval("6 * 7").toString() })
+            } catch (t: Throwable) {
+                error.set(t)
+            } finally {
+                done.countDown()
+            }
+        }.apply { isDaemon = true }
+
+    @Test
+    fun secondThreadRunsPythonWhileTheInitialisingThreadWaitsInKotlin() = PythonTestFixture.withInterpreter {
+        // The initialising thread is not inside withGIL here: it only waits in Kotlin.
+        val done = CountDownLatch(1)
+        val result = AtomicReference<String?>(null)
+        val error = AtomicReference<Throwable?>(null)
+        pythonThread(done, result, error).start()
+        val finished = done.await(timeoutSeconds, TimeUnit.SECONDS)
+        assertTrue(finished, "second thread could not run Python: the GIL is still held after initialisation")
+        assertNull(error.get(), "second thread failed: ${error.get()}")
+        assertEquals("42", result.get())
+    }
+
+    @Test
+    fun withoutGilLetsAnotherThreadRunPythonWhileThisOneWaits() = PythonTestFixture.withInterpreter {
+        val done = CountDownLatch(1)
+        val result = AtomicReference<String?>(null)
+        val error = AtomicReference<Throwable?>(null)
+        val b = pythonThread(done, result, error)
+        var observed = false
+        withGIL {
+            b.start()
+            // Wait in Kotlin with the GIL given up; no Python call inside this block.
+            observed = withoutGIL { done.await(timeoutSeconds, TimeUnit.SECONDS) }
+            // The GIL is held again here, so B (if it was stuck) can only proceed after we leave.
+        }
+        b.join(TimeUnit.SECONDS.toMillis(timeoutSeconds))
+        assertTrue(observed, "B did not finish while A waited in withoutGIL: withoutGIL did not release the GIL")
+        assertNull(error.get(), "B failed: ${error.get()}")
+        assertEquals("42", result.get())
+    }
+}

python-multiplatform/src/nativeTest/kotlin/python/multiplatform/ffi/GilReleaseNativeTest.kt

@@ -0,0 +1,85 @@
+package python.multiplatform.ffi
+
+import platform.posix.usleep
+import python.multiplatform.ffi.upcall.NativeThread
+import kotlin.concurrent.AtomicInt
+import kotlin.concurrent.AtomicReference
+import kotlin.test.Test
+import kotlin.test.assertEquals
+import kotlin.test.assertNull
+import kotlin.test.assertTrue
+import kotlin.time.Duration.Companion.seconds
+import kotlin.time.TimeSource
+
+/**
+ * SPEC C-4 / T-1 on Kotlin/Native (iOS simulator, androidNative): the GIL is released once
+ * [Python3.initialize] returns, so a second thread can run Python while the first waits in Kotlin.
+ * The mirror of `jvmTest`'s `GilReleaseTest`.
+ *
+ * Modelled on `AsyncUpcallNativeDeliveryTest` (nativeTest): same [NativeThread] (a bare
+ * `pthread_create`) and the same `PythonTestFixture` interpreter set-up.
+ *
+ * Every wait is bounded by [waitFor]. If the GIL were still held the second thread would block in
+ * `PyGILState_Ensure`; the bounded wait expires and the assertion fails. A thread that never got in
+ * is deliberately not joined (that join would hang); the process ends the test run.
+ *
+ * Negative control (not a test, it would hang on a GIL build): thread A waiting inside `withGIL`
+ * without `withoutGIL` keeps the GIL, so B could never get in.
+ */
+class GilReleaseNativeTest {
+
+    private val timeout = 20.seconds
+
+    private fun waitFor(flag: AtomicInt): Boolean {
+        val mark = TimeSource.Monotonic.markNow()
+        while (flag.value == 0) {
+            if (mark.elapsedNow() > timeout) return false
+            usleep(1000u)
+        }
+        return true
+    }
+
+    private class Outcome {
+        val done = AtomicInt(0)
+        val result = AtomicReference<String?>(null)
+        val error = AtomicReference<String?>(null)
+    }
+
+    private fun pythonThread(o: Outcome) = NativeThread {
+        try {
+            o.result.value = withGIL { PythonTestFixture.eval("6 * 7").toString() }
+        } catch (t: Throwable) {
+            o.error.value = t.toString()
+        } finally {
+            o.done.value = 1
+        }
+    }
+
+    @Test
+    fun secondThreadRunsPythonWhileTheInitialisingThreadWaitsInKotlin() = PythonTestFixture.withInterpreter {
+        val o = Outcome()
+        val b = pythonThread(o)
+        b.start()
+        val finished = waitFor(o.done)
+        assertTrue(finished, "second thread could not run Python: the GIL is still held after initialisation")
+        b.join()
+        assertNull(o.error.value, "second thread failed: ${o.error.value}")
+        assertEquals("42", o.result.value)
+    }
+
+    @Test
+    fun withoutGilLetsAnotherThreadRunPythonWhileThisOneWaits() = PythonTestFixture.withInterpreter {
+        val o = Outcome()
+        val b = pythonThread(o)
+        var observed = false
+        withGIL {
+            b.start()
+            // Wait in Kotlin with the GIL given up; no Python call inside this block.
+            observed = withoutGIL { waitFor(o.done) }
+        }
+        assertTrue(observed, "B did not finish while A waited in withoutGIL: withoutGIL did not release the GIL")
+        b.join()
+        assertNull(o.error.value, "B failed: ${o.error.value}")
+        assertEquals("42", o.result.value)
+    }
+}