fix(connections): read every subprocess pipe through one reader that cannot raise or read after it stops

[datlechin]

The Unit tests job of run 36150062875 (attempt 1, PR #3130) aborted the test host with NSFileHandleOperationException: -[NSConcreteFileHandle availableData]: unknown error, raised inside the stderr readabilityHandler of ProcessNativeDumpRunner. XCTest pinned it on AppSettingsJsonExtractorTests/testNpgsql() only because that case was running when the process died. Attempt 2 passed. The same handler runs for every backup and restore driven by mysqldump, pg_dump and the rest, so the app could die the same way.

What was wrong

The xcresult's StandardOutputAndStandardError.txt has the whole sequence, 52ms after ProcessNativeDumpRunnerTests passed:

[general] Encountered read failure 35 Resource temporarily unavailable
*** Terminating app due to uncaught exception 'NSFileHandleOperationException',
    reason: '*** -[NSConcreteFileHandle availableData]: unknown error'
  3 Foundation  -[NSConcreteFileHandle availableData] + 312
  4 TablePro.debug.dylib  closure #1 in ProcessNativeDumpRunner.start()
  6 Foundation  __33-[NSConcreteFileHandle _monitor:]_block_invoke + 68

Read failure 35 is EAGAIN, which a read only gets from a non-blocking descriptor. The one thing that made this pipe non-blocking was drainStderr, added by #3049 (unreleased): the termination handler cleared readabilityHandler, set O_NONBLOCK on the read end and read it empty under stderrLock. Clearing the handler cancels the dispatch source but does not recall a callback Foundation has already dispatched. That callback waited on stderrLock for the drain, then called availableData on a pipe that was now empty, non-blocking, and still held open by a writer (the test's surviving grandchild, or any child the app is spawning at that moment). availableData turns every failed read into an Objective-C exception, and one unwinding through Swift ends the process. The "unknown error" is errno clobbered by Foundation's own log call before it built the exception.

Two faults, then: a read API that raises instead of reporting, and a teardown that lets a late callback read a descriptor the owner has already drained. LSPTransport.stop() has the second one in its other form: it cleared the stderr handler and then closed the handle, so a callback already dispatched reads a closed handle, which raises too.

FileHandle.read(upToCount:) throws instead of raising, but it cannot stand in for availableData. Measured on a pipe: with 6 bytes waiting and the writer open, read(upToCount: 65_536) blocked until killed, because it waits for the whole count or end of file; on a non-blocking pipe it read the 6 bytes, hit EAGAIN, threw, and discarded them.

What changed

• DescriptorRead (Core/Process): one read(2) of whatever has arrived, EINTR retried, empty at end of file, a failure thrown as POSIXError. hasInputWithoutWaiting asks poll(2) whether a read would return at once, so a drain never has to set O_NONBLOCK on a descriptor another reader shares.
• PipeReader (Core/Process): owns a read end's readabilityHandler. Every read happens under one lock and only while a consumer is installed. Both ways of stopping clear the handler and then remove the consumer under that lock, so once they return nothing is reading and nothing will: a callback Foundation dispatched earlier finds no consumer and returns without touching the descriptor. stop(drainingUpTo:) first hands over what the pipe holds, up to a limit, without waiting on a writer. stopAtEndOfFile() reads until every writer has closed. A read error is logged and ends reading; EAGAIN is a callback with nothing to read and keeps it going; end of file clears the handler, where availableData left Foundation calling back with empty data until someone cleared it.
• Every site moves onto it, with its old drain semantics:
  • ProcessNativeDumpRunner: stop(drainingUpTo: stderrByteCap), the bounded drain it already had, now without O_NONBLOCK. drainStderr and its duplicated doc paragraph are gone.
  • ProcessSupervisedRunner: stopAtEndOfFile(), which is what readToEnd() did. ingestLock is gone; the reader's lock gives the fix(connections): keep a helper process last stderr line when it exits right after writing it #2613 guarantee that no line reaches the stream after finish closes it.
  • PreConnectHookRunner: stop(drainingUpTo: pipeCapacity). It had no drain, so a script that wrote its reason and exited at once could come back as Pre-connect script failed with exit code 3 and no message.
  • LSPTransport.stop(): stops the reader before closing the handle.
  • BridgeStdin (the tablepro-mcp helper): reads stdin through DescriptorRead, and a failed read is logged and ends the stream instead of aborting the helper. DescriptorRead.swift joins the mcp-server target in project.yml.
  • MCPBridgeIntegrationTests' harness had the same handler shape in the test host, and now uses PipeReader.

Why not the other shapes

• Keeping availableData and only fencing the late callback would fix this ordering and leave the next closed or non-blocking descriptor fatal. Replacing only the read would leave LSPTransport closing a handle a callback may still be reading, which is exactly the descriptor-reuse hazard a raw read(2) would otherwise walk into.
• DispatchIO stream channels close the descriptor safely, but give no "take what is buffered now and stop" for a pipe a grandchild keeps open, which is the drain the dump runner needs.
• ProcessSupervisedRunner keeps reading to end of file. A bounded drain would stop its termination from waiting on a helper that inherited stderr (measured: 5.01s against 0.008s with a sleep 5 & helper), but that wait is also what keeps a command that backgrounds its own helper counted as running, so changing it is a product decision and not part of this fix.

Verification

Standalone harnesses compile the real source file (at origin/main and at this branch) with swiftc -O and a main.swift, 16 runs at a time:

Site	Workload	origin/main	This branch
ProcessNativeDumpRunner	200 rounds per launch, half immediate exits, half with a grandchild still writing stderr	launch aborted in 3 of 40 on a quiet machine and 9 of 40 beside a build: 12 of 80 launches, 256,000 runs	0 of 80; every stderr captured, every cap held
same, replaying a callback dispatched before the drain	10 runs	raised in 10 of 10	0 of 10
PreConnectHookRunner	failing script, 16,000 runs	message lost in 352 (quiet machine), 1,850 (beside a build)	0, 0
ProcessSupervisedRunner	two stderr lines then exit, 64,000 runs	0 lines lost	0 lines lost
LSPTransport	start and stop against a server writing stderr nonstop, 64,000 cycles	0 crashes	0 crashes, no hang

The harness reproduced CI's exact signature, including Encountered read failure 35 and the same 16-frame stack. Instrumenting main's runner showed the mechanism: in 16,000 runs, 12 callbacks ran after the drain, all 12 on a non-blocking descriptor, 6 of them on an empty pipe. The LSPTransport window is real but too narrow to hit in 64,000 stops; it changed for the shape, not a measured crash.

• New DescriptorReadTests (6): arrived bytes returned without waiting, the limit, end of file, EAGAIN and EBADF thrown, poll readiness.
• New PipeReaderTests (6): order and self-stop at end of file; a callback dispatched before a drain and run after it reads nothing (the CI ordering) and leaves the descriptor blocking; a callback with nothing to read keeps the reader going; the bounded drain; the end-of-file drain; stop waits for a delivery in progress.
• New PreConnectHookRunnerTests: 64 failing scripts, 16 at a time, each error carries the script's message. Against origin/main's runner it failed 3 runs out of 3.
• verify.sh build PASS (both the app and mcp-server compile the new files). verify.sh test DescriptorReadTests PipeReaderTests SupervisedProcessRunnerTests ProcessNativeDumpRunnerTests MCPBridgeIntegrationTests TunnelCommandManagerTests CloudSQLProxyManagerTests CloudflareTunnelManagerTests PreConnectHookRunnerTests: 66/66 PASS (xcresult counts).
• swiftlint lint --strict on every changed Swift file: 0 violations. check-log-privacy.py, check-test-suite-attributes.py, audit-refactor-health.sh --check: pass.

One test fact worth keeping: end of file is not immediate after closing a pipe's write end in a process that is spawning children. With four threads launching /usr/bin/true, 14 of 20,000 closes were not yet end of file at once (0 of 20,000 without them), so the readiness test waits for it.

Not covered

• The crash itself came from fix(export): give a dump the TLS options its own client tool takes #3049's drain, which has not shipped, so there is no CHANGELOG line for it; the existing entry for that drain still describes it. The pre-connect message loss has shipped and gets its own line.
• Other FileHandle reads that raise on error (readDataToEndOfFile, readData(ofLength:)) remain in GzipProcess, CLIExecutableFinder, AgentAuthenticator, PrivilegedShell, CellImageRenderer, AWSCredentialResolver and a few plugins. Each reads a pipe synchronously after its process exits, with no handler racing it, so none has this failure shape. They are left for their own change.
• LSPTransport.writeMessage uses the raising FileHandle.write(_:), which can abort on EPIPE once the server has exited. Same exception class, a different path, not changed here.
• No UI flow changed, so there is no TableProUITests automation.