feat(go): streaming-stdio Popen — Process with per-stream StdioMode (RFC #67)#124
Conversation
…RFC multikernel#67) Go ergonomic layer over the C ABI sandlock_popen, mirroring the FFI and Python bindings. Sandbox.Popen(stdio, cmd...) forks the confined child with per-stream stdio and returns a live *Process — the streaming counterpart of Spawn. Each stream set to StdioPiped is handed back on the Process as an *os.File (Stdin/Stdout/Stderr) the caller reads/writes while the child runs; inherit/null streams leave it nil. The zero Stdio inherits all three (identical to Spawn). - StdioMode (Inherit=0/Piped=1/Null=2) shares the stable ABI discriminants; an out-of-range mode is rejected before the child is spawned. - os.NewFile takes ownership of each piped fd, so closing the file closes the fd — the fds never leak (via Close, the SetFinalizer reap, or the *os.File's own finalizer). On a null handle the FFI leaves the out-fds at -1, so no stream is wrapped on the error path. - Wait closes a still-open piped Stdin first to deliver EOF (so a reader child like cat exits), then waits and frees the handle. Close closes all piped streams regardless of handle state and reaps the child. - No wait-timeout is added: unlike the Python binding, Kill (process-group signal by PID) already interrupts a blocked Wait from another goroutine, so the undrained-pipe hang has a built-in escape hatch. Tests mirror the FFI/Python popen suites: stdout streaming, stdin/stdout roundtrip through cat, wait-closes-unclosed-stdin (watchdog), both stdout+stderr piped, null yields no stream, invalid mode rejected, Kill interrupts a blocked Wait, Close closes streams + reaps. README documents Popen/Stdio. Refs multikernel#67
| // the duration, so no other handle operation aliases it. | ||
| // | ||
| // Wait closes a still-open piped Stdin to deliver EOF, so do not write to Stdin | ||
| // from another goroutine while Wait runs — the write would race a closing fd. |
There was a problem hiding this comment.
Doc gap: nothing says the Result of a Popen'd process carries no captured output. The Python binding documents this in both popen and Process.wait; here neither Wait's doc nor the README's Popen bullet mentions it, and the README still describes Wait as returning a "captured Result". A user coming from Run will read res.Stdout and get an empty slice with no hint why (the bytes went to p.Stdout, possibly already drained). One sentence here and one in the README fixes it.
| // before Wait, or a child that reads to EOF (e.g. cat) never exits and Wait | ||
| // blocks. Likewise drain a piped Stdout/Stderr before Wait, or a child that | ||
| // fills the pipe buffer blocks on write. As an escape hatch, Kill from another | ||
| // goroutine interrupts a blocked Wait; Close reaps the child and closes the |
There was a problem hiding this comment.
Kill-after-exit idempotency now differs across the three bindings. FFI sandlock_handle_kill returns 0 for an already-exited process, Python Process.kill() is a silent no-op after wait(), but Go's Kill() returns ErrNotRunning once the handle is freed (signal checks p.h == nil before the ESRCH mapping can apply). Since this doc actively promotes "Kill from another goroutine" as the escape hatch, the watchdog-races-normal-exit case (Wait returns just as the watchdog fires Kill) yields an error in Go where the other two bindings say success. Pre-existing signal semantics, but worth aligning while the contract is being written down: Kill on an already-reaped Process should arguably be nil too.
|
|
||
| // Stdio selects the wiring of a Popen'd process's three standard streams. The | ||
| // zero value wires all three as StdioInherit, matching Spawn. | ||
| type Stdio struct { |
There was a problem hiding this comment.
Test gap: the zero-Stdio-equals-Spawn contract is stated in three places (here, the Popen doc, the README) but never exercised. TestPopenNullStdoutNoStream is the closest test. A cheap Popen(sandlock.Stdio{}, ...) test asserting all three stream fields are nil and the child runs to success would pin the ABI default (StdioInherit == 0) that the zero-value contract relies on.
…zero Stdio - Kill: a process already reaped by Wait now returns nil (like ESRCH), not ErrNotRunning — matching the FFI (sandlock_handle_kill returns 0) and the Python binding (no-op after wait()). Keeps the "Kill from another goroutine" escape hatch race-free when a Kill fires just as Wait reaps the child. Only the reaped case (h == nil) collapses to nil; a live handle with no pid stays an error, so a genuine no-op kill is never reported as success (and killpg(-0) is never reachable). - Wait doc + README: state that a Popen'd process's Result carries the exit status only; piped output was streamed to Stdout/Stderr and inherited/null output went to the parent fd or /dev/null, so (unlike Run) Result.Stdout/Result.Stderr are always empty. - Tests: zero-Stdio-equals-Spawn (all three streams inherited, no caller stream, child succeeds) pins the StdioInherit==0 zero-value contract; Kill-after-Wait-is-nil covers the idempotency alignment above.
|
Thanks — addressed all three:
Force-pushed; CI green. |
2 participants
Go ergonomic layer over the C ABI
sandlock_popen(merged in #120), mirroring the FFI and Python bindings. Part of the RFC #67 popen series (core #118, FFI #120).Sandbox.Popen(stdio Stdio, cmd...) (*Process, error)forks the confined child with per-stream stdio — the streaming counterpart ofSpawn. Each stream set toStdioPipedis handed back on the*Processas an*os.File(Stdin/Stdout/Stderr); inherit/null streams leave it nil. The zeroStdioinherits all three (identical toSpawn).StdioMode(Inherit=0/Piped=1/Null=2) shares the stable ABI discriminants; an out-of-range mode is rejected before the child is spawned.os.NewFiletakes ownership of each piped fd, so closing the file closes the fd — no leaks (viaClose, theSetFinalizerreap, or the*os.File's own finalizer). On a null handle the FFI leaves the out-fds at -1, so no stream is wrapped on the error path.Waitcloses a still-open pipedStdinfirst to deliver EOF (so a reader child likecatexits), then waits and frees the handle.Closecloses all piped streams regardless of handle state and reaps the child.Kill(process-group signal by PID) already interrupts a blockedWaitfrom another goroutine, so the undrained-pipe hang has a built-in escape hatch.Tests mirror the FFI/Python popen suites: stdout streaming, stdin/stdout roundtrip through
cat, wait-closes-unclosed-stdin (watchdog), both stdout+stderr piped, null yields no stream, invalid mode rejected, Kill interrupts a blocked Wait, Close closes streams + reaps, finalizer reaps an abandoned process, Close-after-Wait. README documentsPopen/Stdio.-raceclean.Refs #67