Skip to content

Lazygit is 'suspended' when running a custom command #4320

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
karunsiri opened this issue Feb 24, 2025 · 8 comments
Open

Lazygit is 'suspended' when running a custom command #4320

karunsiri opened this issue Feb 24, 2025 · 8 comments
Labels
bug Something isn't working

Comments

@karunsiri
Copy link

Describe the bug
When I run any custom command, lazygit will be suspended with the last bit of output being

Press enter to return to lazyget

If I run lazy git from other program, for example, neovim, lazygit will just hangs permanently (because of the suspension, I think)

The custom command is git pull --all. Full output.

+ /bin/zsh -i -c git pull --all; exit $?

Already up to date.

Press enter to return to lazygitzsh: suspended (tty input)  lazygit

To Reproduce
Steps to reproduce the behavior:

  1. Open lazygit
  2. Run custom command
    git fetch --all
  3. lazygit is suspended to background

Expected behavior
Lazygit is not suspended and I am brought back to the tty.

Screenshots

Image

Version info:
Lazygit: commit=101bbb0ac56a1cf594301f45bda22c551f1aa870, build date=2025-02-22T11:33:02Z, build source=binaryRelease, version=0.47.1, os=darwin, arch=arm64, git version=2.39.5 (Apple Git-154)
git: git version 2.39.5 (Apple Git-154)

Additional context
If I run fg after lazygit is suspended, I'm brought back the lazygit instance and can quit the program with a shortcut q.

@karunsiri karunsiri added the bug Something isn't working label Feb 24, 2025
@stefanhaller
Copy link
Collaborator

Damn. We were hoping this would be fixed with #4126 (and it seems for most users it did help). No idea why it still doesn't work for you.

If we can't find any other way of fixing this, we may have to add a config option to disable the interactive shell behavior.

As a workaround for now, you might try if launching lazygit like this solves it:

lg() 
{
  export SHELL=xyz 
  lazygit
}

@karunsiri
Copy link
Author

Thank you for your promptly response @stefanhaller!
Unfortunately, the issue still persists. I don't think I have seen this problem before. I jumped from version 0.43.1 to 0.47.1.

@karunsiri
Copy link
Author

Additional context:

I ran lazygit --debug and lazygit --logs in separate terminals and it is outputting log lines until before I bring up an interactive shell window that it stops printing logs. Run custom command and also no logs.

Then lazygit was suspended (still no logs). When running fg to bring it back to foreground the logs reappear to what seems to be status refresh, etc.

Feb 24 19:27:27 |INFO| refreshing all scopes in async mode
Feb 24 19:27:27 |INFO| Refresh took 118.974µs
Feb 24 19:27:27 |DEBU| RunCommand command="git log --graph --color=always --abbrev-commit --decorate --date=relative --pretty=medium refs/heads/main --"
Feb 24 19:27:27 |DEBU| using cache for key status.showUntrackedFiles
Feb 24 19:27:27 |DEBU| RunCommand command="git status --untracked-files=all --porcelain -z --find-renames=50%"

@RBird111
Copy link

RBird111 commented Mar 2, 2025

@stefanhaller
I was able to reproduce the behavior locally, and after poking around a bit, I think I found a way to prevent job suspension in zsh interactive shells. The core problem is that job control gets turned on by default when running an interactive shell (at least for bash and zsh) and that's what keeps wrestling control away from lazygit.

Quick & Dirty Fix:
For zsh, passing in the --nomonitor option when the shell gets created seems to fix the issue.

Current behavior:

Current behavior

Behavior with --nomonitor:

Behavior with --nomonitor

Unfortunately, it doesn't seem like a similar fix can be used when invoking bash because of how/when environment options get set. However, if the current fix implemented using exit is working for interactive bash sessions then this is a moot point anyway.

