Fix orphaned processes on Windows when quickly navigating between commits (#5885)

Fixes #5879: with an external diff command configured, quickly
navigating between commits on Windows accumulates orphaned
`git.exe`/`difft.exe`/`conhost.exe` processes that keep computing their
diffs in the background and persist after lazygit exits.

Stopping a pty task on Windows relied on `ClosePseudoConsole`, whose
`CTRL_CLOSE_EVENT` only reaches clients *attached to the pseudoconsole
at that moment*. Attachment happens asynchronously during child startup,
so a task stopped within the first few milliseconds of its life — which
is exactly what rapid navigation produces — misses the event entirely
and survives, together with its whole process tree (git for Windows
spawns through a two-level git.exe wrapper, so a single task has several
attach windows).

The fix puts the child into a job object before it executes its first
instruction (created suspended → assigned → resumed), so every
descendant is in the job from the start. The pty teardown still closes
the pseudoconsole first and gives clients that received the close event
a moment (500ms) to exit through their own handlers — git's cleans up
lock files — and then terminates whatever is left in the job.
`JOB_OBJECT_LIMIT_KILL_ON_JOB_CLOSE` doubles as a safety net: if lazygit
exits without running the teardown, the OS closes the job handle and
reaps the tree.

Validated with a harness that mimics lazygit's stop path with randomized
0–120ms stop delays: before the fix, 3 of 30 process trees survived as
permanent orphans per run; after it, three runs of 30 all came back with
zero orphans, with the job kill catching exactly the children that
missed the close event (6 of 90) while the rest still exited gracefully
through the event as before.

pkg/commands/oscommands/pty_unix.go

@@ -32,3 +32,9 @@ func StartPty(cmd *exec.Cmd, cols, rows uint16) (StartedPty, error) {
 		Wait:    cmd.Wait,
 	}, nil
 }
+
+// TerminateLivePtys is a no-op on Unix: stopping a pty task signals the
+// child (SIGTERM, plus SIGHUP to the foreground process group when the
+// master closes), and the processes clean themselves up without lazygit
+// having to wait for them.
+func TerminateLivePtys() {}

pkg/commands/oscommands/pty_windows.go

