From 63986e7e008725cd28cc771a5541b99a13124805 Mon Sep 17 00:00:00 2001 From: Harry Bayliss Date: Wed, 27 May 2026 14:35:33 +0100 Subject: [PATCH] Fix data race on PTY master between Read and Close pumpChild's PTY.Read raced Session.Shutdown's PTY.Close on the master field (Close set it nil while Read read it). Benign at process exit on main, but the daemon now runs Shutdown routinely (daemon stop). Guard the field with a mutex, capturing the fd under the lock and doing the blocking I/O outside it so Close still unblocks an in-flight Read. Caught under: go test -race -run 'Daemon|NetClient|Owner' -count=5. --- internal/pty/pty.go | 39 ++++++++++++++++++++++++++++++--------- 1 file changed, 30 insertions(+), 9 deletions(-) diff --git a/internal/pty/pty.go b/internal/pty/pty.go index 925a6b0..f18c696 100644 --- a/internal/pty/pty.go +++ b/internal/pty/pty.go @@ -6,13 +6,22 @@ import ( "io" "os" "os/exec" + "sync" "syscall" cpty "github.com/creack/pty" ) // PTY holds a child process attached to a pseudo-terminal master fd. +// +// mu guards the master field only. Read/Write/Resize capture the *os.File +// under the lock and then do the (potentially blocking) I/O without holding +// it, so Close can swap master to nil and close the fd concurrently — closing +// the captured *os.File unblocks an in-flight Read. This avoids a data race +// between pumpChild's Read and Session.Shutdown's Close, which the daemon now +// hits routinely (daemon stop, not just process exit). type PTY struct { + mu sync.Mutex master *os.File cmd *exec.Cmd } @@ -45,24 +54,33 @@ func Start(argv []string, env []string, workDir string, cols, rows uint16) (*PTY } func (p *PTY) Read(b []byte) (int, error) { - if p.master == nil { + p.mu.Lock() + m := p.master + p.mu.Unlock() + if m == nil { return 0, io.ErrClosedPipe } - return p.master.Read(b) + return m.Read(b) } func (p *PTY) Write(b []byte) (int, error) { - if p.master == nil { + p.mu.Lock() + m := p.master + p.mu.Unlock() + if m == nil { return 0, io.ErrClosedPipe } - return p.master.Write(b) + return m.Write(b) } func (p *PTY) Resize(cols, rows uint16) error { - if p.master == nil { + p.mu.Lock() + m := p.master + p.mu.Unlock() + if m == nil { return io.ErrClosedPipe } - return cpty.Setsize(p.master, &cpty.Winsize{Cols: cols, Rows: rows}) + return cpty.Setsize(m, &cpty.Winsize{Cols: cols, Rows: rows}) } // Wait blocks until the child exits and returns its exit error if any. @@ -83,12 +101,15 @@ func (p *PTY) Pid() int { // Close terminates the child (best effort) and releases the master fd. func (p *PTY) Close() error { + p.mu.Lock() + m := p.master + p.master = nil + p.mu.Unlock() var firstErr error - if p.master != nil { - if err := p.master.Close(); err != nil && firstErr == nil { + if m != nil { + if err := m.Close(); err != nil { firstErr = err } - p.master = nil } if p.cmd != nil && p.cmd.Process != nil { pid := p.cmd.Process.Pid