From 1bdd6293268f88e7e837510eb83ce07744daf207 Mon Sep 17 00:00:00 2001 From: ridiculousfish Date: Sun, 30 Jan 2022 13:15:50 -0800 Subject: [PATCH] Prevent signals from tearing multi-char bindings Say the user has a multi-char binding (typically an escape sequence), and a signal arrives partway through the binding. The signal has an event handler which enques some readline event, for example, `repaint`. Prior to this change, the readline event would cause the multi-char binding to fail. This would cause bits of the escape sequence to be printed to the screen. Fix this by noticing when a sequence was "interrupted" by a non-char event, and then rotating a sequence of such interruptions to the front of the queue. Fixes #8628 --- CHANGELOG.rst | 1 + src/input.cpp | 27 +++++++++++- src/input_common.cpp | 9 ++++ src/input_common.h | 3 ++ tests/pexpects/torn_escapes.py | 81 ++++++++++++++++++++++++++++++++++ 5 files changed, 120 insertions(+), 1 deletion(-) create mode 100644 tests/pexpects/torn_escapes.py diff --git a/CHANGELOG.rst b/CHANGELOG.rst index ab9830f47..290387d93 100644 --- a/CHANGELOG.rst +++ b/CHANGELOG.rst @@ -121,6 +121,7 @@ Interactive improvements - The Web-based configuration tool gained a number of improvements, including the ability to set pager colors. - The default ``fish_title`` prints a shorter title with shortened $PWD and no more redundant "fish" (:issue:`8641`). - Holding down an arrow key won't freeze the terminal with long periods of flashing (:issue:`8610`). +- Multi-char bindings are no longer interrupted if a signal handler enqueues an event. (:issue:`8628`). New or improved bindings ^^^^^^^^^^^^^^^^^^^^^^^^ diff --git a/src/input.cpp b/src/input.cpp index 35ad325b4..4ead777f0 100644 --- a/src/input.cpp +++ b/src/input.cpp @@ -504,6 +504,14 @@ class event_queue_peeker_t { idx_ = 0; } + /// Test if any of our peeked events are readline or check_exit. + bool char_sequence_interrupted() const { + for (const auto &evt : peeked_) { + if (evt.is_readline() || evt.is_check_exit()) return true; + } + return false; + } + /// Reset our index back to 0. void restart() { idx_ = 0; } @@ -594,7 +602,9 @@ static bool try_peek_sequence(event_queue_peeker_t *peeker, const wcstring &str) } /// \return the first mapping that matches, walking first over the user's mapping list, then the -/// preset list. \return null if nothing matches. +/// preset list. +/// \return none if nothing matches, or if we may have matched a longer sequence but it was +/// interrupted by a readline event. maybe_t inputter_t::find_mapping(event_queue_peeker_t *peeker) { const input_mapping_t *generic = nullptr; const auto &vars = parser_->vars(); @@ -627,6 +637,12 @@ maybe_t inputter_t::find_mapping(event_queue_peeker_t *peeker) peeker->restart(); } + if (peeker->char_sequence_interrupted()) { + // We might have matched a longer sequence, but we were interrupted, e.g. by a signal. + FLOG(reader, "torn sequence, rearranging events"); + return none(); + } + if (escape) { // We need to reconsume the escape. peeker->next(); @@ -671,6 +687,15 @@ void inputter_t::mapping_execute_matching_or_generic(const command_handler_t &co } peeker.restart(); + if (peeker.char_sequence_interrupted()) { + // This may happen if we received a signal in the middle of an escape sequence or other + // multi-char binding. Move these non-char events to the front of the queue, handle them + // first, and then later we'll return and try the sequence again. See #8628. + peeker.consume(); + this->promote_interruptions_to_front(); + return; + } + FLOGF(reader, L"no generic found, ignoring char..."); auto evt = peeker.next(); peeker.consume(); diff --git a/src/input_common.cpp b/src/input_common.cpp index 3120363fc..7d5b5ece6 100644 --- a/src/input_common.cpp +++ b/src/input_common.cpp @@ -255,6 +255,15 @@ void input_event_queue_t::push_back(const char_event_t& ch) { queue_.push_back(c void input_event_queue_t::push_front(const char_event_t& ch) { queue_.push_front(ch); } +void input_event_queue_t::promote_interruptions_to_front() { + // Find the first sequence of non-char events. + // EOF is considered a char: we don't want to pull EOF in front of real chars. + auto is_char = [](const char_event_t& ch) { return ch.is_char() || ch.is_eof(); }; + auto first = std::find_if_not(queue_.begin(), queue_.end(), is_char); + auto last = std::find_if(first, queue_.end(), is_char); + std::rotate(queue_.begin(), first, last); +} + void input_event_queue_t::prepare_to_select() {} void input_event_queue_t::select_interrupted() {} input_event_queue_t::~input_event_queue_t() = default; diff --git a/src/input_common.h b/src/input_common.h index ea517cd6e..c02621492 100644 --- a/src/input_common.h +++ b/src/input_common.h @@ -207,6 +207,9 @@ class input_event_queue_t { /// will be the next character returned by readch. void push_front(const char_event_t &ch); + /// Find the first sequence of non-char events, and promote them to the front. + void promote_interruptions_to_front(); + /// Add multiple characters or readline events to the front of the queue of unread characters. /// The order of the provided events is not changed, i.e. they are not inserted in reverse /// order. diff --git a/tests/pexpects/torn_escapes.py b/tests/pexpects/torn_escapes.py new file mode 100644 index 000000000..b26db8fc7 --- /dev/null +++ b/tests/pexpects/torn_escapes.py @@ -0,0 +1,81 @@ +#!/usr/bin/env python3 +import os +import signal +from pexpect_helper import SpawnedProc + + +sp = SpawnedProc() +send, sendline, sleep, expect_prompt, expect_str, expect_re = ( + sp.send, + sp.sendline, + sp.sleep, + sp.expect_prompt, + sp.expect_str, + sp.expect_re, +) +# Ensure that signals don't tear escape sequences. See #8628. +expect_prompt() + +# Allow for long delays before matching escape. +sendline(r"set -g fish_escape_delay_ms 2000") +expect_prompt() + +# Set up a handler for SIGUSR1. +sendline(r"set -g sigusr1_count 0") +expect_prompt() + +sendline( + r""" + function usr1_handler --on-signal SIGUSR1; + set sigusr1_count (math $sigusr1_count + 1); + echo Got SIGUSR1 $sigusr1_count; + commandline -f repaint; + end +""".strip().replace( + "\n", os.linesep + ) +) +expect_prompt() + +# Set up a wacky binding with an escape. +sendline(r"function wacky_handler; echo Wacky Handler; end") +expect_prompt() + +sendline(r"bind abc\edef wacky_handler") +expect_prompt() + +# We can respond to SIGUSR1. +os.kill(sp.spawn.pid, signal.SIGUSR1) +expect_str(r"Got SIGUSR1 1") +sendline(r"") +expect_prompt() + +# Our wacky binding works. +send("abc\x1bdef") +expect_str(r"Wacky Handler") +sendline(r"") +expect_prompt() + +# Now we interleave the sequence with SIGUSR1. +# What we expect to happen is that the signal is "shuffled to the front" ahead of the key events. +send("abc") +sleep(0.05) +os.kill(sp.spawn.pid, signal.SIGUSR1) +sleep(0.05) +send("\x1bdef") +expect_str(r"Got SIGUSR1 2") +expect_str(r"Wacky Handler") +sendline(r"") +expect_prompt() + +# As before but it comes after the ESC. +# The signal will arrive while we are waiting in getch_timed(). +send("abc\x1b") +sleep(0.05) +os.kill(sp.spawn.pid, signal.SIGUSR1) +sleep(0.05) +send("def") +expect_str(r"Got SIGUSR1 3") +expect_str(r"Wacky Handler") +sendline(r"") +expect_prompt()