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.
This commit is contained in:
@@ -6,13 +6,22 @@ import (
|
|||||||
"io"
|
"io"
|
||||||
"os"
|
"os"
|
||||||
"os/exec"
|
"os/exec"
|
||||||
|
"sync"
|
||||||
"syscall"
|
"syscall"
|
||||||
|
|
||||||
cpty "github.com/creack/pty"
|
cpty "github.com/creack/pty"
|
||||||
)
|
)
|
||||||
|
|
||||||
// PTY holds a child process attached to a pseudo-terminal master fd.
|
// 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 {
|
type PTY struct {
|
||||||
|
mu sync.Mutex
|
||||||
master *os.File
|
master *os.File
|
||||||
cmd *exec.Cmd
|
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) {
|
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 0, io.ErrClosedPipe
|
||||||
}
|
}
|
||||||
return p.master.Read(b)
|
return m.Read(b)
|
||||||
}
|
}
|
||||||
|
|
||||||
func (p *PTY) Write(b []byte) (int, error) {
|
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 0, io.ErrClosedPipe
|
||||||
}
|
}
|
||||||
return p.master.Write(b)
|
return m.Write(b)
|
||||||
}
|
}
|
||||||
|
|
||||||
func (p *PTY) Resize(cols, rows uint16) error {
|
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 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.
|
// 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.
|
// Close terminates the child (best effort) and releases the master fd.
|
||||||
func (p *PTY) Close() error {
|
func (p *PTY) Close() error {
|
||||||
|
p.mu.Lock()
|
||||||
|
m := p.master
|
||||||
|
p.master = nil
|
||||||
|
p.mu.Unlock()
|
||||||
var firstErr error
|
var firstErr error
|
||||||
if p.master != nil {
|
if m != nil {
|
||||||
if err := p.master.Close(); err != nil && firstErr == nil {
|
if err := m.Close(); err != nil {
|
||||||
firstErr = err
|
firstErr = err
|
||||||
}
|
}
|
||||||
p.master = nil
|
|
||||||
}
|
}
|
||||||
if p.cmd != nil && p.cmd.Process != nil {
|
if p.cmd != nil && p.cmd.Process != nil {
|
||||||
pid := p.cmd.Process.Pid
|
pid := p.cmd.Process.Pid
|
||||||
|
|||||||
Reference in New Issue
Block a user