Make tig the process group leader and automatically clean child processes #828
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
and give it the foreground connection to the TTY. This announces to the OS "I am an interactive program taking input". In the most common case, this will already have been arranged by the user's shell. This init is logically done as early as possible, as child processes inherit these characteristics. This also allows tig to easily ensure child process cleanup at exit.
jonas
reviewed
May 21, 2018
| @@ -11,6 +11,9 @@ test scripts as long as `PATH` is set to include the directories `src/` | |||
| and `test/tools`. The latter directory is where the test helper | |||
| libraries are located, the most important of which is `libtest.sh`. | |||
|
|
|||
| The test suite requires `stty -tostop` to be set in the running terminal, | |||
| which is typically the default. | |||
|
Thanks. |
|
This breaks my tight usage quite a bit. I often launch Tig from Vim, and now when I quit Tig, Vim is stopped and I have to run |
|
Without knowing what vim does, there might be no easy fix for that, and you have to revert. I am very familiar with how a daemonized process works and less familiar with an interactive process. I didn't try to look at how tig responds to The deepest motivation is really that the #736 requires this, but is not important. |
koutcher
added a commit
that referenced
this pull request
Jan 5, 2023
Make sure that a command still running in a view is properly terminated when the view is closed. This is mostly visible in the pager view. That should pretty much make #828 redundant.
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Make tig the process-group leader and give it the foreground connection to the TTY. This announces to the OS "I am the interactive program taking input".
Being process-group leader allows for a simple and robust idiom for cleaning all child processes at exit time: tig can signal HUP to the entire process group (which tig can be certain that it owns). While tig doesn't intentionally keep very long running child processes, it does depend on child processes quite a bit, so this cleanup is just good practice.
As noted in
test/README.adoc, this PR adds a minor issue: if the user setsstty tostopin their interactive shell, then tig's test suite will not be able to run. Actually, it will run, but then constantly stop again as output is generated. That is as intended forstty tostop: not to let multiple processes contend for writes to the terminal. The same problem would occur withtig-pick.However
stty tostopis not the default setting on any Unixlike so far as I knowtig-pickare edge usesIf a bug report does come in regarding
tig-pick, I'm sure there's a fix with some added complexity in eithertigortig-pick.