Use a low TTY query timeout only if first query failed

As mentioned in the comment, query timeouts can happen if either
1. the terminal doesn't support primary device attribute
2. there is extreme (network) latency

In the first case, we want to turn the timeout way down.  In the
second case, probably not, especially because the network latency
may be transient. Let's keep the high timeout in the second case.

Fixes 7ef4e7dfe7 (Time out terminal queries after a while,
2025-09-21).
This commit is contained in:
Johannes Altmanninger
2025-09-25 20:13:28 +02:00
parent 7e3fac561d
commit b907bc775a
5 changed files with 66 additions and 66 deletions

View File

@@ -262,7 +262,8 @@ pub fn fish_key_reader(
streams,
continuous_mode,
verbose,
InputEventQueue::new(streams.stdin_fd),
// Won't be querying, so no timeout value needed.
InputEventQueue::new(streams.stdin_fd, None),
)
}

View File

@@ -1,11 +1,10 @@
use crate::common::{
fish_reserved_codepoint, is_windows_subsystem_for_linux, read_blocked, shell_modes,
str2wcstring, PROGRAM_NAME, WSL,
str2wcstring, WSL,
};
use crate::env::{EnvStack, Environment};
use crate::fd_readable_set::{FdReadableSet, Timeout};
use crate::flog::{FloggableDebug, FloggableDisplay, FLOG};
use crate::global_safety::RelaxedAtomicBool;
use crate::key::{
self, alt, canonicalize_control_char, canonicalize_keyed_control_char, char_to_symbol,
function_key, shift, Key, Modifiers, ViewportPosition,
@@ -728,6 +727,7 @@ fn parse_mask(mask: u32) -> (Modifiers, bool) {
}
// A data type used by the input machinery.
#[derive(Default)]
pub struct InputData {
// The file descriptor from which we read input, often stdin.
pub in_fd: RawFd,
@@ -746,11 +746,14 @@ pub struct InputData {
// Transient storage to avoid repeated allocations.
pub event_storage: Vec<CharEvent>,
// How long to wait for responses for TTY queries.
pub blocking_query_timeout: Option<Duration>,
}
impl InputData {
/// Construct from the fd from which to read.
pub fn new(in_fd: RawFd) -> Self {
pub fn new(in_fd: RawFd, blocking_query_timeout: Option<Duration>) -> Self {
Self {
in_fd,
queue: VecDeque::new(),
@@ -758,6 +761,7 @@ pub fn new(in_fd: RawFd) -> Self {
input_function_args: Vec::new(),
function_status: false,
event_storage: Vec::new(),
blocking_query_timeout,
}
}
@@ -836,22 +840,10 @@ fn readch(&mut self) -> CharEvent {
return mevt;
}
const INITIAL_QUERY_TIMEOUT_SECONDS: u64 = 2;
static ABANDON_WAITING_FOR_QUERIES: RelaxedAtomicBool = RelaxedAtomicBool::new(false);
match next_input_event(
self.get_in_fd(),
if self.is_blocked_querying() {
Timeout::Duration(if ABANDON_WAITING_FOR_QUERIES.load() {
// This should small enough so the delay on incompatible terminals is
// not noticeable, and high enough so we can still receive some query
// responses if we ever get into this state on a compatible terminal,
// which can happen after extreme (network) latency exceeds our initial
// timeout. In future, we should tolerate this better.
Duration::from_millis(30)
} else {
Duration::from_secs(INITIAL_QUERY_TIMEOUT_SECONDS)
})
Timeout::Duration(self.get_input_data().blocking_query_timeout.unwrap())
} else {
Timeout::Forever
},
@@ -977,23 +969,6 @@ fn readch(&mut self) -> CharEvent {
return key_evt;
}
InputEventTrigger::TimeoutElapsed => {
if !ABANDON_WAITING_FOR_QUERIES.load() {
let program = PROGRAM_NAME.get().unwrap();
FLOG!(
warning,
wgettext_fmt!(
"%s could not read response to primary device attribute query after waiting for %d seconds. \
This is often due to a missing feature in your terminal. \
See 'help terminal-compatibility' or 'man fish-terminal-compatibility'. \
This %s process will no longer wait for outstanding queries, \
which disables some optional features.",
program,
INITIAL_QUERY_TIMEOUT_SECONDS,
program
),
);
ABANDON_WAITING_FOR_QUERIES.store(true);
}
return CharEvent::QueryResult(QueryResultEvent::Timeout);
}
}
@@ -1801,9 +1776,9 @@ pub struct InputEventQueue {
}
impl InputEventQueue {
pub fn new(in_fd: RawFd) -> Self {
pub fn new(in_fd: RawFd, blocking_query_timeout: Option<Duration>) -> Self {
Self {
data: InputData::new(in_fd),
data: InputData::new(in_fd, blocking_query_timeout),
blocking_query: RefCell::new(None),
}
}

View File

@@ -14,7 +14,7 @@
};
use crate::fds::{open_dir, BEST_O_SEARCH};
use crate::global_safety::RelaxedAtomicBool;
use crate::input_common::{CharEvent, TerminalQuery};
use crate::input_common::TerminalQuery;
use crate::io::IoChain;
use crate::job_group::MaybeJobId;
use crate::operation_context::{OperationContext, EXPANSION_LIMIT_DEFAULT};
@@ -37,7 +37,6 @@
#[cfg(not(target_has_atomic = "64"))]
use portable_atomic::AtomicU64;
use std::cell::{Ref, RefCell, RefMut};
use std::collections::VecDeque;
use std::ffi::{CStr, OsStr};
use std::fs::File;
use std::io::Write;
@@ -47,6 +46,7 @@
#[cfg(target_has_atomic = "64")]
use std::sync::atomic::AtomicU64;
use std::sync::Arc;
use std::time::Duration;
pub enum BlockData {
Function {
@@ -446,7 +446,8 @@ pub struct Parser {
pub blocking_query: RefCell<Option<TerminalQuery>>,
pub pending_input: RefCell<VecDeque<CharEvent>>,
// Timeout for blocking terminal queries.
pub blocking_query_timeout: RefCell<Option<Duration>>,
}
impl Parser {
@@ -466,7 +467,7 @@ pub fn new(variables: EnvStack, cancel_behavior: CancelBehavior) -> Parser {
profile_items: RefCell::default(),
global_event_blocks: AtomicU64::new(0),
blocking_query: RefCell::new(None),
pending_input: RefCell::new(VecDeque::new()),
blocking_query_timeout: RefCell::new(None),
};
match open_dir(CStr::from_bytes_with_nul(b".\0").unwrap(), BEST_O_SEARCH) {

View File

@@ -273,7 +273,11 @@ fn querying_allowed(in_fd: RawFd) -> bool {
pub fn terminal_init(vars: &dyn Environment, inputfd: RawFd) -> InputEventQueue {
reader_interactive_init();
let mut input_queue = InputEventQueue::new(inputfd);
const INITIAL_QUERY_TIMEOUT_SECONDS: u64 = 2;
let mut input_queue = InputEventQueue::new(
inputfd,
Some(Duration::from_secs(INITIAL_QUERY_TIMEOUT_SECONDS)),
);
let _init_tty_metadata = ScopeGuard::new((), |()| {
initialize_tty_metadata();
@@ -302,7 +306,28 @@ pub fn terminal_init(vars: &dyn Environment, inputfd: RawFd) -> InputEventQueue
Implicit(Eof) => reader_sighup(),
Implicit(CheckExit) => {}
Implicit(QueryInterrupted) => break,
CharEvent::QueryResult(Response(QueryResponse::PrimaryDeviceAttribute) | Timeout) => {
CharEvent::QueryResult(Response(QueryResponse::PrimaryDeviceAttribute)) => {
break;
}
CharEvent::QueryResult(Timeout) => {
let program = PROGRAM_NAME.get().unwrap();
FLOG!(
warning,
wgettext_fmt!(
"%s could not read response to primary device attribute query after waiting for %d seconds. \
This is often due to a missing feature in your terminal. \
See 'help terminal-compatibility' or 'man fish-terminal-compatibility'. \
This %s process will no longer wait for outstanding queries, \
which disables some optional features.",
program,
INITIAL_QUERY_TIMEOUT_SECONDS,
program
),
);
input_queue
.get_input_data_mut()
.blocking_query_timeout
.replace(Duration::from_millis(30));
break;
}
CharEvent::QueryResult(Response(_)) => (),
@@ -315,14 +340,6 @@ pub fn terminal_init(vars: &dyn Environment, inputfd: RawFd) -> InputEventQueue
let input_data = input_queue.get_input_data();
// We blocked execution of code and mappings so input function args must be empty.
assert!(input_data.input_function_args.is_empty());
if input_data.paste_buffer.is_some() {
// The terminal should never interleave query responses with a bracketed paste
// command. hence this should only happen on timeout.
FLOG!(
reader,
"Bracketed paste was interrupted; dropping uncommitted paste buffer"
)
}
assert!(input_data.event_storage.is_empty());
FLOGF!(
reader,
@@ -364,13 +381,10 @@ pub fn current_data() -> Option<&'static mut ReaderData> {
/// If `history_name` is empty, then save history in-memory only; do not write it to disk.
pub fn reader_push<'a>(parser: &'a Parser, history_name: &wstr, conf: ReaderConfig) -> Reader<'a> {
assert_is_main_thread();
if !parser.interactive_initialized.swap(true) {
let mut input_queue = terminal_init(parser.vars(), conf.inputfd);
let inputfd = conf.inputfd;
let input_data = if !parser.interactive_initialized.swap(true) {
let mut input_queue = terminal_init(parser.vars(), inputfd);
let input_data = input_queue.get_input_data_mut();
parser
.pending_input
.borrow_mut()
.extend(std::mem::take(&mut input_data.queue));
parser.vars().set_one(
L!("fish_terminal"),
EnvMode::GLOBAL,
@@ -384,16 +398,21 @@ pub fn reader_push<'a>(parser: &'a Parser, history_name: &wstr, conf: ReaderConf
parser
.vars()
.set_one(L!("_"), EnvMode::GLOBAL, L!("fish").to_owned());
}
let old = parser
.blocking_query_timeout
.replace(input_data.blocking_query_timeout);
assert!(old.is_none());
std::mem::take(input_data)
} else {
InputData::new(inputfd, *parser.blocking_query_timeout.borrow())
};
let hist = History::with_name(history_name);
hist.resolve_pending();
let data = ReaderData::new(hist, conf, reader_data_stack().is_empty());
let data = ReaderData::new(input_data, hist, conf, reader_data_stack().is_empty());
reader_data_stack().push(data);
let data = current_data().unwrap();
data.command_line_changed(EditableLineTag::Commandline, AutosuggestionUpdate::Remove);
let mut reader = Reader { data, parser };
reader.insert_front(parser.pending_input.take());
reader
Reader { data, parser }
}
/// Return to previous reader environment.
@@ -1279,8 +1298,12 @@ fn reader_received_sighup() -> bool {
}
impl ReaderData {
fn new(history: Arc<History>, conf: ReaderConfig, is_top_level: bool) -> Pin<Box<Self>> {
let input_data = InputData::new(conf.inputfd);
fn new(
input_data: InputData,
history: Arc<History>,
conf: ReaderConfig,
is_top_level: bool,
) -> Pin<Box<Self>> {
let mut command_line = EditableLine::default();
if is_top_level {
let state = commandline_state_snapshot();

View File

@@ -2,7 +2,7 @@
#[test]
fn test_push_front_back() {
let mut queue = InputEventQueue::new(0);
let mut queue = InputEventQueue::new(0, None);
queue.push_front(CharEvent::from_char('a'));
queue.push_front(CharEvent::from_char('b'));
queue.push_back(CharEvent::from_char('c'));
@@ -16,7 +16,7 @@ fn test_push_front_back() {
#[test]
fn test_promote_interruptions_to_front() {
let mut queue = InputEventQueue::new(0);
let mut queue = InputEventQueue::new(0, None);
queue.push_back(CharEvent::from_char('a'));
queue.push_back(CharEvent::from_char('b'));
queue.push_back(CharEvent::from_readline(ReadlineCmd::Undo));
@@ -41,7 +41,7 @@ fn test_promote_interruptions_to_front() {
#[test]
fn test_insert_front() {
let mut queue = InputEventQueue::new(0);
let mut queue = InputEventQueue::new(0, None);
queue.push_back(CharEvent::from_char('a'));
queue.push_back(CharEvent::from_char('b'));