Possible General Fix(es):

  • If the goal is just to load user-defined aliases/functions for use in custom commands then creating interactive shell sessions might be overkill. There might be a way to source a user's init files without instantiating a full interactive shell.
  • Set up signal handlers for SIGTTIN and SIGTTOU.
  • There's a possibility that the issue could be completely sidestepped by having the subprocess read-from/write-to a buffer instead of os.Std*. This might prevent job control from trying to stop the process group (lazygit and the custom command).

This is an interesting problem! When I have more time during the week I'll take a closer look at how custom commands are currently implemented and see if I can put together a PR to address the issue.

[OT]:
lazygit has become one of my most used command-line tools. I can't imagine how much work it must take to maintain a project like this so thank you for doing it!

@stefanhaller
Copy link
Collaborator

Thanks for the breakdown and the documentation links. It does sound like this could finally help shed some more light on the issue.

I think I understand what's going on now; however, I still don't understand why this only happens for some people and not others, and apparently only sometimes (even before we added the ; exit $? bit at the end). This feels like a timing issue to me; if you're unlucky enough that the interactive shell we spawned is still around at the time we read from the terminal to answer the "Press enter to return to lazygit" prompt, then you get the problem. Which makes me wonder why it can still be around at that time; maybe we somehow don't properly wait for the process to complete? On the other hand, we do use its exit code, so how can we do this if the process didn't terminate yet? This is still a mystery to me.

@stefanhaller
Copy link
Collaborator

The code that runs the shell command is here. As you can see, it simply calls subprocess.Run(), which calls Start() and Wait(). And Wait is documented to Wait releases any resources associated with the [Cmd]., which I understand as the process being gone when Wait returns.

@RBird111
Copy link

RBird111 commented Mar 9, 2025

I tried tackling this problem again this evening and, unfortunately, I still haven't figured it out. I do have some observations/notes though:

I think you're 100% right about this being a timing issue but I don't think the problem is that the interactive shell is still around in some cases. I attached a debugger to a running process and tried stepping through to see where the process was getting suspended. The suspension happened here which kind of makes sense since stdin is being read and so we get suspended (tty input). I say 'kind of makes sense' because I reran this a few times while running top and the subprocess is always gone by this point. After I commented out those lines lazygit still ends up getting suspended here. In both cases the subprocess is long gone.

I decided to try starting the process without a controlling terminal by adding the following to runSubprocess (syscall.SysProcAttr):

	subprocess.SysProcAttr = &syscall.SysProcAttr{Noctty: true}
	err := subprocess.Run()

This actually ended up sort of working. The process no longer got suspended but there were a couple of side effects:

  1. When using a bash shell this leads to some extra noise in the output:
+ /usr/bin/bash -i -c ls

bash: cannot set terminal process group (104851): Inappropriate ioctl for device
bash: no job control in this shell
CODE-OF-CONDUCT.md  CONTRIBUTING.md  Dockerfile  LICENSE  Makefile  README.md  VISION.md  cmd  demo  docs  go.mod  go.sum  lazygit  main.go  pkg  schema  scripts  test  vendor

Press enter to return to lazygit
  1. From what I can tell custom commands that get run like this end up in a separate process group, and so while they can receive input from stdin and print to stdout, they will ignore things like Ctrl/Cmd + C to terminate the process. This could lead to the potentially worse outcome of lazygit seeming to become unresponsive without any feedback as to why.

So not giving the subprocess a controlling terminal doesn't fix the issue but the experiment did lead me to a new possible explanation. Under certain conditions when a custom command is run and the subprocess exits it, for whatever reason, hands the controlling terminal back to lazygit's parent process (at least temporarily). When lazygit attempts to access /dev/tty, either when checking for the enter key or while trying to resume, it gets suspended by the user's shell.

I'll need to do some more experimenting but I wanted write down some of my thoughts while they're still fresh. I'll give this another go when I have some more time.

@stefanhaller
Copy link
Collaborator

but the experiment did lead me to a new possible explanation. Under certain conditions when a custom command is run and the subprocess exits it, for whatever reason, hands the controlling terminal back to lazygit's parent process (at least temporarily).

