diff --git a/backend/internal/terminal/pty_unix.go b/backend/internal/terminal/pty_unix.go index 4849215be..e5ca6f34b 100644 --- a/backend/internal/terminal/pty_unix.go +++ b/backend/internal/terminal/pty_unix.go @@ -7,6 +7,7 @@ import ( "errors" "os" "os/exec" + "sync" "github.com/creack/pty" ) @@ -27,8 +28,10 @@ func defaultSpawn(ctx context.Context, argv []string) (ptyProcess, error) { } type creackPTY struct { - f *os.File - cmd *exec.Cmd + f *os.File + cmd *exec.Cmd + closeOnce sync.Once + closeErr error } func (p *creackPTY) Read(b []byte) (int, error) { return p.f.Read(b) } @@ -42,11 +45,18 @@ func (p *creackPTY) Wait() error { return p.cmd.Wait() } // Close stops the attach process and releases the PTY. tmux attach exits cleanly // when the master closes, but kill the process to be sure it does not linger. +// +// It is idempotent: both the session run loop (after copyOut returns) and +// session.close (via Manager.Close) call Close on the same PTY, and cmd.Wait +// must run exactly once. A second concurrent Wait on the same process blocks +// forever, deadlocking daemon shutdown when a terminal is still attached. func (p *creackPTY) Close() error { - closeErr := p.f.Close() - if p.cmd.Process != nil { - _ = p.cmd.Process.Kill() - } - _ = p.cmd.Wait() - return closeErr + p.closeOnce.Do(func() { + p.closeErr = p.f.Close() + if p.cmd.Process != nil { + _ = p.cmd.Process.Kill() + } + _ = p.cmd.Wait() + }) + return p.closeErr } diff --git a/backend/internal/terminal/pty_unix_test.go b/backend/internal/terminal/pty_unix_test.go new file mode 100644 index 000000000..7d8c04ff5 --- /dev/null +++ b/backend/internal/terminal/pty_unix_test.go @@ -0,0 +1,33 @@ +//go:build !windows + +package terminal + +import ( + "context" + "testing" + "time" +) + +// TestCreackPTYCloseIsIdempotent guards the shutdown deadlock: the session run +// loop and session.close both call Close on the same PTY, so cmd.Wait must run +// exactly once. Without the sync.Once a second Wait blocks forever, so this test +// would hang (caught by the watchdog) rather than fail. +func TestCreackPTYCloseIsIdempotent(t *testing.T) { + p, err := defaultSpawn(context.Background(), []string{"/bin/sh", "-c", "sleep 30"}) + if err != nil { + t.Fatalf("spawn: %v", err) + } + + done := make(chan struct{}) + go func() { + _ = p.Close() + _ = p.Close() // second close must not block on a second cmd.Wait + close(done) + }() + + select { + case <-done: + case <-time.After(5 * time.Second): + t.Fatal("creackPTY.Close did not return: double Close deadlocked on cmd.Wait") + } +}