Browse Source

Fix bugs in async code

1. Insert `-` in `echo -nE "$suggestion"`. This is necessary to prevent
   `"$suggestion"` from being treated as an option for `echo`.
2. Close file descriptors only in `_zsh_autosuggest_async_response` to
   ensure that each file descriptor is closed only once.

It's the second bug that prompted the fix. The original code in some
cases could close the same file descriptor twice. The code relied on
an invalid assumption that `_zsh_autosuggest_async_response` cannot
fire after the file descriptor is closed. Here's a demo that shows
this assumption being violated:

    () {
      emulate -L zsh

      function callback1() {
        zle -I
        emulate -L zsh -o xtrace
        : "$@"
        zle -F $fd1
        exec {fd1}>&-
        zle -F $fd2
        exec {fd2}>&-
      }

      function callback2() {
        zle -I
        emulate -L zsh -o xtrace
        : "$@"
      }

      exec {fd1} </dev/null
      exec {fd2} </dev/null
      zle -F $fd1 callback1
      zle -F $fd2 callback2
    }

And here's the output I get if the code is pasted into an interactive zsh:

    +callback1:3> : 12
    +callback1:4> zle -F 12
    +callback1:6> zle -F 13
    +callback2:3> : 13

Note that `callback2` fires after its file descriptor has been closed
by `callback1`.

This bug was the culprit of several issues filed against powerlevel10k.
In a nutshell:

1. `_zsh_autosuggest_async_request` opens a file.
2. `_zsh_autosuggest_async_request` closes the file descriptor.
3. powerlevel10k opens a file and gets the same file descriptor as above.
4. `_zsh_autosuggest_async_response` fires and closes the same file descriptor.
5. powerlevel10k encounters errors when trying to read from the file descriptor.
pull/753/head
Roman Perepelitsa 1 year ago
parent
commit
167d52e7d8
2 changed files with 120 additions and 92 deletions
  1. +59
    -45
      src/async.zsh
  2. +61
    -47
      zsh-autosuggestions.zsh

+ 59
- 45
src/async.zsh View File

