Close the ConPTY output pipe before the HPCON on cancellation

.github/workflows/ci.yml

@@ -10,13 +10,19 @@ jobs:
   test:
     name: test (${{ matrix.os }})
     runs-on: ${{ matrix.os }}
+    # A job takes about a minute. A pty teardown that deadlocks hangs the
+    # test binary instead of failing it, so bound the job.
+    timeout-minutes: 10
     strategy:
       fail-fast: false
       matrix:
         os:
           - ubuntu-latest
           - macos-latest
           - windows-latest
+          # Older than Windows 11 24H2: its `ClosePseudoConsole` still waits
+          # for the pseudoconsole to exit (docs/invariants.md).
+          - windows-2022
 
     steps:
       - name: Checkout

docs/invariants.md

@@ -74,6 +74,21 @@ rationale. Nothing here is pending work; it is documented so it doesn't get
   std handles into the child and its output bypasses the ConPTY.
 - The ConPTY output pipe only reaches EOF after `ClosePseudoConsole`, so
   the wait task must close the HPCON as soon as `wait_pid` returns.
+- The cancellation cleanup closes our end of the output pipe *before*
+  `ClosePseudoConsole` (decided 2026-10-11). Before Windows 11 24H2 (build
+  26100) `ClosePseudoConsole` waits for the pseudoconsole to exit, and the
+  pseudoconsole blocks writing to a full output pipe; once the caller has
+  stopped reading, closing the HPCON first can wait forever. The call runs on
+  the event-loop thread, so nothing else runs either — not the grace-period
+  kill, not a timeout. Microsoft's documentation requires closing the output
+  pipe first or reading it until the call returns. From 24H2 on the call
+  returns immediately, so a runner that new cannot catch a regression: the
+  `windows-2022` CI job exists for this, and the flooding-child test in
+  `pty_win32_test.mbt` fails there by running into the job's
+  `timeout-minutes`. The normal exit path is deliberately different — it
+  keeps the pipe open, because the caller may still read up to the EOF that
+  only arrives after `ClosePseudoConsole` — and therefore still depends on
+  the caller draining the pty on those Windows versions.
 - `ClosePseudoConsole` must never run twice on the same HPCON (heap
   corruption, 0xC0000374) — closing is guarded by `hpc : Ref[PseudoConsole?]`.
 - `CreateProcessW` mutates the command-line buffer in place; always pass a

pty_win32.mbt

@@ -168,10 +168,13 @@ pub async fn[X] spawn(
   let session : FixedArray[@handle.Handle] = [invalid_handle, invalid_handle]
   let pid = pty_win32_spawn(hpc, session, cmd~, app~, env~, cwd~)
   if pid < 0 {
+    // Read the error before the cleanup below, like the unix side does: the
+    // closes are free to overwrite the thread's last-error value.
+    let errno = @os_error.get_errno()
     input_writer.close()
     output_reader.close()
     hpc.close()
-    raise @os_error.OSError(@os_error.get_errno(), context="@pty.spawn")
+    raise @os_error.OSError(errno, context="@pty.spawn")
   }
   guard! session is [process_handle, job_handle]
   let process = Process(process_handle)
@@ -198,6 +201,13 @@ pub async fn[X] spawn(
       // that makes the cleanup run at all.
       @async.protect_from_cancel() <| () => {
         wait_pid_with(pid) <| () => {
+          // Our end of the output pipe goes first. Before Windows 11 24H2
+          // `ClosePseudoConsole` waits for the pseudoconsole to exit, and
+          // the pseudoconsole blocks on a full output pipe that nobody
+          // reads any more: closing the HPCON first can hang right here, on
+          // the event-loop thread (docs/invariants.md). The group defer
+          // closes the pipe again; `RawFdStream::close` is idempotent.
+          output_reader.close()
           close_pseudo_console()
           // Grace period before the hard kill; matches the ~5s the OS
           // itself grants on CTRL_CLOSE_EVENT. Keep in sync with the

pty_win32_test.mbt

@@ -187,3 +187,29 @@ async test "@pty.Pty::resize raises EBADF after the group closed the pty" {
     _ => fail("expected resize on a closed pty to raise EBADF")
   }
 }
+
+///|
+// Guards the ConPTY teardown order recorded in docs/invariants.md: the child
+// floods the pseudoconsole and nobody reads the output pipe, so by the time
+// the group body returns the pipe is full and the pseudoconsole is blocked
+// writing to it. Before Windows 11 24H2 a `ClosePseudoConsole` issued in
+// that state never returns, and it runs on the event-loop thread, so no
+// timeout can fire either: the failure mode of this test is the CI job's
+// `timeout-minutes`, not an assertion. The bound covers the 5000ms grace
+// period plus the hard kill.
+#cfg(platform="windows")
+async test "@pty.spawn cancellation ends a child that floods an unread pty" {
+  let completed = @async.with_timeout_opt(15000) <| () => {
+    @async.with_task_group() <| group => {
+      @pty.spawn(
+        group,
+        "cmd",
+        ["/c", "for /L %i in (0,0,1) do @echo flooding the pseudoconsole"],
+        no_wait=true,
+      )
+      |> ignore()
+      @async.sleep(1000)
+    }
+  }
+  assert_true(completed is Some(_))
+}