Skip to content

Commit 98801da

Browse files
authored
Terminate git processes more gracefully to avoid the stale index.lock problem (#4782)
- **PR Description** Instead of killing git processes when we no longer need them, close their ptys or output pipes; this makes them terminate more gracefully, and it avoids the problem of left-over index.lock files that the next git command chokes on. To fix the index.lock problem, only the first two commits of the branch are needed. I decided to go a bit further and add more commits to make similar changes to other places in the code; these might be considered more risky than necessary, and we might decide to drop or revert them if they cause problems. But it does allow us to remove the kill package dependency altogether. Fixes #2050.
2 parents 47afa7e + 1a72561 commit 98801da

File tree

12 files changed

+37
-281
lines changed

12 files changed

+37
-281
lines changed

go.mod

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -16,7 +16,6 @@ require (
1616
github.com/jesseduffield/generics v0.0.0-20250517122708-b0b4a53a6f5c
1717
github.com/jesseduffield/go-git/v5 v5.14.1-0.20250407170251-e1a013310ccd
1818
github.com/jesseduffield/gocui v0.3.1-0.20250711082438-4aa4fd0b4d22
19-
github.com/jesseduffield/kill v0.0.0-20250101124109-e216ddbe133a
2019
github.com/jesseduffield/lazycore v0.0.0-20221012050358-03d2e40243c5
2120
github.com/jesseduffield/minimal/gitignore v0.3.3-0.20211018110810-9cde264e6b1e
2221
github.com/kardianos/osext v0.0.0-20190222173326-2bc1f35cddc0

go.sum

Lines changed: 0 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -196,8 +196,6 @@ github.com/jesseduffield/go-git/v5 v5.14.1-0.20250407170251-e1a013310ccd h1:ViKj
196196
github.com/jesseduffield/go-git/v5 v5.14.1-0.20250407170251-e1a013310ccd/go.mod h1:lRhCiBr6XjQrvcQVa+UYsy/99d3wMXn/a0nSQlhnhlA=
197197
github.com/jesseduffield/gocui v0.3.1-0.20250711082438-4aa4fd0b4d22 h1:vhMwEsLlMtuKKo9/z3Qcggycgad8oV7+siwOZEnJDOs=
198198
github.com/jesseduffield/gocui v0.3.1-0.20250711082438-4aa4fd0b4d22/go.mod h1:sLIyZ2J42R6idGdtemZzsiR3xY5EF0KsvYEGh3dQv3s=
199-
github.com/jesseduffield/kill v0.0.0-20250101124109-e216ddbe133a h1:UDeJ3EBk04bXDLOPvuqM3on8HvyJfISw0+UMqW+0a4g=
200-
github.com/jesseduffield/kill v0.0.0-20250101124109-e216ddbe133a/go.mod h1:FSWDLKT0NQpntbDd1H3lbz51fhCVlMzy/J0S6nM727Q=
201199
github.com/jesseduffield/lazycore v0.0.0-20221012050358-03d2e40243c5 h1:CDuQmfOjAtb1Gms6a1p5L2P8RhbLUq5t8aL7PiQd2uY=
202200
github.com/jesseduffield/lazycore v0.0.0-20221012050358-03d2e40243c5/go.mod h1:qxN4mHOAyeIDLP7IK7defgPClM/z1Kze8VVQiaEjzsQ=
203201
github.com/jesseduffield/minimal/gitignore v0.3.3-0.20211018110810-9cde264e6b1e h1:uw/oo+kg7t/oeMs6sqlAwr85ND/9cpO3up3VxphxY0U=

pkg/commands/oscommands/cmd_obj_runner.go

Lines changed: 23 additions & 20 deletions
Original file line numberDiff line numberDiff line change
@@ -171,16 +171,17 @@ func (self *cmdObjRunner) RunAndProcessLines(cmdObj *CmdObj, onLine func(line st
171171
line := scanner.Text()
172172
stop, err := onLine(line)
173173
if err != nil {
174+
stdoutPipe.Close()
174175
return err
175176
}
176177
if stop {
177-
_ = Kill(cmd)
178+
stdoutPipe.Close() // close the pipe so that the called process terminates
178179
break
179180
}
180181
}
181182

182183
if scanner.Err() != nil {
183-
_ = Kill(cmd)
184+
stdoutPipe.Close()
184185
return scanner.Err()
185186
}
186187

@@ -335,7 +336,7 @@ func (self *cmdObjRunner) runAndDetectCredentialRequest(
335336
tr := io.TeeReader(handler.stdoutPipe, cmdWriter)
336337

337338
go utils.Safe(func() {
338-
self.processOutput(tr, handler.stdinPipe, promptUserForCredential, cmdObj)
339+
self.processOutput(tr, handler.stdinPipe, promptUserForCredential, handler.close, cmdObj)
339340
})
340341
})
341342
}
@@ -344,6 +345,7 @@ func (self *cmdObjRunner) processOutput(
344345
reader io.Reader,
345346
writer io.Writer,
346347
promptUserForCredential func(CredentialType) <-chan string,
348+
closeFunc func() error,
347349
cmdObj *CmdObj,
348350
) {
349351
checkForCredentialRequest := self.getCheckForCredentialRequestFunc()
@@ -357,25 +359,26 @@ func (self *cmdObjRunner) processOutput(
357359
if ok {
358360
responseChan := promptUserForCredential(askFor)
359361
if responseChan == nil {
360-
// Returning a nil channel means we should kill the process.
361-
// Note that we don't break the loop after this, because we
362-
// still need to drain the output, otherwise the Wait() call
363-
// later might block.
364-
if err := Kill(cmdObj.GetCmd()); err != nil {
362+
// Returning a nil channel means we should terminate the process.
363+
// We achieve this by closing the pty that it's running in. Note that this won't
364+
// work for the case where we're not running in a pty (i.e. on Windows), but
365+
// in that case we'll never be prompted for credentials, so it's not a concern.
366+
if err := closeFunc(); err != nil {
365367
self.log.Error(err)
366368
}
367-
} else {
368-
if task != nil {
369-
task.Pause()
370-
}
371-
toInput := <-responseChan
372-
if task != nil {
373-
task.Continue()
374-
}
375-
// If the return data is empty we don't write anything to stdin
376-
if toInput != "" {
377-
_, _ = writer.Write([]byte(toInput))
378-
}
369+
break
370+
}
371+
372+
if task != nil {
373+
task.Pause()
374+
}
375+
toInput := <-responseChan
376+
if task != nil {
377+
task.Continue()
378+
}
379+
// If the return data is empty we don't write anything to stdin
380+
if toInput != "" {
381+
_, _ = writer.Write([]byte(toInput))
379382
}
380383
}
381384
}

pkg/commands/oscommands/cmd_obj_runner_test.go

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -127,7 +127,7 @@ func TestProcessOutput(t *testing.T) {
127127
writer := &strings.Builder{}
128128

129129
cmdObj := &CmdObj{task: gocui.NewFakeTask()}
130-
runner.processOutput(reader, writer, toChanFn(scenario.promptUserForCredential), cmdObj)
130+
runner.processOutput(reader, writer, toChanFn(scenario.promptUserForCredential), func() error { return nil }, cmdObj)
131131

132132
if writer.String() != scenario.expectedToWrite {
133133
t.Errorf("expected to write '%s' but got '%s'", scenario.expectedToWrite, writer.String())

pkg/commands/oscommands/os.go

Lines changed: 0 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -13,7 +13,6 @@ import (
1313
"github.com/samber/lo"
1414

1515
"github.com/atotto/clipboard"
16-
"github.com/jesseduffield/kill"
1716
"github.com/jesseduffield/lazygit/pkg/common"
1817
"github.com/jesseduffield/lazygit/pkg/config"
1918
"github.com/jesseduffield/lazygit/pkg/utils"
@@ -264,16 +263,6 @@ func (c *OSCommand) PipeCommands(cmdObjs ...*CmdObj) error {
264263
return nil
265264
}
266265

267-
// Kill kills a process. If the process has Setpgid == true, then we have anticipated that it might spawn its own child processes, so we've given it a process group ID (PGID) equal to its process id (PID) and given its child processes will inherit the PGID, we can kill that group, rather than killing the process itself.
268-
func Kill(cmd *exec.Cmd) error {
269-
return kill.Kill(cmd)
270-
}
271-
272-
// PrepareForChildren sets Setpgid to true on the cmd, so that when we run it as a subprocess, we can kill its group rather than the process itself. This is because some commands, like `docker-compose logs` spawn multiple children processes, and killing the parent process isn't sufficient for killing those child processes. We set the group id here, and then in subprocess.go we check if the group id is set and if so, we kill the whole group rather than just the one process.
273-
func PrepareForChildren(cmd *exec.Cmd) {
274-
kill.PrepareForChildren(cmd)
275-
}
276-
277266
func (c *OSCommand) CopyToClipboard(str string) error {
278267
escaped := strings.ReplaceAll(str, "\n", "\\n")
279268
truncated := utils.TruncateWithEllipsis(escaped, 40)

pkg/gui/tasks_adapter.go

Lines changed: 12 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -18,10 +18,13 @@ func (gui *Gui) newCmdTask(view *gocui.View, cmd *exec.Cmd, prefix string) error
1818

1919
manager := gui.getManager(view)
2020

21+
var r io.ReadCloser
2122
start := func() (*exec.Cmd, io.Reader) {
22-
r, err := cmd.StdoutPipe()
23+
var err error
24+
r, err = cmd.StdoutPipe()
2325
if err != nil {
2426
gui.c.Log.Error(err)
27+
r = nil
2528
}
2629
cmd.Stderr = cmd.Stdout
2730

@@ -32,8 +35,15 @@ func (gui *Gui) newCmdTask(view *gocui.View, cmd *exec.Cmd, prefix string) error
3235
return cmd, r
3336
}
3437

38+
onClose := func() {
39+
if r != nil {
40+
r.Close()
41+
r = nil
42+
}
43+
}
44+
3545
linesToRead := gui.linesToReadFromCmdTask(view)
36-
if err := manager.NewTask(manager.NewCmdTask(start, prefix, linesToRead, nil), cmdStr); err != nil {
46+
if err := manager.NewTask(manager.NewCmdTask(start, prefix, linesToRead, onClose), cmdStr); err != nil {
3747
gui.c.Log.Error(err)
3848
}
3949

pkg/tasks/tasks.go

Lines changed: 1 addition & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -5,12 +5,10 @@ import (
55
"fmt"
66
"io"
77
"os/exec"
8-
"strings"
98
"sync"
109
"time"
1110

1211
"github.com/jesseduffield/gocui"
13-
"github.com/jesseduffield/lazygit/pkg/commands/oscommands"
1412
"github.com/jesseduffield/lazygit/pkg/utils"
1513
"github.com/sasha-s/go-deadlock"
1614
"github.com/sirupsen/logrus"
@@ -167,14 +165,7 @@ func (self *ViewBufferManager) NewCmdTask(start func() (*exec.Cmd, io.Reader), p
167165
// and the user is flicking through a bunch of items.
168166
self.throttle = time.Since(startTime) < THROTTLE_TIME && timeToStart > COMMAND_START_THRESHOLD
169167

170-
// Kill the still-running command.
171-
if err := oscommands.Kill(cmd); err != nil {
172-
if !strings.Contains(err.Error(), "process already finished") {
173-
self.Log.Errorf("error when trying to kill cmd task: %v; Command: %v %v", err, cmd.Path, cmd.Args)
174-
}
175-
}
176-
177-
// for pty's we need to call onDone here so that cmd.Wait() doesn't block forever
168+
// close the task's stdout pipe (or the pty if we're using one) to make the command terminate
178169
onDone()
179170
}
180171
})

vendor/github.com/jesseduffield/kill/LICENSE

Lines changed: 0 additions & 21 deletions
This file was deleted.

vendor/github.com/jesseduffield/kill/README.md

Lines changed: 0 additions & 3 deletions
This file was deleted.

vendor/github.com/jesseduffield/kill/kill_default_platform.go

Lines changed: 0 additions & 33 deletions
This file was deleted.

0 commit comments

Comments
 (0)