mirror of
https://github.com/jesseduffield/lazygit.git
synced 2026-09-10 07:36:27 -04:00
d251faddec
The pty teardown in Close runs on a background goroutine that doesn't get to finish when lazygit is quitting: the process exits milliseconds after the view buffer managers are closed. The job objects still cover the clients -- KILL_ON_JOB_CLOSE reaps them when the process's handles are rundown at exit -- but nothing reaps the conhost, so on Windows builds whose conhost fails to run down on its own, quitting leaks one conhost per live pty. This is not a rare timing window: a diff longer than what has been read keeps its git process (and thus its pty and conhost) running for the entire time it is displayed, so that scrolling can read more. Quitting while looking at a long diff is therefore the common case, and with an external differ configured it leaks a conhost on affected builds on almost every quit. Fix this by having the gui's shutdown path wait synchronously for the in-flight teardowns after closing the view buffer managers. A quit signal makes the teardowns skip the conhost rundown wait -- the conhost serves nothing once its clients are dead, and the exit must not stall for its sake -- so the wait normally completes in milliseconds, keeping quit as fast as before; a 2-second cap protects the exit path even if a teardown wedges. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
106 lines
3.3 KiB
Go
106 lines
3.3 KiB
Go
package oscommands
|
|
|
|
import (
|
|
"os/exec"
|
|
"testing"
|
|
"time"
|
|
|
|
"github.com/stretchr/testify/assert"
|
|
)
|
|
|
|
// The requested size can legitimately be zero: the pty inherits the main
|
|
// view's dimensions, and that view is zero-sized while hidden, e.g. in
|
|
// full-screen mode with a side panel focused.
|
|
func TestStartPtyWithZeroSize(t *testing.T) {
|
|
// The command deliberately produces no output: go test runs with
|
|
// redirected std handles, which CreateProcess duplicates into the child
|
|
// in place of handles to the attached pseudoconsole, so command output
|
|
// would bypass the pty and pollute the test log.
|
|
sp, err := StartPty(exec.Command("cmd", "/c", "exit 0"), 0, 0)
|
|
assert.NoError(t, err)
|
|
|
|
if err == nil {
|
|
_ = sp.Wait()
|
|
_ = sp.Pty.Close()
|
|
}
|
|
}
|
|
|
|
// StartPty must identify the conhost.exe that CreatePseudoConsole spawned to
|
|
// serve the pty: the teardown in Close reaps it on Windows builds whose
|
|
// conhost fails to run down on its own, and a failed identification silently
|
|
// degrades to not reaping. If this fails, the child-scan in
|
|
// openNewConhostChild no longer matches how Windows hosts pseudoconsoles.
|
|
func TestStartPtyIdentifiesConhost(t *testing.T) {
|
|
sp, err := StartPty(exec.Command("cmd", "/c", "exit 0"), 80, 24)
|
|
assert.NoError(t, err)
|
|
if err != nil {
|
|
return
|
|
}
|
|
|
|
assert.NotZero(t, sp.Pty.(*winPty).conhost)
|
|
|
|
_ = sp.Wait()
|
|
_ = sp.Pty.Close()
|
|
}
|
|
|
|
// TerminateLivePtys must reap a still-running pty synchronously: it runs
|
|
// when lazygit is about to exit, where the asynchronous teardown would not
|
|
// get to finish. Note that it switches the package's pty teardowns into
|
|
// quit mode for the remainder of the test binary's lifetime; that's fine
|
|
// for the other tests here, which must hold in either mode (quit mode only
|
|
// shortens the teardown's conhost rundown wait).
|
|
func TestTerminateLivePtysReapsRunningPty(t *testing.T) {
|
|
// The output redirect is there for the reason described in
|
|
// TestStartPtyWithZeroSize.
|
|
sp, err := StartPty(exec.Command("cmd", "/c", "ping -n 30 127.0.0.1 >nul"), 80, 24)
|
|
assert.NoError(t, err)
|
|
if err != nil {
|
|
return
|
|
}
|
|
|
|
_ = sp.Pty.Close()
|
|
TerminateLivePtys()
|
|
|
|
// The teardown has completed as part of TerminateLivePtys, so the child
|
|
// must be gone already; the timeout is generosity, not a grace period.
|
|
exited := make(chan struct{})
|
|
go func() {
|
|
_ = sp.Wait()
|
|
close(exited)
|
|
}()
|
|
select {
|
|
case <-exited:
|
|
case <-time.After(time.Second):
|
|
t.Fatal("child process was not terminated by TerminateLivePtys")
|
|
}
|
|
}
|
|
|
|
// Closing the pty must terminate the process tree it was running, even when
|
|
// it is closed so soon after starting that the child hasn't attached to the
|
|
// pseudoconsole yet: such a child misses the CTRL_CLOSE_EVENT that the close
|
|
// delivers to attached clients, and only the job-object kill reaps it.
|
|
// Without the kill, cmd and its ping child keep running for ~30 seconds and
|
|
// the Wait here times out.
|
|
func TestClosePtyTerminatesChildProcessTree(t *testing.T) {
|
|
// The output redirect is there for the reason described in
|
|
// TestStartPtyWithZeroSize.
|
|
sp, err := StartPty(exec.Command("cmd", "/c", "ping -n 30 127.0.0.1 >nul"), 80, 24)
|
|
assert.NoError(t, err)
|
|
if err != nil {
|
|
return
|
|
}
|
|
|
|
_ = sp.Pty.Close()
|
|
|
|
exited := make(chan struct{})
|
|
go func() {
|
|
_ = sp.Wait()
|
|
close(exited)
|
|
}()
|
|
select {
|
|
case <-exited:
|
|
case <-time.After(5 * time.Second):
|
|
t.Fatal("child process was not terminated by closing the pty")
|
|
}
|
|
}
|