@ -9,49 +9,59 @@ _zsh_autosuggest_async_request() {
typeset -g _ZSH_AUTOSUGGEST_ASYNC_FD _ZSH_AUTOSUGGEST_CHILD_PID
# If we've got a pending request, cancel it
if [[ -n "$_ZSH_AUTOSUGGEST_ASYNC_FD" ]] && { true <&$_ZSH_AUTOSUGGEST_ASYNC_FD } 2>/dev/null; then
# Close the file descriptor and remove the handler
exec {_ZSH_AUTOSUGGEST_ASYNC_FD}<&-
zle -F $_ZSH_AUTOSUGGEST_ASYNC_FD
# We won't know the pid unless the user has zsh/system module installed
if [[ -n "$_ZSH_AUTOSUGGEST_CHILD_PID" ]]; then
# Zsh will make a new process group for the child process only if job
# control is enabled (MONITOR option)
if [[ -o MONITOR ]]; then
# Send the signal to the process group to kill any processes that may
# have been forked by the suggestion strategy
kill -TERM -$_ZSH_AUTOSUGGEST_CHILD_PID 2>/dev/null
if (( _ZSH_AUTOSUGGEST_CHILD_PID )); then
kill -TERM -- $_ZSH_AUTOSUGGEST_CHILD_PID 2>/dev/null
_ZSH_AUTOSUGGEST_CHILD_PID=
fi
_ZSH_AUTOSUGGEST_ASYNC_FD=
{
# Fork a process to fetch a suggestion and open a pipe to read from it
exec {_ZSH_AUTOSUGGEST_ASYNC_FD}< <(
# Suppress error messages
exec 2>/dev/null
# Tell parent process our pid
if (( ${+sysparams} )); then
echo ${sysparams[pid]} || return
else
# Kill just the child process since it wasn't placed in a new process
# group. If the suggestion strategy forked any child processes they may
# be orphaned and left behind.
kill -TERM $_ZSH_AUTOSUGGEST_CHILD_PID 2>/dev/null
echo || return
fi
fi
fi
# Fork a process to fetch a suggestion and open a pipe to read from it
exec {_ZSH_AUTOSUGGEST_ASYNC_FD}< <(
# Tell parent process our pid
echo $sysparams[pid]
# Fetch and print the suggestion
local suggestion
_zsh_autosuggest_fetch_suggestion "$1"
echo -nE - "$suggestion"
) || return
# Fetch and print the suggestion
local suggestion
_zsh_autosuggest_fetch_suggestion "$1"
echo -nE "$suggestion"
)
# There's a weird bug here where ^C stops working unless we force a fork
# See https://github.com/zsh-users/zsh-autosuggestions/issues/364
autoload -Uz is-at-least
is-at-least 5.8 || command true
# There's a weird bug here where ^C stops working unless we force a fork
# See https://github.com/zsh-users/zsh-autosuggestions/issues/364
autoload -Uz is-at-least
is-at-least 5.8 || command true
# Read the pid from the child process
read _ZSH_AUTOSUGGEST_CHILD_PID <&$_ZSH_AUTOSUGGEST_ASYNC_FD || return
# Read the pid from the child process
read _ZSH_AUTOSUGGEST_CHILD_PID <&$_ZSH_AUTOSUGGEST_ASYNC_FD
# Zsh will make a new process group for the child process only if job
# control is enabled (MONITOR option)
if [[ -o MONITOR ]]; then
# If we need to kill the background process in the future, we'll send
# SIGTERM to the process group to kill any processes that may have
# been forked by the suggestion strategy
_ZSH_AUTOSUGGEST_CHILD_PID=-$_ZSH_AUTOSUGGEST_CHILD_PID
fi
# When the fd is readable, call the response handler
zle -F "$_ZSH_AUTOSUGGEST_ASYNC_FD" _zsh_autosuggest_async_response
# When the fd is readable, call the response handler
zle -F "$_ZSH_AUTOSUGGEST_ASYNC_FD" _zsh_autosuggest_async_response
} always {
# Clean things up if there was an error
if (( $? && _ZSH_AUTOSUGGEST_ASYNC_FD )); then
exec {_ZSH_AUTOSUGGEST_ASYNC_FD}<&-
_ZSH_AUTOSUGGEST_ASYNC_FD=
_ZSH_AUTOSUGGEST_CHILD_PID=
fi
}
}
# Called when new data is ready to be read from the pipe
@ -61,16 +71,20 @@ _zsh_autosuggest_async_response() {
emulate -L zsh
local suggestion
if (( $1 == _ZSH_AUTOSUGGEST_ASYNC_FD )); then
_ZSH_AUTOSUGGEST_ASYNC_FD=
_ZSH_AUTOSUGGEST_CHILD_PID=
if [[ $# == 1 || $2 == "hup" ]]; then
# Read everything from the fd
IFS='' read -rd '' -u $1 suggestion
fi
fi
if [[ -z "$2" || "$2" == "hup" ]]; then
# Read everything from the fd and give it as a suggestion
IFS='' read -rd '' -u $1 suggestion
zle autosuggest-suggest -- "$suggestion"
# Always remove the handler and close the fd
zle -F $1
exec {1}<&-
# Close the fd
exec {1}<&-
if [[ -n $suggestion ]]; then
zle autosuggest-suggest -- "$suggestion"
fi
# Always remove the handler
zle -F "$1"
}

+ 61
- 47
zsh-autosuggestions.zsh View File

@ -764,49 +764,59 @@ _zsh_autosuggest_async_request() {
typeset -g _ZSH_AUTOSUGGEST_ASYNC_FD _ZSH_AUTOSUGGEST_CHILD_PID
# If we've got a pending request, cancel it
if [[ -n "$_ZSH_AUTOSUGGEST_ASYNC_FD" ]] && { true <&$_ZSH_AUTOSUGGEST_ASYNC_FD } 2>/dev/null; then
# Close the file descriptor and remove the handler
exec {_ZSH_AUTOSUGGEST_ASYNC_FD}<&-
zle -F $_ZSH_AUTOSUGGEST_ASYNC_FD
# We won't know the pid unless the user has zsh/system module installed
if [[ -n "$_ZSH_AUTOSUGGEST_CHILD_PID" ]]; then
# Zsh will make a new process group for the child process only if job
# control is enabled (MONITOR option)
if [[ -o MONITOR ]]; then
# Send the signal to the process group to kill any processes that may
# have been forked by the suggestion strategy
kill -TERM -$_ZSH_AUTOSUGGEST_CHILD_PID 2>/dev/null
else
# Kill just the child process since it wasn't placed in a new process
# group. If the suggestion strategy forked any child processes they may
# be orphaned and left behind.
kill -TERM $_ZSH_AUTOSUGGEST_CHILD_PID 2>/dev/null
fi
fi
if (( _ZSH_AUTOSUGGEST_CHILD_PID )); then
kill -TERM -- $_ZSH_AUTOSUGGEST_CHILD_PID 2>/dev/null
_ZSH_AUTOSUGGEST_CHILD_PID=
fi
# Fork a process to fetch a suggestion and open a pipe to read from it
exec {_ZSH_AUTOSUGGEST_ASYNC_FD}< <(
# Tell parent process our pid
echo $sysparams[pid]
_ZSH_AUTOSUGGEST_ASYNC_FD=
# Fetch and print the suggestion
local suggestion
_zsh_autosuggest_fetch_suggestion "$1"
echo -nE "$suggestion"
)
# There's a weird bug here where ^C stops working unless we force a fork
# See https://github.com/zsh-users/zsh-autosuggestions/issues/364
autoload -Uz is-at-least
is-at-least 5.8 || command true
{
# Fork a process to fetch a suggestion and open a pipe to read from it
exec {_ZSH_AUTOSUGGEST_ASYNC_FD}< <(
# Suppress error messages
exec 2>/dev/null
# Tell parent process our pid
if (( ${+sysparams} )); then
echo ${sysparams[pid]} || return
else
echo || return
fi
# Read the pid from the child process
read _ZSH_AUTOSUGGEST_CHILD_PID <&$_ZSH_AUTOSUGGEST_ASYNC_FD
# Fetch and print the suggestion
local suggestion
_zsh_autosuggest_fetch_suggestion "$1"
echo -nE - "$suggestion"
) || return
# There's a weird bug here where ^C stops working unless we force a fork
# See https://github.com/zsh-users/zsh-autosuggestions/issues/364
autoload -Uz is-at-least
is-at-least 5.8 || command true
# Read the pid from the child process
read _ZSH_AUTOSUGGEST_CHILD_PID <&$_ZSH_AUTOSUGGEST_ASYNC_FD || return
# Zsh will make a new process group for the child process only if job
# control is enabled (MONITOR option)
if [[ -o MONITOR ]]; then
# If we need to kill the background process in the future, we'll send
# SIGTERM to the process group to kill any processes that may have
# been forked by the suggestion strategy
_ZSH_AUTOSUGGEST_CHILD_PID=-$_ZSH_AUTOSUGGEST_CHILD_PID
fi
# When the fd is readable, call the response handler
zle -F "$_ZSH_AUTOSUGGEST_ASYNC_FD" _zsh_autosuggest_async_response
# When the fd is readable, call the response handler
zle -F "$_ZSH_AUTOSUGGEST_ASYNC_FD" _zsh_autosuggest_async_response
} always {
# Clean things up if there was an error
if (( $? && _ZSH_AUTOSUGGEST_ASYNC_FD )); then
exec {_ZSH_AUTOSUGGEST_ASYNC_FD}<&-
_ZSH_AUTOSUGGEST_ASYNC_FD=
_ZSH_AUTOSUGGEST_CHILD_PID=
fi
}
}
# Called when new data is ready to be read from the pipe
@ -816,18 +826,22 @@ _zsh_autosuggest_async_response() {
emulate -L zsh
local suggestion
if (( $1 == _ZSH_AUTOSUGGEST_ASYNC_FD )); then
_ZSH_AUTOSUGGEST_ASYNC_FD=
_ZSH_AUTOSUGGEST_CHILD_PID=
if [[ $# == 1 || $2 == "hup" ]]; then
# Read everything from the fd
IFS='' read -rd '' -u $1 suggestion
fi
fi
if [[ -z "$2" || "$2" == "hup" ]]; then
# Read everything from the fd and give it as a suggestion
IFS='' read -rd '' -u $1 suggestion
zle autosuggest-suggest -- "$suggestion"
# Always remove the handler and close the fd
zle -F $1
exec {1}<&-
# Close the fd
exec {1}<&-
if [[ -n $suggestion ]]; then
zle autosuggest-suggest -- "$suggestion"
fi
# Always remove the handler
zle -F "$1"
}
#--------------------------------------------------------------------#

Loading…
Cancel
Save