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

read followed by ctrl+c terminates bash shell-session [bash 5.1 fixed] #627

Closed
vanillaSprinkles opened this issue Apr 29, 2020 · 18 comments
Closed
Labels

Comments

@vanillaSprinkles
Copy link

Describe the bug

With direnv hook enabled in a shell-session, calling read ans followed by a ctrl+c kills both read and current shell-session (occurs on both Mac and Archlinux systems)

Further testing on Archlinux:

  • root user was unable to reproduce
  • all other users dropped out (even with 0 modifications to .bashrc, .profile, ect

Exit code of ctrl+c fromread ans (on archlinux) yields 130, and is followed through to shell.

To Reproduce
Steps to reproduce the behavior:

  1. in bash shell-session:
    eval "$(direnv hook bash)"
  2. send key sequence
    <ctrl+c>
  3. notice current shell-session is terminated

Expected behavior
bash shell-session should not terminate.

Environment

  • OS: mac os, archlinux
  • Shell: "mac" bash 3, gnu bash 5
  • Direnv version 2.21.1

Additional context
Ran the test as root on archlinux; it did not terminate my shell-session.

@zimbatm
Copy link
Member

zimbatm commented Apr 30, 2020

It looks like a bug in bash to me. I can reproduce the same behavior by setting this in my shell:

$ PROMPT_COMMAND="trap -- '' SIGINT; sleep 0.1; trap SIGINT;"
$ read anc
^C
<exits>

@zimbatm
Copy link
Member

zimbatm commented Apr 30, 2020

I can reproduce this both in bash 4.4.23(1) and 5.0.16(1)

@carlpett
Copy link

I have this problem as well, on Fedora.
FC31 with bash-5.0.11-1.fc31.x86_64/direnv-2.20.1-1.fc31.x86_64 worked
FC32 with bash-5.0.11-2.fc32.x86_64/direnv-2.21.2-1.fc32.x86_64 exhibits the problem mentioned above. Also happens with loops and similar constructs, so I wonder if there's something different about builtins?

@carlpett
Copy link

Look at the diff in the hook between the two versions I had here
The addition of trap on SIGINT seems like a pretty likely suspect. And replacing eval "$(direnv hook bash)" in my .bashrc with the old hook indeed makes it work.

@zimbatm
Copy link
Member

zimbatm commented May 26, 2020

Removing the trap means that direnv won't be able to record cancelled evaluation.

@carlpett
Copy link

Okay, but this is how it was before 2.21 when #555 was merged, right? I'm not sure what direnv does with this recording functionality, but exiting the shell when interrupting a builtin doesn't seem like a great tradeoff to get it?

@zimbatm
Copy link
Member

zimbatm commented May 26, 2020

In cases where the .envrc loads a big dependency, for example it's fetching something from the Internet. The user might want to be able to cancel that download. Without recording that cancellation, the download will be re-triggered on the next prompt. At this point the only solution for the user to escape this is to close the terminal.

@carlpett
Copy link

Right, and that sounds like a good thing to avoid. But isn't quitting the terminal at any time during the entire session worse? I would expect this would be a magnitudes more common situation?

Anyway, I'm not trying to argue that such functionality isn't good and desired! I just initially got the impression that the exit-the-shell behaviour was considered an okay tradeoff rather than a bug, and wanted to highlight that at least for certain users (me) it certainly breaks the day-to-day usage pretty badly.

@zimbatm
Copy link
Member

zimbatm commented May 27, 2020

I'd rather find a solution that works for both scenarios. Personally I am never hitting that issue but encounter the cancel one quite a lot.

Current workaround:

Replace eval "$(direnv hook bash)" with

_direnv_hook() {
  local previous_exit_status=$?;
  eval "$(direnv export bash)";
  return $previous_exit_status;
};
if ! [[ "${PROMPT_COMMAND:-}" =~ _direnv_hook ]]; then
  PROMPT_COMMAND="_direnv_hook${PROMPT_COMMAND:+;$PROMPT_COMMAND}"
fi

@zimbatm
Copy link
Member

zimbatm commented May 27, 2020

side note: zsh is behaving properly

@barlik
Copy link

barlik commented May 28, 2020

I've been seeing this exact issue for a couple of days now. My terminal was unexpectedly closed after starting fzf and then cancelling it by pressing ^C.

Finally got some time to look at it and found the culprit in trap commands inside _direnv_hook().

Pressing Ctrl-C during any subshell evaluation terminates the shell.

$ cat bash.rc  
eval "$(direnv hook bash)"

$ bash --rcfile bash.rc 
bash$ echo $PROMPT_COMMAND 
_direnv_hook
bash$ $(sleep 10) # pressing ^C during those 10 seconds will terminate the shell
^C
$ # inner shell terminated

Another problem with the the hook is that it's overwriting any user defined SIGINT traps.

$ trap 'echo 1' SIGINT
$ trap
$ trap 'echo 1' EXIT
$ trap
trap -- 'echo 1' EXIT

@fdr
Copy link

fdr commented Jul 30, 2020

So is this a bash bug after all? Should I switch to zsh? Kind of a drag to carefully use old releases, though it's made a lot easier with asdf-vm. I'm on Fedora 32.

@zimbatm
Copy link
Member

zimbatm commented Jul 30, 2020

Yep, it's a bash bug. Not sure if it has been reported yet.

@roberth
Copy link

roberth commented Oct 21, 2020

I've reported the problem yesterday before finding this issue. https://savannah.gnu.org/support/index.php?110335

@roberth
Copy link

roberth commented Dec 21, 2020

Chet Ramey has released bash 5.1 which is supposed to fix this issue.

@lrworth
Copy link

lrworth commented Dec 22, 2020

Anyone using nixpkgs who wants this fixed real bad like I did: NixOS/nixpkgs#107354 (bash: 5.0 -> 5.1)

@vanillaSprinkles
Copy link
Author

Sorry I dropped the ball on following up on a ton of this; thanks @roberth for reporting to bash directly. it is fixed in bash 5.1; but folks using mac's bash 3.x (though I have not tested on the latest-and-greatest-mac-update) will potentially still encounter the problem (even with latest direnv).

Using the workaround above #627 (comment) (removing the trap) does allow mac-bash-3.x to work, though I think that is a minor edge-case.

Thanks!

@vanillaSprinkles vanillaSprinkles changed the title read followed by ctrl+c terminates bash shell-session read followed by ctrl+c terminates bash shell-session [bash 5.1 fixed] May 4, 2021
sdt added a commit to sdt/.dotfiles that referenced this issue May 18, 2021
See: direnv/direnv#627

Apparently fixed in bash 5.1

If I hit ^C in uselect, this would exit the shell. This was driving me
nutso!
@shangxiao
Copy link

Just noting here my seemingly related issue which was also fixed by upgrading bash:

I had this annoying problem that simply holding down ctrl-c in bash eventually ended up killing the session. Narrowed it down to direnv then found this issue. Upgrading to latest bash seems to have fixed it 👍

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Development

No branches or pull requests

8 participants