Compare commits

...

3 Commits

Author SHA1 Message Date
Johannes Altmanninger
65556ac2ae Increase grace period for decoding escape sequences
Historically, fish has treated input bytes [0x1b, 'b'] as alt-b (rather than
"escape,b") if the second byte arrives within 30ms of the first.

Since we made builtin bind to match key events instead of raw byte sequences
key events (as per the kitty keyboard protocol), we have another place where
we do similar disambiguation: when we read keys such as alt-left ("\e[1;3D"),
we only consider bytes to be part of this sequence if stdin is immediately
readable (actually after a 1ms timeout since e1be842 (Work around
torn byte sequences in qemu kbd input with 1ms timeout, 2025-03-04)).

This is technically wrong but has worked in practice (for Kakoune etc.).

Issue #11668 reports two issues on some Windows terminals with a remote fish shell:
- the "bracketed paste finished" sequence may be split into multiple packets,
  which causes a delay of > 1ms between individual bytes being readable.
- AutoHotKey scripts simulating seven "left" keys result in sequence tearing
  as well.

Try to fix the paste case by increasing the timeout when parsing escape
sequences.

Also increase the timeout for terminals that support the kitty keyboard
protocol.  The user should only notice the delay caused by the timeout after
presses one of escape,O, escape,P, escape,[, or escape,] while the kitty
keyboard protocol is disabled (e.g. while an external command is running).
This case also virtually increases fish_escape_delay_ms; hopefully this edge
case is not ever relevant.

Part of #11668
2025-07-25 11:47:18 +02:00
Johannes Altmanninger
25e5cc23c1 Block interrupts and uvar events while decoding key
readb() has only one caller that passes blocking=false: try_readb().
This function is used while decoding keys; anything but a successful read
is treated as "end of input sequence".