I guess it must be something like that. If this is the case, and the "temporarily" is true, I wonder if we could simply wait until we are the foreground process again. Something along these lines:

diff --git a/pkg/gui/gui.go b/pkg/gui/gui.go
index aa44aa92c..8ceaa14d8 100644
--- a/pkg/gui/gui.go
+++ b/pkg/gui/gui.go
@@ -11,6 +11,9 @@ import (
 	"sort"
 	"strings"
 	"sync"
+	"time"
+
+	"golang.org/x/sys/unix"
 
 	"github.com/jesseduffield/gocui"
 	"github.com/jesseduffield/lazycore/pkg/boxlayout"
@@ -974,6 +977,13 @@ func (gui *Gui) runSubprocess(cmdObj oscommands.ICmdObj) error { //nolint:unpara
 	subprocess.Stderr = io.Discard
 	subprocess.Stdin = nil
 
+	i := 0
+	for unix.Getpid() != unix.Getpgrp() {
+		time.Sleep(time.Millisecond)
+		i++
+	}
+	fmt.Fprintf(os.Stdout, "Had to sleep %d ms\n", i)
+
 	if gui.integrationTest == nil && (gui.Config.GetUserConfig().PromptToReturnFromSubprocess || err != nil) {
 		fmt.Fprintf(os.Stdout, "\n%s", style.FgGreen.Sprint(gui.Tr.PressEnterToReturn))
 

In my experiments this always printed "0 ms", but that's not a surprise because I could never reproduce the issue on my machine. I'm curious what this prints for you.

I'm not actually suggesting we do something like this, though. For example, when users start lazygit from a wrapper script (or using make run like I always do during development), then the prompt hangs forever, because it's the wrapper script (or the make process) that becomes the foreground process, not lazygit.

Also, during experimentation I only now realized how much of a performance problem it is to launch an interactive shell. For many people it may not make as much of a difference, but on my system, calling zsh -ic true takes 600ms, whereas zsh -c true takes less than 2ms. This is too high of a price to pay for using shell aliases.

So I would now propose to revert the whole interactive shell approach, and instead have some user configuration that allows people to tell lazygit about a shell startup file containing their alias definitions. It's a shame that it no longer works without any configuration, but I don't see any better solution at this point. Unfortunately this isn't totally trivial either, as it requires calling shopt -s expand_aliases in the case of bash so that aliases work in a noninteractive shell.

See #4385 for an implemenation of this.

stefanhaller added a commit that referenced this issue Apr 7, 2025
- **PR Description**

In version 0.45.0 we started to use an interactive shell for running
shell commands (see #4159). The idea was that this allows users to use
their aliases and shell functions in lazygit without having to do any
additional configuration.

Unfortunately, this hasn't worked out well. For some users this resulted
in lazygit hanging in the background upon trying to return from the
shell command; we tried various fixes for this (see #4126, #4159, and
#4350), but some users still have this problem (e.g. #4320).

Also, starting an interactive shell can be a lot slower than starting a
non-interactive one, depending on how much happens in the `.bashrc` or
`.zshrc` file. For example, on my machine calling `zsh -ic true` takes
600ms, whereas `zsh -c true` takes less than 2ms. This is too high of a
price to pay for using shell aliases, especially when _all_ users have
to pay it, even those who don't care about using their aliases in
lazygit.

This PR reverts all commits related to interactive shells, and instead
introduces a different approach: we let users specify a shell aliases
file that will be sourced before running a command. The downside is that
it doesn't work transparently out of the box, but requires
configuration, and it may also require that users restructure their
shell startup file(s) if they currently only have a single big one. The
advantage is that only users who actually want to use aliases or
functions are affected, and that we can now use this mechanism not only
for shell commands, but also for custom commands and for calling the
editor (some users have asked for this in the past).
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

No branches or pull requests

3 participants