jonas / tig

Text-mode interface for git
https://jonas.github.io/tig/
GNU General Public License v2.0
12.46k stars 611 forks source link

100% CPU when closing terminal window before quitting tig #164

Closed bsander closed 6 years ago

bsander commented 11 years ago

If I close my terminal window which has a still running instance of tig in it, tig keeps running in the background and climbs up to 100% CPU usage.

I'm on a mac, using iTerm2 and zsh.

jonas commented 11 years ago

Could you try with bash and see if this still happens?

bsander commented 11 years ago

Hmm, apparently this only happens with ZSH, and for me only with the version that ships with OSX.8 (which is zsh 4.3.11 (i386-apple-darwin12.0))

jonas commented 11 years ago

I've tried to reproduce this on OS X 10.8.4 (i386-apple-darwin12.4.0) by manually compiling zsh-4.3.11 and launching it inside iTerm, but didn't succeed. I am not sure if it could be related with the ZSH configuration, since I am using an empty ~/.zshrc.

The only way I see right now is to compile tig with debugging enabled (make clean all-debug) and then connecting to the tig process running at 100% using GDB to see what is going on.

jonas commented 10 years ago

It could be some bug with the bash tig completion file if you loaded that via zsh's compat mode. Anyway, I need more information to find the source of this. Please feel free to reopen this issue if you find the time and want to help fix this.

yegeniy commented 9 years ago

Happens to me fairly consistently with fish (via Terminal) when I leave tig on for a while. (OS X 10.10.3, but was also happening with 10.8 and 10.9)

jonas commented 9 years ago

OK, I'll try to reproduce when I get some time.

ghost commented 8 years ago

I can also confirm, happened with fish shell here as well.

davidfuzju commented 7 years ago

zsh 5.2 (x86_64-apple-darwin16.0) same here

madmax commented 6 years ago

Any news on this issue?

jonas commented 6 years ago

@madmax As I am using neither zsh nor fish any help figuring out what causes this would be much appreciated.

yegeniy commented 6 years ago

~I haven't noticed this with fish in a long, long time.~

EDIT: Just happened again.

koutcher commented 6 years ago

The problem comes from the shell configuration rather than from tig itself as when you close the window, all processes run from the terminal are supposed to receive a hangup signal.

With zsh, this won't happen if you have the command "setopt nohup" in your profile, and you end up with a tig process using 100% CPU most probably because it's looking for its closed tty.

And from fish issues you can see that, at some points, fish had some problems with killing child processes.

jonas commented 6 years ago

Thanks @koutcher for the explanation. I can reproduce by opening a new terminal, running nohup tig and closing the terminal window.

Will look into where there might be a missing error check in the event loop.

jonas commented 6 years ago

Thanks everybody for the feedback. I've implemented a workaround to detect busy loops since I was not able to find a way to determine whether the TTY was lost . Please try it out.

thexhr commented 6 years ago

Hi @jonas,

the busy loop detection kills my tig process whenever I want to browse a large git repo, e.g. like the OpenBSD one (https://github.com/openbsd/src). tig works fine here with smaller repos.

I am running tig Version 2.3.1 on OpenBSD amd64 6.2-current. I upgraded yesterday from 2.3.0 to 2.3.1 and suddenly browsing large repos does not work any longer.

Let me know if I can provide you further info.

thexhr commented 6 years ago

I assume it hit the busy loop detection here since I want to open up a really large repo stored on OpenBSD's Fast File System (FFS) which is not as fast as other file systems. Loading the whole git repo in tig here takes about 5s wall-clock time.

jonas commented 6 years ago

Thanks for reporting this. The workaround is terrible, I might end up removing it again. Will try to get a new release out ASAP.

jonas commented 6 years ago

I've released 2.3.2 just now. Sorry for the noise.

Busy loop detection is now only triggered when no views are loading.

thexhr commented 6 years ago

I can confirm that 2.3.2 fixes my issue. Thanks @jonas for your quick help, much appreciated!

jonas commented 6 years ago

Thanks for testing.

ThomasAdam commented 6 years ago

I think this busy loop detection is probably the wrong idea. Surely, we should simply exit on SIGHUP since that's probably the right thing to do anyway -- even with nohup, since at that point you're not going to be resuming tig on a tty anyway. Perhaps:

diff --git a/src/tig.c b/src/tig.c
index 4ae62e2..828c101 100644
--- a/src/tig.c
+++ b/src/tig.c
@@ -634,6 +634,15 @@ handle_mouse_event(void)
  * (f86be659718c0cd0a67f88b42f07044c23d0d028).
  */

+void
+sighup_handler(int sig)
+{
+   if (die_callback)
+       die_callback();
+
+   exit (EXIT_SUCCESS);
+}
+
 #ifdef DEBUG
 void
 sigsegv_handler(int sig)
@@ -725,6 +734,9 @@ main(int argc, const char *argv[])
    if (signal(SIGPIPE, SIG_IGN) == SIG_ERR)
        die("Failed to setup signal handler");

+   if (signal(SIGHUP, sighup_handler) == SIG_ERR)
+       die("Failed to setup signal handler");
+
 #ifdef DEBUG
    if (signal(SIGSEGV, sigsegv_handler) == SIG_ERR)
        die("Failed to setup signal handler");

Untested, but demonstrates the idea.

jonas commented 6 years ago

I agree it is a terrible hack. And would be open to remove the workaround and mark the issue as "won't fix".

My understanding is that Tig never received the SIGHUP signal because of the way the shell is set up. I tried to see if it was possible to detect that the TTY has been closed, but didn't find a way to do that.

ThomasAdam commented 6 years ago

I couldn't get things into a situation where SIGHUP wasn't sent, and the only way to check the TTY has been closed if the FD is set to -1. There aren't any other ways to check the TTY -- the FD will always remain valid in this condition.

But certainly the SIGHUP approach seems to be the correct thing to do here.

jonas commented 6 years ago

You are right. SIGHUP is passed when running nohup tig and closing the terminal.

Thanks @ThomasAdam. Meh, I don't know what I was thinking. 🤦‍♂️

koutcher commented 6 years ago

That is a bit strange because the purpose of nohup is normally to intercept the HUP signal... It doesn’t hurt to handle HUP properly but I doubt it will solve the issue completely.

Indeed, with tig compiled from master, if you start zsh, do a setopt nohup, start tig and close the window you still end with a tig process using 100% CPU.

On the other hand I’m not sure there’s a need to do anything as users who are bothered by this problem can fix their shell configuration (i.e. remove setopt nohup or add a setopt hup if their zsh was compiled to be in nohup by default).

stefanchrobot commented 6 years ago

Hi @jonas, would it be possible to release a new version with the fix? I'm getting hit by "tig: Busy loop detected" a lot. There's only one issue blocking 2.3.3, but it seems like a big one.

jonas commented 6 years ago

@stefanchrobot Released 2.3.3 based on current master.

stefanchrobot commented 6 years ago

Thanks!

chros73 commented 6 years ago

@jonas thanks! Can you also "publish" the tar.gz and tar.gz.md5 files as well (as with e.g. 2.3.2)? (It may seem to be a silly request but build script is failing without those.)

jonas commented 6 years ago

Done, sorry about that.

stefanchrobot commented 6 years ago

I've been using the 2.3.3 release for a few days now and it's great! Thanks for fixing this!