This means that key input sequences such as \e[1;3D
can be torn apart by
- signals (EINTR) which is more likely since e1be842 (Work around torn byte
  sequences in qemu kbd input with 1ms timeout, 2025-03-04).
- universal variable notifications (from other fish processes)

Fix this by blocking signals and not listening on the uvar fd.  We do
something similar at the next higher level -- key sequence matching --
so extract a function to reuse for key decoding.

Ref: https://github.com/fish-shell/fish-shell/issues/11668#issuecomment-3101341081
2025-07-25 11:47:18 +02:00
Johannes Altmanninger
4012345ba9 Reduce MaybeUninit lifetime 2025-07-25 11:47:18 +02:00

View File

@@ -12,7 +12,7 @@
use crate::reader::{reader_save_screen_state, reader_test_and_clear_interrupted};
use crate::terminal::{Capability, SCROLL_FORWARD_SUPPORTED, SCROLL_FORWARD_TERMINFO_CODE};
use crate::threads::iothread_port;
use crate::tty_handoff::set_kitty_keyboard_capability;
use crate::tty_handoff::{get_kitty_keyboard_capability, set_kitty_keyboard_capability};
use crate::universal_notifier::default_notifier;
use crate::wchar::{encode_byte_to_char, prelude::*};
use crate::wutil::encoding::{mbrtowc, mbstate_t, zero_mbstate};
@@ -21,7 +21,6 @@
use std::collections::VecDeque;
use std::mem::MaybeUninit;
use std::os::fd::RawFd;
use std::ptr;
use std::sync::atomic::{AtomicUsize, Ordering};
use std::time::Duration;
@@ -513,8 +512,35 @@ enum ReadbResult {
NothingToRead,
}
fn readb(in_fd: RawFd, blocking: bool) -> ReadbResult {
fn readb(in_fd: RawFd, blocking: bool, pasting: bool) -> ReadbResult {
let do_readb = || {
let mut arr: [u8; 1] = [0];
if read_blocked(in_fd, &mut arr) != Ok(1) {
// The terminal has been closed.
return ReadbResult::Eof;
}
let c = arr[0];
FLOG!(reader, "Read byte", char_to_symbol(char::from(c), true));
// The common path is to return a u8.
return ReadbResult::Byte(c);
};
assert!(in_fd >= 0, "Invalid in fd");
if !blocking {
return if check_fd_readable(
in_fd,
Duration::from_millis(
if pasting || get_kitty_keyboard_capability() == Capability::Supported {
300
} else {
1
},
),
) {
do_readb()
} else {
ReadbResult::NothingToRead
};
}
let mut fdset = FdReadableSet::new();
loop {
fdset.clear();
@@ -532,11 +558,7 @@ fn readb(in_fd: RawFd, blocking: bool) -> ReadbResult {
}
// Here's where we call select().
let select_res = fdset.check_readable(if blocking {
Timeout::Forever
} else {
Timeout::Duration(Duration::from_millis(1))
});
let select_res = fdset.check_readable(Timeout::Forever);
if select_res < 0 {
let err = errno::errno().0;
if err == libc::EINTR || err == libc::EAGAIN {
@@ -548,32 +570,18 @@ fn readb(in_fd: RawFd, blocking: bool) -> ReadbResult {
}
}
if blocking {
// select() did not return an error, so we may have a readable fd.
// The priority order is: uvars, stdin, ioport.
// Check to see if we want a universal variable barrier.
if let Some(notifier_fd) = notifier_fd {
if fdset.test(notifier_fd) && notifier.notification_fd_became_readable(notifier_fd)
{
return ReadbResult::UvarNotified;
}
// select() did not return an error, so we may have a readable fd.
// The priority order is: uvars, stdin, ioport.
// Check to see if we want a universal variable barrier.
if let Some(notifier_fd) = notifier_fd {
if fdset.test(notifier_fd) && notifier.notification_fd_became_readable(notifier_fd) {
return ReadbResult::UvarNotified;
}
}
// Check stdin.
if fdset.test(in_fd) {
let mut arr: [u8; 1] = [0];
if read_blocked(in_fd, &mut arr) != Ok(1) {
// The terminal has been closed.
return ReadbResult::Eof;
}
let c = arr[0];
FLOG!(reader, "Read byte", char_to_symbol(char::from(c), true));
// The common path is to return a u8.
return ReadbResult::Byte(c);
}
if !blocking {
return ReadbResult::NothingToRead;
return do_readb();
}
// Check for iothread completions only if there is no data to be read from the stdin.
@@ -584,6 +592,55 @@ fn readb(in_fd: RawFd, blocking: bool) -> ReadbResult {
}
}
pub fn check_fd_readable(in_fd: RawFd, timeout: Duration) -> bool {
use std::ptr;
// We are not prepared to handle a signal immediately; we only want to know if we get input on
// our fd before the timeout. Use pselect to block all signals; we will handle signals
// before the next call to readch().
let mut sigs = MaybeUninit::uninit();
let mut sigs = unsafe {
libc::sigfillset(sigs.as_mut_ptr());
sigs.assume_init()
};
// pselect expects timeouts in nanoseconds.
const NSEC_PER_MSEC: u64 = 1000 * 1000;
const NSEC_PER_SEC: u64 = NSEC_PER_MSEC * 1000;
let wait_nsec: u64 = (timeout.as_millis() as u64) * NSEC_PER_MSEC;
let timeout = libc::timespec {
tv_sec: (wait_nsec / NSEC_PER_SEC).try_into().unwrap(),
tv_nsec: (wait_nsec % NSEC_PER_SEC).try_into().unwrap(),
};
// We have one fd of interest.
let mut fdset = MaybeUninit::uninit();
let mut fdset = unsafe {
libc::FD_ZERO(fdset.as_mut_ptr());
fdset.assume_init()
};
unsafe {
libc::FD_SET(in_fd, &mut fdset);
}
let res = unsafe {
libc::pselect(
in_fd + 1,
&mut fdset,
ptr::null_mut(),
ptr::null_mut(),
&timeout,
&sigs,
)
};
// Prevent signal starvation on WSL causing the `torn_escapes.py` test to fail
if is_windows_subsystem_for_linux(WSL::V1) {
// Merely querying the current thread's sigmask is sufficient to deliver a pending signal
let _ = unsafe { libc::pthread_sigmask(0, ptr::null(), &mut sigs) };
}
res > 0
}
// Update the wait_on_escape_ms value in response to the fish_escape_delay_ms user variable being
// set.
pub fn update_wait_on_escape_ms(vars: &EnvStack) {
@@ -757,7 +814,7 @@ fn try_readch(&mut self, blocking: bool) -> Option<CharEvent> {
return Some(mevt);
}
let rr = readb(self.get_in_fd(), blocking);
let rr = readb(self.get_in_fd(), blocking, /*pasting=*/ false);
match rr {
ReadbResult::Eof => {
return Some(CharEvent::Implicit(ImplicitEvent::Eof));
@@ -800,10 +857,16 @@ fn try_readch(&mut self, blocking: bool) -> Option<CharEvent> {
let mut i = 0;
let ok = loop {
if i == buffer.len() {
buffer.push(match readb(self.get_in_fd(), /*blocking=*/ true) {
ReadbResult::Byte(b) => b,
_ => 0,
});
buffer.push(
match readb(
self.get_in_fd(),
/*blocking=*/ true,
/*pasting=*/ false,
) {
ReadbResult::Byte(b) => b,
_ => 0,
},
);
}
match decode_input_byte(
&mut seq,
@@ -882,7 +945,11 @@ fn try_readch(&mut self, blocking: bool) -> Option<CharEvent> {
}
fn try_readb(&mut self, buffer: &mut Vec<u8>) -> Option<u8> {
let ReadbResult::Byte(next) = readb(self.get_in_fd(), /*blocking=*/ false) else {
let ReadbResult::Byte(next) = readb(
self.get_in_fd(),
/*blocking=*/ false,
self.paste_is_buffering(),
) else {
return None;
};
buffer.push(next);
@@ -1403,48 +1470,11 @@ fn readch_timed(&mut self, wait_time_ms: usize) -> Option<CharEvent> {
return Some(evt);
}
// We are not prepared to handle a signal immediately; we only want to know if we get input on
// our fd before the timeout. Use pselect to block all signals; we will handle signals
// before the next call to readch().
let mut sigs = MaybeUninit::uninit();
unsafe { libc::sigfillset(sigs.as_mut_ptr()) };
// pselect expects timeouts in nanoseconds.
const NSEC_PER_MSEC: u64 = 1000 * 1000;
const NSEC_PER_SEC: u64 = NSEC_PER_MSEC * 1000;
let wait_nsec: u64 = (wait_time_ms as u64) * NSEC_PER_MSEC;
let timeout = libc::timespec {
tv_sec: (wait_nsec / NSEC_PER_SEC).try_into().unwrap(),
tv_nsec: (wait_nsec % NSEC_PER_SEC).try_into().unwrap(),
};
// We have one fd of interest.
let mut fdset = MaybeUninit::uninit();
let in_fd = self.get_in_fd();
unsafe {
libc::FD_ZERO(fdset.as_mut_ptr());
libc::FD_SET(in_fd, fdset.as_mut_ptr());
};
let res = unsafe {
libc::pselect(
in_fd + 1,
fdset.as_mut_ptr(),
ptr::null_mut(),
ptr::null_mut(),
&timeout,
sigs.as_ptr(),
)
};
// Prevent signal starvation on WSL causing the `torn_escapes.py` test to fail
if is_windows_subsystem_for_linux(WSL::V1) {
// Merely querying the current thread's sigmask is sufficient to deliver a pending signal
let _ = unsafe { libc::pthread_sigmask(0, ptr::null(), sigs.as_mut_ptr()) };
}
if res > 0 {
return Some(self.readch());
}
None
check_fd_readable(
self.get_in_fd(),
Duration::from_millis(u64::try_from(wait_time_ms).unwrap()),
)
.then(|| self.readch())
}
/// Return the fd from which to read.