-
Notifications
You must be signed in to change notification settings - Fork 2k
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
Command substitution leaks processes if they expect input (command substitutions run in their own process group) #1362
Comments
In bash and zsh, command substitutions run in the process group of the shell. In fish, they run in their own process group. So when they try to read from stdin, they get SIGTTIN or whatever, and stop. Then fish notices they stopped and just continues on, allowing the command substitution to return. |
This has come up on the mailing list regarding fzf. |
zanchey, thanks for pointing me to this issue. So yes, as I already stated in the mailing list, I'm unable to use fzf in fish shell, and this makes fish completely out for me (fzf is SO useful) ridiculousfish, from your message I would assume this behavior can't be changed by definition, like "it's feature, not bug". Am I right? Or, will it be fixed? |
I'd also like to bump this issue. I'm seeing similar problems using the same program (fzf), but this also applies in general to any program that needs to accept input and output to stdout. /cc @junegunn |
@ridiculousfish What is the reason behind command substitutions running in their own process group? |
Is there any workaround for this ? I would like to store the result of a mysql query with |
|
Rather than calling
I'd guess that the original issue with |
Another vote for finding a way to fix this. Is it something that can reasonably be changed? |
Oh sure, we just have to fix it. |
Any pointers on where to start looking? |
The key change is to run command substitutions in the foreground process group, and to properly handle stopped jobs when doing command substitutions .It would be interesting to investigate how bash handles this - what happens if you stop (e.g. ctrl-Z) a job in a command substitution? |
This works around fish-shell/fish-shell#1362.
It swallows Ctrl-Z, but running |
@ridiculousfish Any more progress on this since March? |
No, I haven't done any work on this. I guess the first step would be to answer the question of, what should fish do if a command substitution stops instead of exits? |
Not 100% sure I understand (and my prev gif seems like its wrong to me right now) This is what I think you're saying is happening:
If that's correct, then these are potential solutions? (note that they handle this case, which is probably the common case, but they don't address the general case of a command substitution stopping)
|
This is really a UI question - what ought fish to do if a command substitution stops without exiting? Realistically I think we have three options:
It looks like bash and zsh use option 1. |
seems like a fairly acceptable 90% solution. |
+1, wait forever. Like with this issue. Just kidding. I love you. |
@maletor that probably was intended to be funny, but it comes off as condescending, maybe enjoy the laugh without sharing next time :) |
@ridiculousfish: Honestly, the UI question comes later. The first thing we ought to do is remove the reason these things are stopping in the first place - because they got SIGTTOU, because we don't allow that they use the terminal. That's the most common reason for stopping, so if we remove that, 99% of use for that UI disappears. Note also that currently I'll PR a possible fix for this now. |
The previous command ended up running all of the lines together on a single line and since the first line begins with a comment character, the entire script was effectively commented out! For additional information, see: * fish-shell/fish-shell#159 * fish-shell/fish-shell#3993 Additionally, commands run in a fish command substitution do not have access to the TTY, preventing vaulted from prompting for a password. Using piping avoids this issue all together. For additional information, see: * fish-shell/fish-shell#1362 * fish-shell/fish-shell#3922
The previous command ended up running all of the lines together on a single line and since the first line begins with a comment character, the entire script was effectively commented out! For additional information, see: * fish-shell/fish-shell#159 * fish-shell/fish-shell#3993 Additionally, commands run in a fish command substitution do not have access to the TTY, preventing vaulted from prompting for a password. Using piping avoids this issue all together. For additional information, see: * fish-shell/fish-shell#1362 * fish-shell/fish-shell#3922
I hit this issue again both yesterday and today. I didn't reread the thread, the last comment makes it look like this might be a known deficiency, but I reported it here as it seems like a regression. After some debugging, I was able to identify that the issue occurs when I pipe # ===== Demo the issue =====
# hangs (I'll press C-c)
$ bash -i -c 'echo hello >&2' | grep whatever
Job 3, '$ bash -i -c 'echo hello >&2' |…' has stopped
# works (use the command instead of the function)
$ bash -i -c 'echo hello >&2' | command grep whatever
hello
# works (remove the -i flag)
$ bash -c 'echo hello >&2' | grep whatever
hello
# ===== Other potentially useful info =====
# the grep function
$ type grep
grep is a function with definition
function grep
command grep --color=auto $argv
end
# the fish version
$ fish --version
fish, version 2.7.1
# this is as up-to-date as is installable
$ brew upgrade fish
Error: fish 2.7.1 already installed |
Oh. One other maybe relevant thing: I pressed C-c, but it really stopped the process. This kind of implies to me that it could be a different issue (b/c it resumes when I foreground it, but the the original had to be SIGKILLed, I believe). Let me know if you want me to post this as its own issue. |
Very much a different issue - this one is explicitly about command substitutions - The problem with |
Okay, made a new issue for it here |
Example:
set -l var (cat)
Expected:
$var
contains value of user input.Actual:
$var
contains an empty string, and if you runjobs
, a stopped but not reapedcat
process lingers unless you runkill -9
on it.I understand that in this case, I could use
read
, but this error would still be a thing.The text was updated successfully, but these errors were encountered: