Skip to content
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

warn if exiting fish when background jobs exist #3497

Closed
maletor opened this issue Oct 27, 2016 · 21 comments
Closed

warn if exiting fish when background jobs exist #3497

maletor opened this issue Oct 27, 2016 · 21 comments
Assignees
Labels
Milestone

Comments

@maletor
Copy link

@maletor maletor commented Oct 27, 2016

If there are any running jobs in the shell, we should not be closing without a warning that the jobs are lost into the greater process list hierarchy. I don't think it should matter if the jobs running are background or foreground, but tell me why I'm wrong.

@krader1961
Copy link
Contributor

@krader1961 krader1961 commented Oct 27, 2016

How do you exit the shell if you have a job running in the foreground? As for the case of jobs in the background a warning by itself doesn't seem very useful. Presumably you also intend that fish ask the user if they really want to exit with jobs in the background and require an answer in the affirmative before actually exiting. I'm ambivalent about adding that behavior since it's paternalistic and there's a good chance I deliberately want the backgrounded jobs to continue running.

@floam
Copy link
Member

@floam floam commented Oct 27, 2016

We kind of are a paternalistic shell, though I prefer "friendly".

@maletor
Copy link
Author

@maletor maletor commented Oct 27, 2016

Nobody really wants those background jobs to keep running. Managing processes is better served via a systemd unit. The friendly thing to do would seem to be to display a warning (no ask) and quit if exiting a second time.

What do bash and zsh do? Do people use htop that much? It's not even useful when chromium children is 99% of what's in there.

@krader1961
Copy link
Contributor

@krader1961 krader1961 commented Oct 27, 2016

I think saying "nobody really wants..." is a bit strong. I certainly want that behavior but then I'm a grey beard who has been using UNIX three decades and am definitely not the typical fish user. I switched from zsh and remember it asking me to type exit or [ctrl-D] a second time if I had background jobs. That's equivalent to asking the user to answer in the affirmative that they really want to exit. I have no objection to implementing the zsh like behavior. It's a reasonable safety measure for people not experienced with UNIX process management.

@krader1961 krader1961 removed the RFC label Oct 27, 2016
@krader1961 krader1961 added this to the fish-future milestone Oct 27, 2016
@krader1961 krader1961 changed the title background jobs and closing session warn if exiting fish when background jobs exist Oct 27, 2016
@faho
Copy link
Member

@faho faho commented Oct 27, 2016

Just to be clear, this is already a thing, in fish, for suspended jobs. It seems right to extend it to stopped ones as well.

@maletor
Copy link
Author

@maletor maletor commented Oct 27, 2016

I agree.

@maletor
Copy link
Author

@maletor maletor commented Oct 27, 2016

I think this regressed in #111.

ZSH kills background jobs by default when exiting. The user is forced to use disown or nohup to get rid of the connection to the process. This ensures there are no dangling processes the user is not aware of.

If we can maintain the same behavior as ZSH in this regard that would be great.

@krader1961
Copy link
Contributor

@krader1961 krader1961 commented Dec 18, 2016

Fish was deliberately changed 1.6 years ago to not kill background jobs in issue #1771 because two people expressed displeasure with the behavior. We can, and should, display a warning and require the user to type exit twice if there are any background or stopped jobs. Whether we should revert the change made to resolve issue #1771 is open for debate. Personally I think the answer is yes. A user should be required to use nohup or a similar mechanism if they want to keep a job from being killed when the shell exits or the tty is closed. What's funny is one of the users arguing for the current behavior quoted the bash man page in this comment, says that "So [sending] SIGHUP is correct", then proceeds to argue that fish should not behave like bash in this regard.

@zanchey: You made the change that resulted in the behavior being discussed. Care to comment?

@krader1961 krader1961 self-assigned this Dec 18, 2016
@krader1961 krader1961 modified the milestones: fish 2.5.0, fish-future Dec 18, 2016
@krader1961
Copy link
Contributor

@krader1961 krader1961 commented Dec 18, 2016

I am still interested in feedback from @zanchey, or anyone else, as to why the current behavior is preferable to bash/zsh/etcetera regarding background jobs of an interactive fish shell. Nonetheless I am going to create a PR that makes fish behave like those shells and reverts the change made in response to issue #1771.

@floam
Copy link
Member

@floam floam commented Dec 18, 2016

I like that.

@zanchey
Copy link
Member

@zanchey zanchey commented Jan 5, 2017

In an attempt to defend a change from a couple of years ago, I think what I was hoping to do is file off one of the more surprising behaviours of POSIX shells - the sudden disappearance of processes which aren't obviously under the shell's direct control. Fish is supposed to be friendly, but (having thought about it some more), I think friendliness involves explaining what is happening rather than insulating users from potentially-unwanted behaviour. I've made some more comments in #3658.

