bash-completion
bash-completion copied to clipboard
Aborting a completion attempt leaves `set -o noglob` in effect
Describe the bug
Certain completions turn off globbing with set -o noglob
, perform some operation, and then attempt to reset globbing back to its original state. See for example https://github.com/scop/bash-completion/blob/36ceb272ddf7ef70b7fa79c5c3686080b1510054/bash_completion#L758-L763 but this pattern is used in more than one place.
Unfortunately if the user presses Ctrl+C in the middle of this then the reset code is never run and the shell is left with set -o noglob
in effect. This will likely be very disconcerting for the user. For instance, running ls *
reports ls: cannot access *: No such file or directory
. That's the message you'd normally expect to see if the directory is empty but now you'll see it reported for all directories, giving the impression that all of your files have been deleted.
A user could recover from this by running set +o noglob
but they'd have to realize what was wrong first. I'm assuming in most cases people will just think their terminal is "broken" and be forced to close it and open a new one.
See also #691 .
To reproduce
You'll need a directory somewhere that's very slow to enumerate. One option would be to mount an NFS directory from some slow, distant machine. Another would be to create a local directory with an absurdly large number of files in it and have a cold cache (possibly something like echo 3 > /proc/sys/vm/drop_caches
might help on Linux). Or maybe https://serverfault.com/a/954175. I can personally reproduce this with a large directory on NFS.
# Verify that globbing is turned on to start
$ shopt -po noglob
set +o noglob
$ cd /path/to/your/directory/<TAB><Ctrl+C>
# Globbing is now unexpectedly turned off
$ shopt -po noglob
set -o noglob
Expected behavior
The noglob
setting should retain the previous value.
Versions (please complete the following information)
- [X] Operating system name/distribution and version: CentOS Linux release 7.9.2009 (Core)
- [X] bash version,
echo "$BASH_VERSION"
: 4.2.46(2)-release - [X] bash-completion version,
(IFS=.; echo "${BASH_COMPLETION_VERSINFO[*]}")
: This command outputs "2.11.0", but I'm actually using git commit 36ceb272ddf7ef70b7fa79c5c3686080b1510054.
Additional context
Maybe this could be handled by running the compgen in a subshell so that the setting would not have to be remembered and then explicitly restored?
Debug trace
I'm going to avoid including entire thing for now since it is large and I don't think it will help explain the problem. If you still need it, please tell me and I'll send the full one. It ends with this, the compgen
having been interrupted by Ctrl+C:
+ local -a toks
+ local reset arg=-d
+ [[ -d == -d ]]
++ shopt -po noglob
+ reset='set +o noglob'
+ set -o noglob
+ toks=($(compgen -d -- "${cur-}"))
++ compgen -d -- /path/to/your/directory/
^C
$
I can't reproduce the problem on a different system that uses bash 5.1. I believe that's because of a recent change to bash, introduced in v5.1, that masks SIGINT during the execution of compgen
: https://github.com/bminor/bash/blob/9439ce094c9aa7557a9d53ac7b412a23aa66e36b/subst.c#L6542. This makes the code in bash-completion work as expected but has the downside that you can't abort the completion and have to wait for it to finish no matter how long it takes.
Hmph, yeah, I can see how that would happen.
I wonder if a simple trap "$reset" RETURN
or a derivative would be a pattern we could use to avoid this.
Semi-related: https://github.com/scop/bash-completion/pull/739
I wonder if a simple
trap "$reset" RETURN
or a derivative would be a pattern we could use to avoid this.
I have thought the same thing after I first read this issue and tried it, but RETURN
doesn't seem to be invoked when the completion is canceled by SIGINT
. Currently, I don't come up with a good solution for this issue.
- The subshells suggested by @abbeyj would have additional fork costs, and also it makes it hard to change the state of the parent shell e.g. when a completion function wants to cache the completion results, etc.
-
trap '$reset' INT
might work, but the implementation would become complicated including saving/restoring of the original INT traps. Also, it only solves the problem for SIGINT; it doesn't solve the problem for the general signal. - In ble.sh, I turn off all the signals caused by user inputs by changing the terminal settings by
stty
. - Another solution might be to give up immediately restoring the state, but to restore the state on the next call of the completion. We record the state of whether we are currently adjusting the shell options or not in a global variable. We can check that variable at the start of the completion.
We may also check the state on the execution of user commands by setting the
DEBUG
trap as bash-preexec does, but I'd say it's a fragile hack and hard to implement it in a transparent way to other configurations and user commands.
RETURN
doesn't seem to be invoked when the completion is canceled bySIGINT
Would trap '$reset' INT RETURN
"fix" the general signal part of this problem? Managing original traps to those would still be needed I guess.
give up immediately restoring the state, but to restore the state on the next call of the completion
I'm not sure I follow, but wouldn't this leave the altered state in effect for the user's shell until the next call of the completion?
RETURN
doesn't seem to be invoked when the completion is canceled bySIGINT
Would
trap '$reset' INT RETURN
"fix" the general signal part of this problem? Managing original traps to those would still be needed I guess.
I think the problem with recovering the options could be solved, but then there arises another problem of the completion that can take a long time not being able to be canceled. Specifically, OP describes the case where the filename completion is attempted in a directory with many files on remote filesystems.
sidenote: implementation in ble.sh
I now remembered that I have implemented in ble.sh more complicated processing for this issue of slow filename completions. I attempt the pathname expansions (aka glob expansions) in a background subshell, and wait for its termination or user inputs in the parent shell. The subshell is killed when there are any user inputs before its termination. In this case, the result needs to be passed to the parent shell after the completion of the pathname expansions. Here, the next problem is how to effectively pass the expansion results to the parent shell. It turned out that result=($very_long_list)
can be as slow as the expansion itself and this cannot be canceled when we suppress the signals. I instead parse the result of declare -p result
by awk
. write a NUL-separated list of result elements to file, and read it using mapfile
in the latest version of Bash. Adjustments are needed for older versions of Bash where mapfile
doesn't support an option -d ''
for the NUL delimiter.
Other completions that might take a long time are handled in a similar way separately for each case.
give up immediately restoring the state, but to restore the state on the next call of the completion
I'm not sure I follow, but wouldn't this leave the altered state in effect for the user's shell until the next call of the completion?
Right. So this is merely a partial solution in case all the other solutions are unavailable.
It turned out that the ERR
trap seems to be reliably called in Bash 4.4+, though it doesn't work in Bash 4.2 and 4.3.
$ bash-dev --norc
$ function f1 { trap 'echo f1/ERR:$FUNCNAME' ERR; f2; echo never; false; }; function f2 { sleep 1; }; f1
^Cf1/ERR:f1
$ exit
$ bash-4.4 --norc
$ function f1 { trap 'echo f1/ERR:$FUNCNAME' ERR; f2; echo never; false; }; function f2 { sleep 1; }; f1
^Cf1/ERR:f1
$ exit
$ bash-4.3 --norc
$ function f1 { trap 'echo f1/ERR:$FUNCNAME' ERR; f2; echo never; false; }; function f2 { sleep 1; }; f1
^C
$
edit: Anyway, the user's ERR trap needs to be saved and restored.
I think another way to make sure to recover the original settings is to recheck them before running the user commands.
- For example, this can be done by the
DEBUG
trap asbash-preexec.sh
does. A possible problem with this approach is that a robust implementation could be very complicated as we observe inbash-preexec.sh
. Also, it can conflict or interfere with the otherDEBUG
trap set bybash-preexec.sh
, Starship, etc. Another thing is that it may slow down the execution of the user commands because theDEBUG
trap is invoked for every user command at the top level. - Another approach is to send a signal from the subshell in
PS0
to the main shell as suggested in https://github.com/rcaloras/bash-preexec/issues/28#issuecomment-944940637. I'm not sure how much this is robust. tycho-kirchner/shournal seems to usePS0
for an approximate preexec, but it doesn't use a signal to do works in the main shell. ThisPS0
approach can be used in Bash 4.4+ but is not available in Bash 4.2 and 4.3. A possible conflict could occur in thePS0
value and the choice of the signal. - Another approach is to hook into the readline keybindings to
accept-line
,operate-and-next
, andedit-and-execute
as I described in https://github.com/scop/bash-completion/issues/901#issuecomment-1466093520. This could also conflict with the user's settings, but we can list the user's keybindings toaccept-line
, etc. bybind -p
and modify them.
Note: All of these approaches might be broken by the user's other settings overwriting DEBUG
, PS0
, and keybindings.
We might combine one of the above with #739 _comp_finalize
. We might also handle $_
in #901 with the same approach.