From b72a32bbc6f19223da7993a38db6f7c28ff6aaf5 Mon Sep 17 00:00:00 2001 From: Harry Bayliss Date: Wed, 27 May 2026 13:19:35 +0100 Subject: [PATCH] Fix PTY workdir and process group teardown --- internal/app/child.go | 2 +- internal/pty/pty.go | 9 ++++- internal/pty/pty_test.go | 84 ++++++++++++++++++++++++++++++++++++++++ 3 files changed, 93 insertions(+), 2 deletions(-) create mode 100644 internal/pty/pty_test.go diff --git a/internal/app/child.go b/internal/app/child.go index 91f96f4..300c965 100644 --- a/internal/app/child.go +++ b/internal/app/child.go @@ -228,7 +228,7 @@ func (c *Child) startPTY(cols, rows uint16) (uint64, error) { } starting := StatusStarting c.status.Store(&starting) - p, err := pkgpty.Start(c.Argv, c.Env, cols, rows) + p, err := pkgpty.Start(c.Argv, c.Env, c.WorkDir, cols, rows) if err != nil { em.Close() errored := StatusErrored diff --git a/internal/pty/pty.go b/internal/pty/pty.go index 4f8beca..925a6b0 100644 --- a/internal/pty/pty.go +++ b/internal/pty/pty.go @@ -6,6 +6,7 @@ import ( "io" "os" "os/exec" + "syscall" cpty "github.com/creack/pty" ) @@ -19,11 +20,13 @@ type PTY struct { // Start spawns argv with stdin/stdout/stderr attached to a new PTY sized // (cols, rows). The returned PTY exposes the master fd for the parent to // read from and write to. -func Start(argv []string, env []string, cols, rows uint16) (*PTY, error) { +func Start(argv []string, env []string, workDir string, cols, rows uint16) (*PTY, error) { if len(argv) == 0 { return nil, fmt.Errorf("pty: empty argv") } cmd := exec.Command(argv[0], argv[1:]...) + cmd.Dir = workDir + cmd.SysProcAttr = &syscall.SysProcAttr{Setsid: true, Setctty: true} if env != nil { cmd.Env = ensureTerm(env) } else { @@ -88,6 +91,10 @@ func (p *PTY) Close() error { p.master = nil } if p.cmd != nil && p.cmd.Process != nil { + pid := p.cmd.Process.Pid + if pid > 0 { + _ = syscall.Kill(-pid, syscall.SIGKILL) + } _ = p.cmd.Process.Kill() } return firstErr diff --git a/internal/pty/pty_test.go b/internal/pty/pty_test.go new file mode 100644 index 0000000..1aa9d94 --- /dev/null +++ b/internal/pty/pty_test.go @@ -0,0 +1,84 @@ +package pty + +import ( + "bytes" + "errors" + "os" + "path/filepath" + "strconv" + "strings" + "syscall" + "testing" + "time" +) + +func TestStartUsesWorkDir(t *testing.T) { + dir := t.TempDir() + p, err := Start([]string{"sh", "-c", "pwd"}, nil, dir, 80, 24) + if err != nil { + t.Fatalf("Start: %v", err) + } + defer p.Close() + + var out bytes.Buffer + buf := make([]byte, 256) + deadline := time.Now().Add(5 * time.Second) + for time.Now().Before(deadline) { + n, err := p.Read(buf) + if n > 0 { + out.Write(buf[:n]) + if strings.Contains(out.String(), dir) { + break + } + } + if err != nil { + break + } + } + _ = p.Wait() + + if got := strings.TrimSpace(out.String()); got != dir { + t.Fatalf("pwd output = %q, want %q", got, dir) + } +} + +func TestCloseKillsProcessGroup(t *testing.T) { + dir := t.TempDir() + pidFile := filepath.Join(dir, "sleep.pid") + env := append(os.Environ(), "PIDFILE="+pidFile) + p, err := Start([]string{"sh", "-c", "sleep 30 & echo $! > \"$PIDFILE\"; wait"}, env, "", 80, 24) + if err != nil { + t.Fatalf("Start: %v", err) + } + deadline := time.Now().Add(5 * time.Second) + var childPID int + for time.Now().Before(deadline) { + b, err := os.ReadFile(pidFile) + if err == nil { + childPID, _ = strconv.Atoi(strings.TrimSpace(string(b))) + if childPID > 0 { + break + } + } + time.Sleep(20 * time.Millisecond) + } + if childPID <= 0 { + _ = p.Close() + t.Fatalf("background child pid was not written") + } + + if err := p.Close(); err != nil { + t.Fatalf("Close: %v", err) + } + _ = p.Wait() + + deadline = time.Now().Add(5 * time.Second) + for time.Now().Before(deadline) { + err := syscall.Kill(childPID, 0) + if errors.Is(err, syscall.ESRCH) { + return + } + time.Sleep(20 * time.Millisecond) + } + t.Fatalf("background child pid %d still exists after PTY.Close", childPID) +}