-
Notifications
You must be signed in to change notification settings - Fork 29
feat(go): streaming-stdio Popen — Process with per-stream StdioMode (RFC #67) #124
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -22,6 +22,7 @@ import ( | |
| "context" | ||
| "encoding/json" | ||
| "fmt" | ||
| "os" | ||
| "runtime" | ||
| "strings" | ||
| "sync" | ||
|
|
@@ -799,6 +800,13 @@ type Process struct { | |
| h *C.sandlock_handle_t | ||
| pid int | ||
| waiting bool // a Wait owns the handle; other handle ops must defer to it | ||
|
|
||
| // Caller-owned stdio for a process started by Popen. Each is non-nil only | ||
| // for a stream wired StdioPiped; it owns the pipe fd (closing the file | ||
| // closes the fd). Nil for Spawn'd processes and for inherit/null streams. | ||
| Stdin *os.File | ||
| Stdout *os.File | ||
| Stderr *os.File | ||
| } | ||
|
|
||
| // Spawn forks the sandboxed child, installs the policy, and releases it to | ||
|
|
@@ -838,10 +846,78 @@ func (s *Sandbox) Spawn(cmd ...string) (*Process, error) { | |
| return p, nil | ||
| } | ||
|
|
||
| // fdFile wraps an fd owned by the caller (handed back by sandlock_popen) as an | ||
| // *os.File that closes the fd when closed or finalized, or nil for the -1 | ||
| // sentinel of an inherit/null stream. | ||
| func fdFile(fd C.int, name string) *os.File { | ||
| if fd < 0 { | ||
| return nil | ||
| } | ||
| return os.NewFile(uintptr(fd), name) | ||
| } | ||
|
|
||
| // Popen forks the sandboxed child with per-stream stdio and returns a live | ||
| // Process without waiting — the streaming counterpart of Spawn. For each stream | ||
| // set to StdioPiped, the matching Process field (Stdin/Stdout/Stderr) is an | ||
| // *os.File the caller reads/writes while the child runs; inherit/null streams | ||
| // leave it nil. The zero Stdio inherits all three (identical to Spawn). | ||
| // | ||
| // Deadlock warning (as with os/exec pipes): a piped Stdin is yours — close 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 | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Kill-after-exit idempotency now differs across the three bindings. FFI |
||
| // streams. Wait closes a still-open piped Stdin for you to deliver EOF. | ||
| func (s *Sandbox) Popen(stdio Stdio, cmd ...string) (*Process, error) { | ||
| if len(cmd) == 0 { | ||
| return nil, fmt.Errorf("sandlock: empty command") | ||
| } | ||
| for _, m := range []StdioMode{stdio.Stdin, stdio.Stdout, stdio.Stderr} { | ||
| if m > StdioNull { | ||
| return nil, fmt.Errorf("sandlock: invalid StdioMode %d", uint32(m)) | ||
| } | ||
| } | ||
| policyPtr, err := s.buildPolicy() | ||
| if err != nil { | ||
| return nil, err | ||
| } | ||
| defer C.sandlock_sandbox_free(policyPtr) | ||
|
|
||
| argv, err := cArgv(cmd) | ||
| if err != nil { | ||
| return nil, err | ||
| } | ||
| defer freeArgv(argv) | ||
| ap, ac := argvPtr(argv) | ||
| name := s.cName() | ||
| defer freeName(name) | ||
|
|
||
| fdIn, fdOut, fdErr := C.int(-1), C.int(-1), C.int(-1) | ||
| h := C.sandlock_popen( | ||
| policyPtr, name, ap, ac, | ||
| C.uint32_t(stdio.Stdin), C.uint32_t(stdio.Stdout), C.uint32_t(stdio.Stderr), | ||
| &fdIn, &fdOut, &fdErr, | ||
| ) | ||
| if h == nil { | ||
| return nil, fmt.Errorf("sandlock: popen failed") | ||
| } | ||
| p := &Process{ | ||
| h: h, | ||
| pid: int(C.sandlock_handle_pid(h)), | ||
| Stdin: fdFile(fdIn, "sandlock-stdin"), | ||
| Stdout: fdFile(fdOut, "sandlock-stdout"), | ||
| Stderr: fdFile(fdErr, "sandlock-stderr"), | ||
| } | ||
| runtime.SetFinalizer(p, (*Process).finalize) | ||
| return p, nil | ||
| } | ||
|
|
||
| // finalize is the SetFinalizer cleanup for a Process abandoned without | ||
| // Wait/Close. It can only run once the Process is unreachable, which implies | ||
| // no Wait is in flight (a blocked Wait keeps the Process reachable), so the | ||
| // handle is not concurrently borrowed and is safe to free here. | ||
| // handle is not concurrently borrowed and is safe to free here. The piped | ||
| // streams are intentionally left alone: once the Process is unreachable so are | ||
| // they, and each *os.File's own finalizer closes its fd — no fd leaks. | ||
| func (p *Process) finalize() { | ||
| p.mu.Lock() | ||
| defer p.mu.Unlock() | ||
|
|
@@ -862,23 +938,39 @@ func (p *Process) Pid() int { | |
| return p.pid | ||
| } | ||
|
|
||
| // Wait blocks until the process exits, returns its captured Result, and | ||
| // releases the handle. After Wait the Process is no longer running. | ||
| // Wait blocks until the process exits, returns its Result, and releases the | ||
| // handle. After Wait the Process is no longer running. | ||
| // | ||
| // The Result carries the exit status only. Unlike Run, a Popen'd process sends | ||
| // any StdioPiped output to the caller through the Stdout/Stderr fields (and | ||
| // StdioInherit/StdioNull output to the inherited fd or /dev/null), so it is | ||
| // never captured into Result.Stdout/Result.Stderr — read piped bytes off | ||
| // p.Stdout/p.Stderr, not off the Result. | ||
| // | ||
| // The blocking native wait runs without holding the mutex so that Kill (and | ||
| // Pause/Resume), which signal the process group by PID, can run concurrently | ||
| // and interrupt it. The waiting flag reserves exclusive use of the handle for | ||
| // 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. | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Doc gap: nothing says the |
||
| func (p *Process) Wait() (*Result, error) { | ||
| p.mu.Lock() | ||
| if p.h == nil || p.waiting { | ||
| p.mu.Unlock() | ||
| return nil, ErrNotRunning | ||
| } | ||
| h := p.h | ||
| stdin := p.Stdin | ||
| p.waiting = true | ||
| p.mu.Unlock() | ||
|
|
||
| // Deliver EOF to a still-open piped stdin so a reader child (e.g. cat) can | ||
| // exit; otherwise the native wait blocks forever. Harmless if already closed. | ||
| if stdin != nil { | ||
| _ = stdin.Close() | ||
| } | ||
|
|
||
| r := C.sandlock_handle_wait(h) | ||
|
|
||
| p.mu.Lock() | ||
|
|
@@ -911,11 +1003,29 @@ func (p *Process) Pause() error { return p.signal(syscall.SIGSTOP) } | |
| // Resume sends SIGCONT to the sandbox process group. | ||
| func (p *Process) Resume() error { return p.signal(syscall.SIGCONT) } | ||
|
|
||
| // Kill sends SIGKILL to the sandbox process group. | ||
| // Kill sends SIGKILL to the sandbox process group. It is idempotent: a process | ||
| // that already exited (ESRCH) or was already reaped by Wait (ErrNotRunning) is | ||
| // not an error — killing it is a no-op success, matching the FFI | ||
| // (sandlock_handle_kill returns 0) and the Python binding (silent no-op after | ||
| // wait()). This keeps the "Kill from another goroutine" escape hatch race-free: | ||
| // a Kill that fires just as Wait reaps the child still returns nil. | ||
| func (p *Process) Kill() error { | ||
| err := p.signal(syscall.SIGKILL) | ||
| if err == syscall.ESRCH { | ||
| return nil | ||
| return nil // child already exited (still unreaped) — no-op | ||
| } | ||
| if err == ErrNotRunning { | ||
| // signal() returns ErrNotRunning for two causes: the handle was already | ||
| // reaped by Wait (h == nil) — an idempotent no-op — or a live handle with | ||
| // no pid. Only the reaped case is a success; a missing pid is a genuine | ||
| // error we must surface (and must never turn into killpg(-0), which would | ||
| // signal our own process group). | ||
| p.mu.Lock() | ||
| reaped := p.h == nil | ||
| p.mu.Unlock() | ||
| if reaped { | ||
| return nil | ||
| } | ||
| } | ||
| return err | ||
| } | ||
|
|
@@ -957,6 +1067,13 @@ func (p *Process) Ports() (map[int]int, error) { | |
| func (p *Process) Close() error { | ||
| p.mu.Lock() | ||
| defer p.mu.Unlock() | ||
| // Release caller-owned pipe fds regardless of handle state: a Close after | ||
| // Wait still needs to close the piped stdout/stderr the caller drained. | ||
| for _, f := range []*os.File{p.Stdin, p.Stdout, p.Stderr} { | ||
| if f != nil { | ||
| _ = f.Close() | ||
| } | ||
| } | ||
| if p.h == nil { | ||
| return nil | ||
| } | ||
|
|
||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Test gap: the zero-
Stdio-equals-Spawncontract is stated in three places (here, thePopendoc, the README) but never exercised.TestPopenNullStdoutNoStreamis the closest test. A cheapPopen(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.