@spider-mario
Copy link

@spider-mario spider-mario commented Feb 3, 2017

Nobody really wants those background jobs to keep running. Managing processes is better served via a systemd unit.

Er, I do. This change breaks my workflow (obligatory https://xkcd.com/1172/): when I start my session, I tend to run firefox & google-chrome & clementine. nohup is a bit overkill (and it’s quite a bit more verbose, especially if I don’t want a nohup.out file in my home directory), to say nothing of a systemd unit.

I could not find a fish equivalent to zsh’s disown. Is there one? While it would be worse for me that the old behavior from fish 2.4 (which I thought was much friendlier than bash’s or zsh’s), it would still be miles better than the new behavior in fish 2.5 (for me still).

The warning does say that the processes will be killed with a second ^D, but it does not say how to close the terminal without killing them. Until this is easily doable (e.g. by having an equivalent to disown), I think that this change should be reconsidered.

@krader1961
Copy link
Contributor

@krader1961 krader1961 commented Feb 3, 2017

@spider-mario: Use nohup. If you redirect stdout you won't get a nohup.out file. for example:

nohup sleep 30 </dev/null >/dev/null 2>&1 &

Thanks for the feedback but this won't be reverted to the old behavior. The current behavior is friendlier to people who aren't shell power users and are surprised to find processes they backgrounded still running after they exit the shell. And while fish deliberately breaks from Posix 1003.1 in several ways there is no good reason to be different with respect to this particular behavior. Power users like yourself can easily work around it. For example create your own disown command by creating a file named ~/.config/fish/functions/disown.fish with this content:

function disown
    nohup $argv </dev/null >/dev/null 2>&1 &
end
@spider-mario
Copy link

@spider-mario spider-mario commented Feb 3, 2017

I know about redirecting the output of nohup, which is why I said “it’s quite a bit more verbose, especially if I don’t want a nohup.out file in my home directory”. It also doesn’t work if I want to see the output in the terminal while it’s open. Even without necessarily reverting back to the old behavior, it would be nice to be able to disown “after-the-fact”. Would that be possible?

@krader1961
Copy link
Contributor

@krader1961 krader1961 commented Feb 3, 2017

@spider-mario: Please open an enhancement request.

@krader1961
Copy link
Contributor

@krader1961 krader1961 commented Feb 3, 2017

Turns out disown has already been requested in issue #2810.

@zanchey
Copy link
Member

@zanchey zanchey commented Feb 4, 2017

😢

@floam
Copy link
Member

@floam floam commented Feb 4, 2017

Why the long face?

@GReagle
Copy link
Contributor

@GReagle GReagle commented Feb 6, 2017

I would like some advice on how to deal with this change in behavior.

I have a very strong habit and very frequently start GUI applications from fish doing such things as emacs &; firefox &; pavucontrol &; mupdf mydoc.pdf &
I used bash before fish so before this recent change these applications always kept running when I exited the shell. It is my understanding that fish is not going to go back to the old behavior no matter how much I complain.

It is also my understanding that that the recommended course of action is to use nohup or define a fish function that uses nohup to launch these GUI applications. However I can't use tab completion that way because the real command is the second word on the command line. Any advice is welcome.

@krader1961
Copy link
Contributor

@krader1961 krader1961 commented Feb 7, 2017

I used bash before fish so before this recent change these applications always kept running when I exited the shell.

Your sentence is ambiguous but, yes, that is true. The problem is that if you kill the shell via SIGHUP then those processes are not kept running. Even in bash. The inconsistency causes more problems than it solves. Fish used to be consistent in how it handled the two cases, then wasn't consistent for 1.5 years, and is once again consistent.

The long term solution is for someone to take the time to address issue #2810 and implement a disown command. Until that happens you can either use nohup or the double-fork trick: fish -c 'emacs &'. Either one can be implemented as a function. You can address the completion problem by using the same trick as the sudo completion:

complete -c sudo -d "Command to run" -x -a "(__fish_complete_subcommand_root -u -g)"

Although you'll probably want the __fish_complete_subcommand.fish function.

@GReagle
Copy link
Contributor

@GReagle GReagle commented Feb 8, 2017

Thanks very much @krader1961, that is very helpful. For the potential benefit of others, here is exactly what I did. I created a function called do (stands for "disown", which might be a real fish command one day):

function do
        nohup $argv >/dev/null 2>&1 &
end

and I added this code to my config.fish:
complete -c do -x -a "(__fish_complete_subcommand -u -g)"

@github-actions github-actions bot locked as resolved and limited conversation to collaborators Apr 18, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Linked pull requests

Successfully merging a pull request may close this issue.

7 participants
You can’t perform that action at this time.