From e90daaf81287b4ab5d53166d69651d9aba75ec9e Mon Sep 17 00:00:00 2001 From: Stefan Haller Date: Fri, 10 Jul 2026 08:56:06 +0200 Subject: [PATCH 01/12] Unify the git command lock-retry loops RunWithOutput and RunWithOutputs each carried their own near-identical copy of the index.lock retry loop. Extract the loop into a single retryOnLockError helper so the retry policy lives in one place, ahead of changing that policy. Behavior is unchanged; the added tests characterize it (success and non-lock errors run once, a lock error in the output is retried). Co-Authored-By: Claude Opus 4.8 (1M context) --- pkg/commands/git_cmd_obj_runner.go | 42 +++++------ pkg/commands/git_cmd_obj_runner_test.go | 92 +++++++++++++++++++++++++ 2 files changed, 113 insertions(+), 21 deletions(-) create mode 100644 pkg/commands/git_cmd_obj_runner_test.go diff --git a/pkg/commands/git_cmd_obj_runner.go b/pkg/commands/git_cmd_obj_runner.go index 668feef93ff..a72565b5cfb 100644 --- a/pkg/commands/git_cmd_obj_runner.go +++ b/pkg/commands/git_cmd_obj_runner.go @@ -33,33 +33,33 @@ func (self *gitCmdObjRunner) Run(cmdObj *oscommands.CmdObj) error { } func (self *gitCmdObjRunner) RunWithOutput(cmdObj *oscommands.CmdObj) (string, error) { - var output string - var err error - for range RetryCount { - newCmdObj := cmdObj.Clone() - output, err = self.innerRunner.RunWithOutput(newCmdObj) - - if err == nil || !isRetryableError(output) { - return output, err - } - - // if we have an error based on a lock, we should wait a bit and then retry - self.log.Warn("lock error prevented command from running. Retrying command after a small wait") - time.Sleep(WaitTime) - } - - return output, err + return self.retryOnLockError(func() (string, error) { + return self.innerRunner.RunWithOutput(cmdObj.Clone()) + }) } func (self *gitCmdObjRunner) RunWithOutputs(cmdObj *oscommands.CmdObj) (string, string, error) { var stdout, stderr string + _, err := self.retryOnLockError(func() (string, error) { + var runErr error + stdout, stderr, runErr = self.innerRunner.RunWithOutputs(cmdObj.Clone()) + return stdout + stderr, runErr + }) + return stdout, stderr, err +} + +// retryOnLockError runs the given function, retrying if it fails with a +// transient lock error (see isRetryableError). The string returned by run is +// the command output we inspect to classify the failure. We clone the command +// for each attempt (inside run) because an *exec.Cmd can only be run once. +func (self *gitCmdObjRunner) retryOnLockError(run func() (string, error)) (string, error) { + var output string var err error for range RetryCount { - newCmdObj := cmdObj.Clone() - stdout, stderr, err = self.innerRunner.RunWithOutputs(newCmdObj) + output, err = run() - if err == nil || !isRetryableError(stdout+stderr) { - return stdout, stderr, err + if err == nil || !isRetryableError(output) { + return output, err } // if we have an error based on a lock, we should wait a bit and then retry @@ -67,7 +67,7 @@ func (self *gitCmdObjRunner) RunWithOutputs(cmdObj *oscommands.CmdObj) (string, time.Sleep(WaitTime) } - return stdout, stderr, err + return output, err } // Retry logic not implemented here, but these commands typically don't need to obtain a lock. diff --git a/pkg/commands/git_cmd_obj_runner_test.go b/pkg/commands/git_cmd_obj_runner_test.go new file mode 100644 index 00000000000..e8c73727df4 --- /dev/null +++ b/pkg/commands/git_cmd_obj_runner_test.go @@ -0,0 +1,92 @@ +package commands + +import ( + "errors" + "testing" + + "github.com/jesseduffield/lazygit/pkg/commands/oscommands" + "github.com/jesseduffield/lazygit/pkg/utils" + "github.com/stretchr/testify/assert" +) + +type runnerResult struct { + output string + err error +} + +// scriptedRunner is an ICmdObjRunner stub that returns a preconfigured result +// for each successive call, letting us drive the retry loop deterministically. +// It counts calls so tests can assert whether a command was retried. +type scriptedRunner struct { + results []runnerResult + calls int +} + +func (self *scriptedRunner) next() (string, error) { + result := self.results[self.calls] + self.calls++ + return result.output, result.err +} + +func (self *scriptedRunner) Run(*oscommands.CmdObj) error { + _, err := self.next() + return err +} + +func (self *scriptedRunner) RunWithOutput(*oscommands.CmdObj) (string, error) { + return self.next() +} + +func (self *scriptedRunner) RunWithOutputs(*oscommands.CmdObj) (string, string, error) { + output, err := self.next() + return output, "", err +} + +func (self *scriptedRunner) RunAndProcessLines(*oscommands.CmdObj, func(string) (bool, error)) error { + panic("not implemented") +} + +func newTestRunner(inner *scriptedRunner) *gitCmdObjRunner { + return &gitCmdObjRunner{ + log: utils.NewDummyLog(), + innerRunner: inner, + } +} + +// dummyCmdObj returns a throwaway command; only its clonability matters, since +// the scriptedRunner ignores it and returns preconfigured results. +func dummyCmdObj() *oscommands.CmdObj { + return oscommands.NewDummyCmdObjBuilder(nil).New([]string{"git", "status"}) +} + +func TestRunWithOutputReturnsSuccessWithoutRetrying(t *testing.T) { + inner := &scriptedRunner{results: []runnerResult{{output: "done", err: nil}}} + + output, err := newTestRunner(inner).RunWithOutput(dummyCmdObj()) + + assert.NoError(t, err) + assert.Equal(t, "done", output) + assert.Equal(t, 1, inner.calls) +} + +func TestRunWithOutputDoesNotRetryNonLockError(t *testing.T) { + inner := &scriptedRunner{results: []runnerResult{{output: "boom", err: errors.New("boom")}}} + + _, err := newTestRunner(inner).RunWithOutput(dummyCmdObj()) + + assert.Error(t, err) + assert.Equal(t, 1, inner.calls) +} + +func TestRunWithOutputRetriesWhenLockErrorIsInOutput(t *testing.T) { + inner := &scriptedRunner{results: []runnerResult{ + {output: "fatal: Unable to create '/repo/.git/index.lock': File exists.", err: errors.New("exit status 128")}, + {output: "done", err: nil}, + }} + + output, err := newTestRunner(inner).RunWithOutput(dummyCmdObj()) + + assert.NoError(t, err) + assert.Equal(t, "done", output) + assert.Equal(t, 2, inner.calls) +} From 0902c5c05878b97c17611cec804a99562694372f Mon Sep 17 00:00:00 2001 From: Stefan Haller Date: Fri, 10 Jul 2026 08:56:26 +0200 Subject: [PATCH 02/12] Demonstrate that a lock error in a streamed command isn't retried The gpg helper runs commands like amend with StreamOutput, so their output isn't captured and a failed run returns an empty output string; the index.lock message is carried by the error instead. isRetryableError only inspects the output, so the retry loop never fires for these commands. In practice this means a `shift-A` amend issued while a foreground `git status` refresh briefly holds index.lock fails outright instead of retrying. Co-Authored-By: Claude Opus 4.8 (1M context) --- pkg/commands/git_cmd_obj_runner_test.go | 19 +++++++++++++++++++ 1 file changed, 19 insertions(+) diff --git a/pkg/commands/git_cmd_obj_runner_test.go b/pkg/commands/git_cmd_obj_runner_test.go index e8c73727df4..2ff2795dc29 100644 --- a/pkg/commands/git_cmd_obj_runner_test.go +++ b/pkg/commands/git_cmd_obj_runner_test.go @@ -90,3 +90,22 @@ func TestRunWithOutputRetriesWhenLockErrorIsInOutput(t *testing.T) { assert.Equal(t, "done", output) assert.Equal(t, 2, inner.calls) } + +func TestRunWithOutputRetriesWhenLockErrorIsOnlyInError(t *testing.T) { + // A streamed command (e.g. an amend run through the gpg helper) doesn't + // capture its output, so a lock failure surfaces only in the returned error + // with an empty output string. The retry logic must still recognize it. + inner := &scriptedRunner{results: []runnerResult{ + {output: "", err: errors.New("fatal: Unable to create '/repo/.git/index.lock': File exists.")}, + {output: "", err: nil}, + }} + + _, err := newTestRunner(inner).RunWithOutput(dummyCmdObj()) + + /* EXPECTED: + assert.NoError(t, err) + assert.Equal(t, 2, inner.calls) + ACTUAL: */ + assert.Error(t, err) + assert.Equal(t, 1, inner.calls) +} From c1cd500fa776c54f61b4cf6dddf6de0d90c7aa1a Mon Sep 17 00:00:00 2001 From: Stefan Haller Date: Fri, 10 Jul 2026 08:56:56 +0200 Subject: [PATCH 03/12] Retry lock errors reported only through the command's error Have isRetryableError also inspect the returned error, not just the captured output. Streamed commands (amend, commit, and other operations run through the gpg helper) don't capture output, so their index.lock failures were slipping past the retry loop and surfacing to the user as a hard "Git command failed". Now they retry like every other command. Co-Authored-By: Claude Opus 4.8 (1M context) --- pkg/commands/git_cmd_obj_runner.go | 18 ++++++++++++------ pkg/commands/git_cmd_obj_runner_test.go | 4 ---- 2 files changed, 12 insertions(+), 10 deletions(-) diff --git a/pkg/commands/git_cmd_obj_runner.go b/pkg/commands/git_cmd_obj_runner.go index a72565b5cfb..f22a0f7aaf2 100644 --- a/pkg/commands/git_cmd_obj_runner.go +++ b/pkg/commands/git_cmd_obj_runner.go @@ -20,11 +20,17 @@ type gitCmdObjRunner struct { innerRunner oscommands.ICmdObjRunner } -// isRetryableError returns true if the error output indicates a transient -// lock-related error that may succeed on retry -func isRetryableError(output string) bool { - return strings.Contains(output, ".git/index.lock") || - strings.Contains(output, "cannot lock ref") +// isRetryableError returns true if a failed command hit a transient +// lock-related condition that may succeed on retry. The lock message can reach +// us either in the command's captured output or, for streamed commands whose +// output we don't capture, only in the returned error, so we check both. +func isRetryableError(output string, err error) bool { + text := output + if err != nil { + text += "\n" + err.Error() + } + return strings.Contains(text, ".git/index.lock") || + strings.Contains(text, "cannot lock ref") } func (self *gitCmdObjRunner) Run(cmdObj *oscommands.CmdObj) error { @@ -58,7 +64,7 @@ func (self *gitCmdObjRunner) retryOnLockError(run func() (string, error)) (strin for range RetryCount { output, err = run() - if err == nil || !isRetryableError(output) { + if err == nil || !isRetryableError(output, err) { return output, err } diff --git a/pkg/commands/git_cmd_obj_runner_test.go b/pkg/commands/git_cmd_obj_runner_test.go index 2ff2795dc29..903c6a90b05 100644 --- a/pkg/commands/git_cmd_obj_runner_test.go +++ b/pkg/commands/git_cmd_obj_runner_test.go @@ -102,10 +102,6 @@ func TestRunWithOutputRetriesWhenLockErrorIsOnlyInError(t *testing.T) { _, err := newTestRunner(inner).RunWithOutput(dummyCmdObj()) - /* EXPECTED: assert.NoError(t, err) assert.Equal(t, 2, inner.calls) - ACTUAL: */ - assert.Error(t, err) - assert.Equal(t, 1, inner.calls) } From e3ecb77939e191281ef3a70509be1183acc2b41e Mon Sep 17 00:00:00 2001 From: Stefan Haller Date: Fri, 10 Jul 2026 08:57:26 +0200 Subject: [PATCH 04/12] Recognize index.lock contention in worktrees and submodules The retry check matched the literal ".git/index.lock", which only ever appears for the main worktree. A linked worktree's lock is at .git/worktrees//index.lock and a submodule's is under its own git dir, so contention there was never retried. Match the bare "index.lock" fragment instead, which covers all of them. Co-Authored-By: Claude Opus 4.8 (1M context) --- pkg/commands/git_cmd_obj_runner.go | 7 ++++++- pkg/commands/git_cmd_obj_runner_test.go | 15 +++++++++++++++ 2 files changed, 21 insertions(+), 1 deletion(-) diff --git a/pkg/commands/git_cmd_obj_runner.go b/pkg/commands/git_cmd_obj_runner.go index f22a0f7aaf2..c4d2ec04929 100644 --- a/pkg/commands/git_cmd_obj_runner.go +++ b/pkg/commands/git_cmd_obj_runner.go @@ -24,12 +24,17 @@ type gitCmdObjRunner struct { // lock-related condition that may succeed on retry. The lock message can reach // us either in the command's captured output or, for streamed commands whose // output we don't capture, only in the returned error, so we check both. +// +// We match the bare "index.lock" fragment rather than a fuller path or message +// so we catch the lock wherever git puts it: the main .git dir, a linked +// worktree's git dir (.git/worktrees//index.lock), or a submodule's git +// dir. func isRetryableError(output string, err error) bool { text := output if err != nil { text += "\n" + err.Error() } - return strings.Contains(text, ".git/index.lock") || + return strings.Contains(text, "index.lock") || strings.Contains(text, "cannot lock ref") } diff --git a/pkg/commands/git_cmd_obj_runner_test.go b/pkg/commands/git_cmd_obj_runner_test.go index 903c6a90b05..2cde503f577 100644 --- a/pkg/commands/git_cmd_obj_runner_test.go +++ b/pkg/commands/git_cmd_obj_runner_test.go @@ -105,3 +105,18 @@ func TestRunWithOutputRetriesWhenLockErrorIsOnlyInError(t *testing.T) { assert.NoError(t, err) assert.Equal(t, 2, inner.calls) } + +func TestRunWithOutputRetriesLockErrorInLinkedWorktree(t *testing.T) { + // In a linked worktree the lock lives at .git/worktrees//index.lock + // rather than .git/index.lock, so only matching the bare "index.lock" + // fragment lets the retry fire there too. + inner := &scriptedRunner{results: []runnerResult{ + {output: "", err: errors.New("fatal: Unable to create '/repo/.git/worktrees/feature/index.lock': File exists.")}, + {output: "", err: nil}, + }} + + _, err := newTestRunner(inner).RunWithOutput(dummyCmdObj()) + + assert.NoError(t, err) + assert.Equal(t, 2, inner.calls) +} From 4052057eee1073f2afe7c5cc1e3027995167c642 Mon Sep 17 00:00:00 2001 From: Stefan Haller Date: Fri, 10 Jul 2026 08:58:22 +0200 Subject: [PATCH 05/12] Back off exponentially between lock-error retries MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The retry budget was five fixed 50ms waits (250ms total). A foreground `git status` refresh can hold index.lock for longer than that on a large repo, so the retries could be exhausted before the lock clears. Wait 20ms before the first retry and double each time, giving seven attempts over a bit more than a second — enough to outlast a slow refresh while keeping the common case (a lock that clears almost immediately) fast. The initial delay is now a runner field so tests can zero it out instead of sleeping. Co-Authored-By: Claude Opus 4.8 (1M context) --- pkg/commands/git_cmd_obj_builder.go | 5 +++-- pkg/commands/git_cmd_obj_runner.go | 28 ++++++++++++++++++------- pkg/commands/git_cmd_obj_runner_test.go | 15 +++++++++++++ 3 files changed, 39 insertions(+), 9 deletions(-) diff --git a/pkg/commands/git_cmd_obj_builder.go b/pkg/commands/git_cmd_obj_builder.go index 6bc3b7d1941..20d31c11c1b 100644 --- a/pkg/commands/git_cmd_obj_builder.go +++ b/pkg/commands/git_cmd_obj_builder.go @@ -25,8 +25,9 @@ func NewGitCmdObjBuilder(log *logrus.Entry, innerBuilder *oscommands.CmdObjBuild // the price of having a convenient interface where we can say .New(...).Run() is that our builder now depends on our runner, so when we want to wrap the default builder/runner in new functionality we need to jump through some hoops. We could avoid the use of a decorator function here by just exporting the runner field on the default builder but that would be misleading because we don't want anybody using that to run commands (i.e. we want there to be a single API used across the codebase) updatedBuilder := innerBuilder.CloneWithNewRunner(func(runner oscommands.ICmdObjRunner) oscommands.ICmdObjRunner { return &gitCmdObjRunner{ - log: log, - innerRunner: runner, + log: log, + innerRunner: runner, + initialRetryDelay: defaultInitialRetryDelay, } }) diff --git a/pkg/commands/git_cmd_obj_runner.go b/pkg/commands/git_cmd_obj_runner.go index c4d2ec04929..8112b0f3095 100644 --- a/pkg/commands/git_cmd_obj_runner.go +++ b/pkg/commands/git_cmd_obj_runner.go @@ -11,13 +11,24 @@ import ( // here we're wrapping the default command runner in some git-specific stuff e.g. retry logic if we get an error due to the presence of .git/index.lock const ( - WaitTime = 50 * time.Millisecond - RetryCount = 5 + // defaultInitialRetryDelay is how long we wait before the first retry of a + // command that failed with a transient lock error. We double it before each + // subsequent retry (see retryOnLockError), so across maxRetries attempts we + // wait for a bit over a second in total. That's long enough to outlast the + // brief window during which another git process holds a lock we need — + // typically our own foreground `git status` refresh, which takes index.lock + // to persist its refreshed stat-cache. + defaultInitialRetryDelay = 20 * time.Millisecond + maxRetries = 7 ) type gitCmdObjRunner struct { log *logrus.Entry innerRunner oscommands.ICmdObjRunner + // initialRetryDelay is the wait before the first lock-error retry. It's a + // field rather than the constant directly so tests can set it to zero and + // not actually sleep. + initialRetryDelay time.Duration } // isRetryableError returns true if a failed command hit a transient @@ -64,18 +75,21 @@ func (self *gitCmdObjRunner) RunWithOutputs(cmdObj *oscommands.CmdObj) (string, // the command output we inspect to classify the failure. We clone the command // for each attempt (inside run) because an *exec.Cmd can only be run once. func (self *gitCmdObjRunner) retryOnLockError(run func() (string, error)) (string, error) { + delay := self.initialRetryDelay var output string var err error - for range RetryCount { + for attempt := range maxRetries { output, err = run() if err == nil || !isRetryableError(output, err) { - return output, err + break } - // if we have an error based on a lock, we should wait a bit and then retry - self.log.Warn("lock error prevented command from running. Retrying command after a small wait") - time.Sleep(WaitTime) + if attempt < maxRetries-1 { + self.log.Warnf("lock error prevented command from running; retrying in %s", delay) + time.Sleep(delay) + delay *= 2 + } } return output, err diff --git a/pkg/commands/git_cmd_obj_runner_test.go b/pkg/commands/git_cmd_obj_runner_test.go index 2cde503f577..bf938da542a 100644 --- a/pkg/commands/git_cmd_obj_runner_test.go +++ b/pkg/commands/git_cmd_obj_runner_test.go @@ -50,6 +50,8 @@ func newTestRunner(inner *scriptedRunner) *gitCmdObjRunner { return &gitCmdObjRunner{ log: utils.NewDummyLog(), innerRunner: inner, + // don't actually sleep between retries + initialRetryDelay: 0, } } @@ -106,6 +108,19 @@ func TestRunWithOutputRetriesWhenLockErrorIsOnlyInError(t *testing.T) { assert.Equal(t, 2, inner.calls) } +func TestRunWithOutputGivesUpAfterMaxRetries(t *testing.T) { + results := make([]runnerResult, maxRetries) + for i := range results { + results[i] = runnerResult{err: errors.New("fatal: Unable to create '/repo/.git/index.lock': File exists.")} + } + inner := &scriptedRunner{results: results} + + _, err := newTestRunner(inner).RunWithOutput(dummyCmdObj()) + + assert.Error(t, err) + assert.Equal(t, maxRetries, inner.calls) +} + func TestRunWithOutputRetriesLockErrorInLinkedWorktree(t *testing.T) { // In a linked worktree the lock lives at .git/worktrees//index.lock // rather than .git/index.lock, so only matching the bare "index.lock" From 49eefbcf379935999132041f3e24d4b60f6e1d7d Mon Sep 17 00:00:00 2001 From: Stefan Haller Date: Fri, 10 Jul 2026 09:35:24 +0200 Subject: [PATCH 06/12] Make the user-event queue unbounded MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Update and friends enqueued onto a fixed 256-slot channel with a non-blocking send that panicked when the channel was full. That guard was firing in real use: - Toggling a directory of several hundred files into a custom patch (reliably): the operation runs on a worker behind a waiting status, whose spinner enqueues a content-only render on every tick, and over the long operation these outrun the UI loop and overflow the buffer. - Editing the config in an editor that suspends lazygit: the editor subprocess runs on the UI thread, so the loop drains nothing for the whole editing session, and the full refresh fired on resume fans out across every scope at once — a burst of updates that overflows before the just-resumed loop catches up. - Any time the UI thread blocks for a long time, the periodic refreshes keep enqueuing and eventually overflow. The 256-slot buffer was chosen deliberately, with the panic as a "should never happen" guard, to preserve two properties: FIFO ordering of same-goroutine Update calls (an earlier goroutine-per-Update design reordered them), and no self-deadlock (a blocking send from the UI thread would block against the loop that drains it). But a fixed channel can only offer those by crashing on overflow. Replace it with an unbounded, order-preserving queue: a mutex-guarded slice plus a buffered(1) doorbell channel that wakes the main loop's select. Enqueuing appends and rings the doorbell; the loop drains the slice to empty on each wake. This keeps FIFO order and never blocks the caller, so there is no self-deadlock and no overflow to panic on — under a stall the queue just grows and then drains. This also removes an inconsistency: updateContentOnly did a plain blocking send while update panicked, so the two paths disagreed on what happened when the queue was full. Both now share the same enqueue. Co-Authored-By: Claude Opus 4.8 (1M context) --- pkg/gocui/flush_test.go | 10 +-- pkg/gocui/gui.go | 111 ++++++++++++++++++++++------- pkg/gocui/user_event_queue_test.go | 80 +++++++++++++++++++++ 3 files changed, 171 insertions(+), 30 deletions(-) create mode 100644 pkg/gocui/user_event_queue_test.go diff --git a/pkg/gocui/flush_test.go b/pkg/gocui/flush_test.go index 59bae427ccf..d4082fcf64e 100644 --- a/pkg/gocui/flush_test.go +++ b/pkg/gocui/flush_test.go @@ -39,15 +39,15 @@ func setupViews(t *testing.T, g *Gui) (*View, *View) { return status, main } -// pushContentOnly pushes a content-only event directly to the channel -// (synchronous, deterministic — unlike Update which spawns a goroutine). +// pushContentOnly enqueues a content-only event directly, letting the test +// control the contentOnly flag (which Update/UpdateContentOnly hard-code). func pushContentOnly(g *Gui, f func(*Gui) error) { - g.userEvents <- userEvent{f: f, task: g.NewTask(), contentOnly: true} + g.userEvents.enqueue(userEvent{f: f, task: g.NewTask(), contentOnly: true}) } -// pushRegular pushes a regular event directly to the channel. +// pushRegular enqueues a regular (non-content-only) event directly. func pushRegular(g *Gui, f func(*Gui) error) { - g.userEvents <- userEvent{f: f, task: g.NewTask(), contentOnly: false} + g.userEvents.enqueue(userEvent{f: f, task: g.NewTask(), contentOnly: false}) } func TestFlushContentOnly_SkipsUntaintedViews(t *testing.T) { diff --git a/pkg/gocui/gui.go b/pkg/gocui/gui.go index ee1995911ad..103c904835d 100644 --- a/pkg/gocui/gui.go +++ b/pkg/gocui/gui.go @@ -131,7 +131,7 @@ type Gui struct { viewMouseBindings []*ViewMouseBinding lastClick *clickInfo gEvents chan GocuiEvent - userEvents chan userEvent + userEvents *userEventQueue views []*View currentView *View managers []Manager @@ -238,12 +238,7 @@ func NewGui(opts NewGuiOpts) (*Gui, error) { g.stop = make(chan struct{}) g.gEvents = make(chan GocuiEvent, 20) - // Update does a non-blocking send and panics on a full channel rather than - // blocking (which would deadlock the UI goroutine against itself) or - // silently reordering. The buffer is sized well above the peak occupancy we - // see in practice, so the panic stays unreachable in normal use; if it ever - // fires, that's a real anomaly to investigate, not a cue to grow the buffer. - g.userEvents = make(chan userEvent, 256) + g.userEvents = newUserEventQueue() g.taskManager = newTaskManager() if opts.PlayRecording { @@ -618,29 +613,83 @@ type userEvent struct { contentOnly bool } -// Update enqueues f on the user-events channel for the UI loop to run on its -// next iteration. Multiple Update calls from the same goroutine arrive in -// source order via the channel's FIFO. The send is non-blocking — if the -// channel is full we panic rather than block or silently reorder, since a -// blocked send from the UI goroutine would deadlock against itself and -// silently switching to inline execution would break the ordering guarantee -// callers rely on. The buffer is sized generously enough that this should -// never fire in practice; if it does, that's a signal to investigate, not -// to grow the buffer reflexively. -func (g *Gui) Update(f func(*Gui) error) { - task := g.NewTask() +// userEventQueue is an unbounded, order-preserving FIFO of work enqueued by +// Update and friends for the main loop to run. +// +// It's unbounded (rather than a fixed-size channel) because producers must +// never block or lose work. Update can be called from the UI goroutine itself, +// where a blocking send would deadlock against the loop that drains the queue; +// and it can be called from arbitrary worker goroutines that may enqueue faster +// than the loop drains. That happens while the loop is stalled — suspended for +// a subprocess (the editor runs on the UI thread), or hung in a long handler — +// and also when a long-running worker operation emits a steady stream of +// updates that outpaces the loop (e.g. the waiting-status spinner ticks while a +// large directory is toggled into a custom patch). A fixed channel forces a +// choice between blocking (deadlock), dropping or reordering, and panicking on +// overflow; an unbounded queue avoids all three while preserving FIFO order. +// +// enqueue appends under the mutex and rings the doorbell; the main loop selects +// on the doorbell to wake, then drains the slice to empty. The doorbell is +// buffered(1) and rung with a non-blocking send, so it's a coalescing "work +// pending" flag rather than a per-event signal: a burst of appends leaves at +// most one token, and the loop drains everything the token represents on a +// single wake. A token left over after a drain (because the drain happened to +// empty the slice after the ring) just causes one harmless empty wake. +type userEventQueue struct { + mutex sync.Mutex + events []userEvent + doorbell chan struct{} +} + +func newUserEventQueue() *userEventQueue { + return &userEventQueue{doorbell: make(chan struct{}, 1)} +} + +// enqueue appends an event and wakes the main loop. It never blocks. +func (q *userEventQueue) enqueue(ev userEvent) { + q.mutex.Lock() + q.events = append(q.events, ev) + q.mutex.Unlock() select { - case g.userEvents <- userEvent{f: f, task: task}: + case q.doorbell <- struct{}{}: default: - panic("gocui: userEvents channel full; refusing to block or reorder") } } +// dequeue pops the oldest event, reporting false when the queue is empty. +func (q *userEventQueue) dequeue() (userEvent, bool) { + q.mutex.Lock() + defer q.mutex.Unlock() + + if len(q.events) == 0 { + return userEvent{}, false + } + ev := q.events[0] + if len(q.events) == 1 { + // Release the backing array whenever the queue drains, so a one-off + // burst doesn't pin its peak size for the rest of the session. + q.events = nil + } else { + q.events[0] = userEvent{} + q.events = q.events[1:] + } + return ev, true +} + +// Update enqueues f for the UI loop to run on its next iteration. Multiple +// Update calls from the same goroutine arrive in source order (the queue is +// FIFO). The enqueue never blocks and never drops work; see userEventQueue for +// why the queue is unbounded. +func (g *Gui) Update(f func(*Gui) error) { + task := g.NewTask() + g.userEvents.enqueue(userEvent{f: f, task: task}) +} + // Like Update, but signals that the callback only modifies content. func (g *Gui) UpdateContentOnly(f func(*Gui) error) { task := g.NewTask() - g.userEvents <- userEvent{f: f, task: task, contentOnly: true} + g.userEvents.enqueue(userEvent{f: f, task: task, contentOnly: true}) } // Calls a function in a goroutine. Handles panics gracefully and tracks @@ -766,7 +815,14 @@ func (g *Gui) processEvent() error { if err := g.handleError(g.handleEvent(&ev)); err != nil { return err } - case ev := <-g.userEvents: + case <-g.userEvents.doorbell: + ev, ok := g.userEvents.dequeue() + if !ok { + // A leftover doorbell token whose events were already drained by a + // previous iteration's processRemainingEvents: nothing to run and + // nothing new to render. + return nil + } contentOnly = ev.contentOnly defer func() { ev.task.Done() }() @@ -798,15 +854,20 @@ func (g *Gui) processRemainingEvents() (bool, error) { if err := g.handleError(g.handleEvent(&ev)); err != nil { return false, err } - case ev := <-g.userEvents: + default: + // No gui event is pending; drain a queued user event instead. + // gui events take priority so input stays responsive, but they're + // bounded (buffer of 20), so this can't starve the user-event queue. + ev, ok := g.userEvents.dequeue() + if !ok { + return contentOnly, nil + } contentOnly = ev.contentOnly && contentOnly err := g.handleError(ev.f(g)) ev.task.Done() if err != nil { return false, err } - default: - return contentOnly, nil } } } diff --git a/pkg/gocui/user_event_queue_test.go b/pkg/gocui/user_event_queue_test.go new file mode 100644 index 00000000000..10e86eef8ab --- /dev/null +++ b/pkg/gocui/user_event_queue_test.go @@ -0,0 +1,80 @@ +package gocui + +import ( + "sync" + "testing" + + "github.com/stretchr/testify/assert" +) + +// Enqueuing far more events than the old fixed 256-slot buffer, without the +// main loop draining them, used to panic ("userEvents channel full"). It must +// not: producers can legitimately burst faster than a stalled UI loop drains +// (e.g. one command-log entry per git command when adding a large directory to +// a custom patch, or any producer while the loop is blocked in a subprocess). +// The events must also stay in FIFO order. +func TestUpdateIsUnboundedAndPreservesOrder(t *testing.T) { + g := newTestGui(t) + + const n = 1000 + var got []int + for i := range n { + g.Update(func(*Gui) error { + got = append(got, i) + return nil + }) + } + + // Drain the whole queue the way the main loop's inner drain does. + _, err := g.processRemainingEvents() + assert.NoError(t, err) + + want := make([]int, n) + for i := range want { + want[i] = i + } + assert.Equal(t, want, got) +} + +// Concurrent producers must be able to enqueue safely (run under -race). Only +// same-goroutine order is guaranteed, so we check that every event is delivered +// exactly once and that each producer's own events stay in order. +func TestUpdateConcurrentProducers(t *testing.T) { + g := newTestGui(t) + + const producers = 8 + const perProducer = 500 + + type item struct{ producer, seq int } + var got []item + + var wg sync.WaitGroup + for p := range producers { + wg.Add(1) + go func() { + defer wg.Done() + for seq := range perProducer { + g.Update(func(*Gui) error { + got = append(got, item{p, seq}) + return nil + }) + } + }() + } + // Update is a synchronous, non-blocking enqueue, so once every producer has + // returned, every event is in the queue and a single drain sees them all. + wg.Wait() + + _, err := g.processRemainingEvents() + assert.NoError(t, err) + + assert.Len(t, got, producers*perProducer) + lastSeq := make([]int, producers) + for p := range lastSeq { + lastSeq[p] = -1 + } + for _, it := range got { + assert.Equal(t, lastSeq[it.producer]+1, it.seq, "producer %d events out of order", it.producer) + lastSeq[it.producer] = it.seq + } +} From f0b139f3ab42f833b42792c10fd63f361de04a76 Mon Sep 17 00:00:00 2001 From: Stefan Haller Date: Fri, 10 Jul 2026 12:50:57 +0200 Subject: [PATCH 07/12] Log the user-event queue's high-water mark MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Now that the queue is unbounded, its depth is a useful signal for understanding how the event loop behaves under load — and we expect it to look very different across builds (e.g. master, which carries the bounce-state-updates-to-ui-thread work, versus the v0.63.0 release this fix ships in). Track the deepest the queue has ever been and log an Info line whenever that record is broken, so the numbers show up in the log for later reasoning. The mark is session-wide and doesn't reset when the queue drains. gocui has no logger of its own, so it exposes the new depth through a handler (matching the existing SetFocusHandler / SetOpenHyperlinkFunc pattern) that the gui registers to log via its own logger. Co-Authored-By: Claude Opus 4.8 (1M context) --- pkg/gocui/gui.go | 32 ++++++++++++++++++++++++++++++ pkg/gocui/user_event_queue_test.go | 31 +++++++++++++++++++++++++++++ pkg/gui/gui.go | 4 ++++ 3 files changed, 67 insertions(+) diff --git a/pkg/gocui/gui.go b/pkg/gocui/gui.go index 103c904835d..bb9fc9874b8 100644 --- a/pkg/gocui/gui.go +++ b/pkg/gocui/gui.go @@ -603,6 +603,13 @@ func (g *Gui) SetRenderSearchStatusFunc(renderSearchStatusFunc func(*View, int, g.renderSearchStatusFunc = renderSearchStatusFunc } +// SetUpdateQueueHighWaterMarkHandler registers a diagnostic callback invoked +// with the new depth whenever the queue of pending Update callbacks reaches a +// new maximum. It may be called from any goroutine. +func (g *Gui) SetUpdateQueueHighWaterMarkHandler(f func(depth int)) { + g.userEvents.setHighWaterMarkHandler(f) +} + // userEvent represents an event triggered by the user. type userEvent struct { f func(*Gui) error @@ -639,6 +646,13 @@ type userEventQueue struct { mutex sync.Mutex events []userEvent doorbell chan struct{} + + // highWaterMark is the deepest the queue has ever been, and + // onHighWaterMark (if set) is called with the new depth each time that + // record is broken. Purely diagnostic: it lets us see how deep the queue + // gets in practice (see SetUpdateQueueHighWaterMarkHandler). + highWaterMark int + onHighWaterMark func(int) } func newUserEventQueue() *userEventQueue { @@ -649,14 +663,32 @@ func newUserEventQueue() *userEventQueue { func (q *userEventQueue) enqueue(ev userEvent) { q.mutex.Lock() q.events = append(q.events, ev) + newHighWaterMark := 0 + if len(q.events) > q.highWaterMark { + q.highWaterMark = len(q.events) + newHighWaterMark = q.highWaterMark + } + onHighWaterMark := q.onHighWaterMark q.mutex.Unlock() + // Report outside the lock: the handler does I/O (logging) and must not + // stall other producers or the draining loop. + if newHighWaterMark > 0 && onHighWaterMark != nil { + onHighWaterMark(newHighWaterMark) + } + select { case q.doorbell <- struct{}{}: default: } } +func (q *userEventQueue) setHighWaterMarkHandler(f func(int)) { + q.mutex.Lock() + q.onHighWaterMark = f + q.mutex.Unlock() +} + // dequeue pops the oldest event, reporting false when the queue is empty. func (q *userEventQueue) dequeue() (userEvent, bool) { q.mutex.Lock() diff --git a/pkg/gocui/user_event_queue_test.go b/pkg/gocui/user_event_queue_test.go index 10e86eef8ab..e547debb49a 100644 --- a/pkg/gocui/user_event_queue_test.go +++ b/pkg/gocui/user_event_queue_test.go @@ -36,6 +36,37 @@ func TestUpdateIsUnboundedAndPreservesOrder(t *testing.T) { assert.Equal(t, want, got) } +// The high-water-mark handler fires only when the queue reaches a new maximum +// depth, reporting that depth. It does not reset when the queue drains. +func TestUpdateQueueHighWaterMark(t *testing.T) { + g := newTestGui(t) + + var marks []int + g.SetUpdateQueueHighWaterMarkHandler(func(depth int) { marks = append(marks, depth) }) + + noop := func(*Gui) error { return nil } + + // Three enqueues with no drain: new highs 1, 2, 3. + g.Update(noop) + g.Update(noop) + g.Update(noop) + _, err := g.processRemainingEvents() + assert.NoError(t, err) + + // Two enqueues stay below the previous high of 3: no new marks. + g.Update(noop) + g.Update(noop) + _, err = g.processRemainingEvents() + assert.NoError(t, err) + + // Four enqueues with no drain: only depth 4 beats the previous high. + for range 4 { + g.Update(noop) + } + + assert.Equal(t, []int{1, 2, 3, 4}, marks) +} + // Concurrent producers must be able to enqueue safely (run under -race). Only // same-goroutine order is guaranteed, so we check that every event is delivered // exactly once and that each producer's own events stay in order. diff --git a/pkg/gui/gui.go b/pkg/gui/gui.go index e23afd12430..61d4a90e2d7 100644 --- a/pkg/gui/gui.go +++ b/pkg/gui/gui.go @@ -414,6 +414,10 @@ func (gui *Gui) onNewRepo(startArgs appTypes.StartArgs, contextKey types.Context return nil }) + gui.g.SetUpdateQueueHighWaterMarkHandler(func(depth int) { + gui.c.Log.Infof("User-event queue reached a new high-water mark: %d", depth) + }) + gui.g.SetOnSelectSearchResultFunc(func(v *gocui.View, selectedLineIdx int) { ctx, ok := gui.helpers.View.ContextForView(v.Name()) if ok { From f116874f0a8c243642ee7ea3ae5caffa1e2ec782 Mon Sep 17 00:00:00 2001 From: Stefan Haller Date: Wed, 15 Jul 2026 10:07:15 +0200 Subject: [PATCH 08/12] Fix a deadlock when a Windows pty task is stopped mid-output MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit winPty.Close could block indefinitely, and it is called while holding the global PtyMutex and while the task's onDone sync.Once is executing, so blocking there wedges the task's entire cleanup chain: the next NewTask call blocks on <-notifyStopped while holding waitingMutex, every later task for that view queues up behind it, and onResize blocks on PtyMutex — a full UI freeze. (Reported by a user via go-deadlock's 30s watchdog; a regression from the ConPTY support introduced for v0.63.0.) ClosePseudoConsole is what blocks; before Windows 11 24H2 it can do so in two ways. It flushes the client's pending output into the out pipe, but a stopped task's scanner goroutine has already quit draining, so with a client that's still producing output the flush never completes; this can also wedge the background waiter's closeHpc, which runs with the pipes deliberately left open. And it waits for the console host to exit, but closing only delivers CTRL_CLOSE_EVENT to the attached client without terminating it, so a client that keeps running (git still computing an expensive diff, a pager waiting for input) keeps the host alive arbitrarily long. Run the teardown on a background goroutine so Close returns immediately no matter which of these strikes, and within it close our pipe ends before the pseudoconsole, without taking p.mu: breaking the pipes fails a pending flush fast, which also unblocks a waiter already stuck in one. Co-Authored-By: Claude Fable 5 --- pkg/commands/oscommands/pty_windows.go | 53 +++++++++++++++----------- 1 file changed, 30 insertions(+), 23 deletions(-) diff --git a/pkg/commands/oscommands/pty_windows.go b/pkg/commands/oscommands/pty_windows.go index 0a4a0647763..72ade5110bf 100644 --- a/pkg/commands/oscommands/pty_windows.go +++ b/pkg/commands/oscommands/pty_windows.go @@ -7,6 +7,7 @@ import ( "sync" "unsafe" + "github.com/jesseduffield/lazygit/pkg/utils" "golang.org/x/sys/windows" ) @@ -15,15 +16,13 @@ type winPty struct { inWrite *os.File outRead *os.File - // mu guards the teardown state below and serializes it against Resize. - // hpcClosed gates ClosePseudoConsole (it must run exactly once) and also - // keeps Resize from touching the HPCON once it's been freed: the - // background waiter in StartPty closes the pseudoconsole on child exit, - // which would otherwise race a concurrent onResize and hand - // ResizePseudoConsole a freed handle. + // mu guards hpcClosed, which gates ClosePseudoConsole (it must run + // exactly once) and also keeps Resize from touching the HPCON once it's + // been freed: the background waiter in StartPty closes the pseudoconsole + // on child exit, which would otherwise race a concurrent onResize and + // hand ResizePseudoConsole a freed handle. mu sync.Mutex hpcClosed bool - closed bool } func (p *winPty) Read(buf []byte) (int, error) { return p.outRead.Read(buf) } @@ -49,11 +48,6 @@ func (p *winPty) Resize(cols, rows uint16) error { func (p *winPty) closeHpc() { p.mu.Lock() defer p.mu.Unlock() - p.closeHpcLocked() -} - -// closeHpcLocked closes the pseudoconsole; the caller must hold p.mu. -func (p *winPty) closeHpcLocked() { if p.hpcClosed { return } @@ -61,18 +55,31 @@ func (p *winPty) closeHpcLocked() { windows.ClosePseudoConsole(p.hpc) } +// Close tears the pty down without waiting for it: the teardown runs on a +// background goroutine and Close returns immediately. +// +// It has to, because ClosePseudoConsole can block for a long time: before +// Windows 11 24H2 it waits for the console host to exit, and since closing +// only delivers CTRL_CLOSE_EVENT to the attached client without terminating +// it, a client that keeps running (git still computing an expensive diff, a +// pager waiting for input) keeps the host — and with it ClosePseudoConsole — +// alive arbitrarily long. Close is called while holding the global PtyMutex +// and while the task's onDone once is executing, where blocking wedges every +// subsequent task for the view (and with it the UI), so none of this may +// happen on the caller's thread. +// +// Within the teardown, the pipe ends must be closed before the +// pseudoconsole, and without holding p.mu: closing the pseudoconsole flushes +// the client's pending output into the out pipe, and with the task stopped +// 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. func (p *winPty) Close() error { - p.mu.Lock() - defer p.mu.Unlock() - if p.closed { - return nil - } - p.closed = true - // Closing the pseudoconsole breaks the pipes; the child's next write - // fails and it exits. Then we close our ends of the pipes. - p.closeHpcLocked() - p.inWrite.Close() - p.outRead.Close() + go utils.Safe(func() { + p.inWrite.Close() + p.outRead.Close() + p.closeHpc() + }) return nil } From a65d468cd3812b98c0acb2baf8ad3e548911be40 Mon Sep 17 00:00:00 2001 From: Stefan Haller Date: Wed, 15 Jul 2026 10:56:56 +0200 Subject: [PATCH 09/12] Determine the latest tag from the checked-out commit's history The Get Latest Tag step used to pick the most recently created tag in the entire repo, regardless of whether it is reachable from the commit being released. In preparation for supporting releases from branches other than master, use the nearest tag that is an ancestor of the checked-out commit instead. This way, a patch release cut from an older release branch bumps that branch's own latest tag even when master already carries a newer release, and the "changes since last release" check compares against the release that actually precedes this one in history. --- .github/workflows/release.yml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.github/workflows/release.yml b/.github/workflows/release.yml index 9b9815d447b..84636424b4c 100644 --- a/.github/workflows/release.yml +++ b/.github/workflows/release.yml @@ -54,7 +54,7 @@ jobs: - name: Get Latest Tag run: | - latest_tag=$(git describe --tags $(git rev-list --tags --max-count=1) || echo "v0.0.0") + latest_tag=$(git describe --tags --abbrev=0 || echo "v0.0.0") if ! [[ $latest_tag =~ ^v[0-9]+\.[0-9]+\.[0-9]+$ ]]; then echo "Error: Tag format is invalid. Expected format: vX.X.X" From dda0af0f483b34c7105d64dceca0b088c2e7e55f Mon Sep 17 00:00:00 2001 From: Stefan Haller Date: Wed, 15 Jul 2026 13:16:21 +0200 Subject: [PATCH 10/12] Allow having branch and tag with the same name When creating a patch release from a branch called `v0.63.1`, the new tag would get the same name and pushing it would fail with `error: src refspec v0.63.1 matches more than one`. --- .github/workflows/release.yml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.github/workflows/release.yml b/.github/workflows/release.yml index 84636424b4c..a3a5f62c2fc 100644 --- a/.github/workflows/release.yml +++ b/.github/workflows/release.yml @@ -151,7 +151,7 @@ jobs: git config user.name "github-actions[bot]" git config user.email "github-actions[bot]@users.noreply.github.com" git tag "$NEW_TAG" -a -m "Release $NEW_TAG" - git push origin "$NEW_TAG" + git push origin "refs/tags/$NEW_TAG" - name: Setup Go uses: actions/setup-go@v6 From 1d99ba56fc768ae64ffc125c1f5d300060eed860 Mon Sep 17 00:00:00 2001 From: Stefan Haller Date: Wed, 15 Jul 2026 10:57:22 +0200 Subject: [PATCH 11/12] Allow releasing from a branch other than master This is useful for cutting a patch release for the previous version when master already contains work that shouldn't be released yet; for example, v0.63.1 had to be tagged and released by hand from a v0.63.1 branch off the v0.63.0 tag because the workflow could only release master. Scheduled runs are unaffected: with no input provided, the ref is empty and the checkout falls back to the default branch. --- .github/workflows/release.yml | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/.github/workflows/release.yml b/.github/workflows/release.yml index a3a5f62c2fc..1ef271d6faf 100644 --- a/.github/workflows/release.yml +++ b/.github/workflows/release.yml @@ -17,6 +17,11 @@ on: options: - minor - patch + branch: + description: 'Branch to release from' + type: string + required: true + default: 'master' ignore_blocks: description: 'Ignore blocking PRs/issues' type: boolean @@ -49,6 +54,7 @@ jobs: uses: actions/checkout@v7 with: repository: jesseduffield/lazygit + ref: ${{ inputs.branch }} token: ${{ secrets.LAZYGIT_RELEASE_PAT }} fetch-depth: 0 From bd8c06ddc0910bccd6522131a2ad9c0c7c143396 Mon Sep 17 00:00:00 2001 From: Stefan Haller Date: Wed, 15 Jul 2026 11:01:56 +0200 Subject: [PATCH 12/12] Rename version_bump options to be extra clear I keep getting slightly confused as to which is which, so make this extra clear. While at it, change the default to minor, this is the option that is more often used now that we don't have regular scheduled releases any more. --- .github/workflows/release.yml | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/.github/workflows/release.yml b/.github/workflows/release.yml index 1ef271d6faf..e3cf63bbf5b 100644 --- a/.github/workflows/release.yml +++ b/.github/workflows/release.yml @@ -13,10 +13,10 @@ on: description: 'Version bump type' type: choice required: true - default: 'patch' + default: 'minor (normal)' options: - - minor - - patch + - minor (normal) + - patch (hotfix) branch: description: 'Branch to release from' type: string @@ -127,7 +127,7 @@ jobs: IFS='.' read -r major minor patch <<< "$LATEST_TAG" if [[ "$EVENT_NAME" == "workflow_dispatch" ]]; then - if [[ "$VERSION_BUMP" == "patch" ]]; then + if [[ "$VERSION_BUMP" == "patch (hotfix)" ]]; then patch=$((patch + 1)) else minor=$((minor + 1))