@@ -4,7 +4,9 @@ import (
 	"fmt"
 	"os"
 	"os/exec"
+	"strings"
 	"sync"
+	"time"
 	"unsafe"
 
 	"github.com/jesseduffield/lazygit/pkg/utils"
@@ -12,7 +14,14 @@ import (
 )
 
 type winPty struct {
-	hpc     windows.Handle
+	hpc windows.Handle
+	// job holds the child and every descendant it spawns; terminating it
+	// kills whatever is left of the process tree (see Close).
+	job windows.Handle
+	// conhost is a handle to the conhost.exe serving this pty, or 0 if it
+	// couldn't be identified. Held so that the teardown in Close can reap
+	// it on Windows builds whose conhost fails to run down on its own.
+	conhost windows.Handle
 	inWrite *os.File
 	outRead *os.File
 
@@ -64,6 +73,46 @@ func (p *winPty) closeHpc() {
 	windows.ClosePseudoConsole(p.hpc)
 }
 
+// How long Close waits for the conhost to run itself down after its clients
+// are gone, before concluding that it never will (see Close) and reaping it.
+const conhostExitTimeout = time.Second
+
+var (
+	// ptyTeardowns counts the in-flight teardown goroutines spawned by
+	// Close; TerminateLivePtys waits for them when lazygit exits.
+	ptyTeardowns sync.WaitGroup
+	// ptyQuit is closed by TerminateLivePtys. In-flight teardowns skip the
+	// conhost rundown wait once it is closed: the conhost serves nothing
+	// once its clients are gone, and the exit must not stall for its sake.
+	ptyQuit     = make(chan struct{})
+	ptyQuitOnce sync.Once
+)
+
+// TerminateLivePtys synchronously terminates the process trees and console
+// hosts of all ptys whose teardown hasn't finished yet. Call it when lazygit
+// is about to exit: the asynchronous teardowns in Close won't get to finish
+// (the conhost rundown wait outlives the process), and while
+// KILL_ON_JOB_CLOSE reaps the clients when the job handles are closed at
+// process death, nothing would reap the conhosts on the Windows builds that
+// need it (see Close). A long diff on screen keeps its git process running
+// the whole time it is shown, so quitting with such a teardown in flight is
+// the rule, not the exception.
+func TerminateLivePtys() {
+	ptyQuitOnce.Do(func() { close(ptyQuit) })
+
+	done := make(chan struct{})
+	go utils.Safe(func() {
+		ptyTeardowns.Wait()
+		close(done)
+	})
+	select {
+	case <-done:
+	case <-time.After(2 * time.Second):
+		// Don't hold up the exit any longer; the job handles' rundown
+		// still covers the clients.
+	}
+}
+
 // Close tears the pty down without waiting for it: the teardown runs on a
 // background goroutine and Close returns immediately.
 //
@@ -83,11 +132,65 @@ func (p *winPty) closeHpc() {
 // nobody is reading anymore, so that flush can only complete once the pipe
 // is broken. The background waiter's closeHpc may already be wedged in such
 // a flush while holding p.mu; closing the pipes is what unblocks it.
+//
+// Closing the pseudoconsole delivers CTRL_CLOSE_EVENT only to the clients
+// attached to it at that moment. A child that is stopped right after being
+// spawned is still starting up and not attached yet, so the event misses it
+// and it survives, running its command to completion as an orphan — and
+// keeping its console host alive with it (#5879); the same holds for
+// grandchildren spawned while the console is going down, and for clients
+// that ignore the event (the Windows flavor of #5675). The job kill reaps
+// all of those. There is no point in delaying it: the close event is not a
+// graceful signal worth waiting on — git and the common diff tools leave it
+// to the default handler, which calls ExitProcess at whatever instruction
+// the process happens to execute — so clients that got the event are
+// already dying. Killing at an arbitrary point cannot leak a stale
+// index.lock, because pty-rendered commands don't take that lock (see
+// withPtyGitConfig in pkg/gui/pty.go).
+//
+// The pseudoconsole close gets its own goroutine because the kill must not
+// wait for it: on builds where ClosePseudoConsole blocks until the console
+// host exits (pre-24H2), the host keeps running as long as a surviving
+// client does, and that client only goes away through the job kill —
+// sequencing the kill after a blocking close would thus deadlock in
+// exactly the case the kill exists for.
+//
+// After the kill, the conhost serving the pty is reaped as well if it
+// doesn't exit by itself: a healthy conhost runs down once the reference
+// handle is closed and its clients are gone, but conhost builds before
+// Windows 11 24H2 fail to complete the rundown when a client attached
+// after the close event was delivered and was then killed — the fate of
+// exactly the clients the job kill is for — and such a conhost sits
+// around forever, serving nothing (#5879). The reap is inert on healthy
+// builds: the wait succeeds and only the handle is closed.
+//
+// When lazygit is quitting, the conhost rundown wait is skipped; see
+// TerminateLivePtys.
 func (p *winPty) Close() error {
+	ptyTeardowns.Add(1)
 	go utils.Safe(func() {
+		defer ptyTeardowns.Done()
+
 		p.inWrite.Close()
 		p.outRead.Close()
-		p.closeHpc()
+		go utils.Safe(p.closeHpc)
+
+		_ = windows.TerminateJobObject(p.job, 1)
+		_ = windows.CloseHandle(p.job)
+
+		if p.conhost != 0 {
+			timeout := conhostExitTimeout
+			select {
+			case <-ptyQuit:
+				timeout = 0
+			default:
+			}
+			event, err := windows.WaitForSingleObject(p.conhost, uint32(timeout/time.Millisecond))
+			if err != nil || event != windows.WAIT_OBJECT_0 {
+				_ = windows.TerminateProcess(p.conhost, 1)
+			}
+			_ = windows.CloseHandle(p.conhost)
+		}
 	})
 	return nil
 }
@@ -123,6 +226,52 @@ func startWaiter(proc *os.Process, p *winPty) func() error {
 	}
 }
 
+// conhostScanMu serializes CreatePseudoConsole and the child-process scans
+// around it, so that two concurrently starting ptys can't make each other's
+// "which conhost is new" diff ambiguous.
+var conhostScanMu sync.Mutex
+
+// conhostChildren returns the pids of all conhost.exe processes that are
+// direct children of this process. Errors just yield a smaller (possibly
+// empty) set; the caller treats identification as best-effort.
+func conhostChildren() map[uint32]bool {
+	pids := map[uint32]bool{}
+	snap, err := windows.CreateToolhelp32Snapshot(windows.TH32CS_SNAPPROCESS, 0)
+	if err != nil {
+		return pids
+	}
+	defer func() { _ = windows.CloseHandle(snap) }()
+	me := uint32(os.Getpid())
+	var pe windows.ProcessEntry32
+	pe.Size = uint32(unsafe.Sizeof(pe))
+	for err := windows.Process32First(snap, &pe); err == nil; err = windows.Process32Next(snap, &pe) {
+		if pe.ParentProcessID == me && strings.EqualFold(windows.UTF16ToString(pe.ExeFile[:]), "conhost.exe") {
+			pids[pe.ProcessID] = true
+		}
+	}
+	return pids
+}
+
+// openNewConhostChild returns a handle to the single conhost child that
+// appeared since the before scan, or 0 if there isn't exactly one candidate
+// or it can't be opened.
+func openNewConhostChild(before map[uint32]bool) windows.Handle {
+	var found []uint32
+	for pid := range conhostChildren() {
+		if !before[pid] {
+			found = append(found, pid)
+		}
+	}
+	if len(found) != 1 {
+		return 0
+	}
+	h, err := windows.OpenProcess(windows.SYNCHRONIZE|windows.PROCESS_TERMINATE, false, found[0])
+	if err != nil {
+		return 0
+	}
+	return h
+}
+
 func StartPty(cmd *exec.Cmd, cols, rows uint16) (sp StartedPty, err error) {
 	// Two pipes: one for the child's stdin (we never write to it, but ConPTY
 	// needs a handle), one for the child's stdout/stderr multiplexed through
@@ -148,9 +297,24 @@ func StartPty(cmd *exec.Cmd, cols, rows uint16) (sp StartedPty, err error) {
 
 	// CreatePseudoConsole dupes the handles it needs internally; we release
 	// our references to the child-side ends immediately after.
-	var hpc windows.Handle
+	//
+	// It also spawns the conhost.exe serving the console session, as a
+	// direct child of this process. The teardown in Close needs a handle to
+	// that conhost (see there), but Windows offers no way to obtain one
+	// from the HPCON, so identify it by diffing our conhost children around
+	// the call. Open a real handle right away so that pid reuse can't later
+	// misdirect the teardown's reap. If identification fails, the handle
+	// stays 0 and the teardown skips the reap.
+	var hpc, conhost windows.Handle
 	size := clampPtySize(cols, rows)
-	if err = windows.CreatePseudoConsole(size, inRead, outWrite, 0, &hpc); err != nil {
+	conhostScanMu.Lock()
+	conhostsBefore := conhostChildren()
+	err = windows.CreatePseudoConsole(size, inRead, outWrite, 0, &hpc)
+	if err == nil {
+		conhost = openNewConhostChild(conhostsBefore)
+	}
+	conhostScanMu.Unlock()
+	if err != nil {
 		_ = windows.CloseHandle(inRead)
 		_ = windows.CloseHandle(outWrite)
 		return StartedPty{}, fmt.Errorf("CreatePseudoConsole: %w", err)
@@ -160,9 +324,40 @@ func StartPty(cmd *exec.Cmd, cols, rows uint16) (sp StartedPty, err error) {
 	defer func() {
 		if err != nil {
 			windows.ClosePseudoConsole(hpc)
+			if conhost != 0 {
+				_ = windows.CloseHandle(conhost)
+			}
 		}
 	}()
 
+	// The child goes into a job object so that the teardown in Close can
+	// terminate the whole process tree. KILL_ON_JOB_CLOSE makes the OS do
+	// that when the last handle to the job is closed, which doubles as a
+	// safety net: if lazygit exits without running the teardown, the handle
+	// is closed for it and the tree is reaped.
+	job, err := windows.CreateJobObject(nil, nil)
+	if err != nil {
+		return StartedPty{}, fmt.Errorf("CreateJobObject: %w", err)
+	}
+	defer func() {
+		if err != nil {
+			// Kills the child on error paths where it was already assigned
+			// to the job; plain handle cleanup before that.
+			_ = windows.CloseHandle(job)
+		}
+	}()
+	limits := windows.JOBOBJECT_EXTENDED_LIMIT_INFORMATION{
+		BasicLimitInformation: windows.JOBOBJECT_BASIC_LIMIT_INFORMATION{
+			LimitFlags: windows.JOB_OBJECT_LIMIT_KILL_ON_JOB_CLOSE,
+		},
+	}
+	if _, err = windows.SetInformationJobObject(
+		job, windows.JobObjectExtendedLimitInformation,
+		uintptr(unsafe.Pointer(&limits)), uint32(unsafe.Sizeof(limits)),
+	); err != nil {
+		return StartedPty{}, fmt.Errorf("SetInformationJobObject: %w", err)
+	}
+
 	// Attach the pseudoconsole to the child via a process attribute list.
 	attrList, err := windows.NewProcThreadAttributeList(1)
 	if err != nil {
@@ -221,7 +416,7 @@ func StartPty(cmd *exec.Cmd, cols, rows uint16) (sp StartedPty, err error) {
 		nil, // process security
 		nil, // thread security
 		false,
-		windows.EXTENDED_STARTUPINFO_PRESENT|windows.CREATE_UNICODE_ENVIRONMENT,
+		windows.EXTENDED_STARTUPINFO_PRESENT|windows.CREATE_UNICODE_ENVIRONMENT|windows.CREATE_SUSPENDED,
 		envPtr,
 		dirPtr,
 		&si.StartupInfo,
@@ -230,6 +425,22 @@ func StartPty(cmd *exec.Cmd, cols, rows uint16) (sp StartedPty, err error) {
 	if err != nil {
 		return StartedPty{}, fmt.Errorf("CreateProcess: %w", err)
 	}
+
+	// The child was created suspended so that it can be assigned to the job
+	// before it runs its first instruction; that way every descendant it
+	// ever spawns is in the job from the start.
+	if err = windows.AssignProcessToJobObject(job, pi.Process); err != nil {
+		// Not in the job yet, so the deferred job-handle close can't reap it.
+		_ = windows.TerminateProcess(pi.Process, 1)
+		_ = windows.CloseHandle(pi.Thread)
+		_ = windows.CloseHandle(pi.Process)
+		return StartedPty{}, fmt.Errorf("AssignProcessToJobObject: %w", err)
+	}
+	if _, err = windows.ResumeThread(pi.Thread); err != nil {
+		_ = windows.CloseHandle(pi.Thread)
+		_ = windows.CloseHandle(pi.Process)
+		return StartedPty{}, fmt.Errorf("ResumeThread: %w", err)
+	}
 	_ = windows.CloseHandle(pi.Thread)
 
 	// Re-open the process by PID to get an *os.Process to wait on. Do this
@@ -245,6 +456,8 @@ func StartPty(cmd *exec.Cmd, cols, rows uint16) (sp StartedPty, err error) {
 
 	wp := &winPty{
 		hpc:     hpc,
+		job:     job,
+		conhost: conhost,
 		inWrite: os.NewFile(uintptr(inWrite), "conpty-in"),
 		outRead: os.NewFile(uintptr(outRead), "conpty-out"),
 	}

pkg/commands/oscommands/pty_windows_test.go

@@ -3,6 +3,7 @@ package oscommands
 import (
 	"os/exec"
 	"testing"
+	"time"
 
 	"github.com/stretchr/testify/assert"
 )
@@ -23,3 +24,82 @@ func TestStartPtyWithZeroSize(t *testing.T) {
 		_ = 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")
+	}
+}

pkg/gui/gui.go

@@ -1005,6 +1005,12 @@ func (gui *Gui) RunAndHandleError(startArgs appTypes.StartArgs) error {
 				manager.Close()
 			}
 
+			// The pty teardowns spawned by the manager closes above run on
+			// background goroutines that won't get to finish before the
+			// process exits; reap their process trees synchronously instead
+			// so that they don't outlive lazygit.
+			oscommands.TerminateLivePtys()
+
 			close(gui.stopChan)
 
 			if errors.Is(err, gocui.ErrQuit) {

pkg/gui/pty.go

@@ -49,9 +49,9 @@ type ptyCmd struct {
 	wait    func() error
 }
 
-func (p ptyCmd) Wait() error             { return p.wait() }
-func (p ptyCmd) String() string          { return p.cmd.String() }
-func (p ptyCmd) GetProcess() *os.Process { return p.process }
+func (p ptyCmd) Wait() error      { return p.wait() }
+func (p ptyCmd) String() string   { return p.cmd.String() }
+func (p ptyCmd) Terminate() error { return oscommands.TerminateProcessGracefully(p.process) }
 
 // Some commands need to output for a terminal to active certain behaviour.
 // For example, git won't invoke the GIT_PAGER env var unless it thinks it's

pkg/tasks/tasks.go

@@ -4,7 +4,6 @@ import (
 	"bufio"
 	"fmt"
 	"io"
-	"os"
 	"os/exec"
 	"sync"
 	"sync/atomic"
@@ -24,7 +23,9 @@ import (
 type Cmd interface {
 	Wait() error
 	String() string
-	GetProcess() *os.Process
+	// Terminate makes the process stop early, as gracefully as the platform
+	// allows. It doesn't wait for the process to exit.
+	Terminate() error
 }
 
 // ExecCmd adapts *exec.Cmd to Cmd.
@@ -32,8 +33,11 @@ type ExecCmd struct {
 	*exec.Cmd
 }
 
-func (c ExecCmd) GetProcess() *os.Process {
-	return c.Process
+// Terminate sends SIGTERM on Unix. On Windows it does nothing, so a stopped
+// command keeps running until it next writes to its (by then closed) output
+// pipe.
+func (c ExecCmd) Terminate() error {
+	return oscommands.TerminateProcessGracefully(c.Process)
 }
 
 // This file revolves around running commands that will be output to the main panel
@@ -213,10 +217,7 @@ func (self *ViewBufferManager) NewCmdTask(start func() (Cmd, io.Reader), prefix
 				// when flicking through several very long diffs when diff.algorithm = histogram is
 				// being used, in which case multiple git processes continue to calculate expensive
 				// diffs in the background even though they have been stopped already.
-				//
-				// Unfortunately this will do nothing on Windows, so Windows users will have to live
-				// with the higher CPU usage.
-				if err := oscommands.TerminateProcessGracefully(cmd.GetProcess()); err != nil {
+				if err := cmd.Terminate(); err != nil {
 					self.Log.Errorf("error when trying to terminate cmd task: %v; Command: %v", err, cmd.String())
 				}