Fix out of order terminal query responses

This forces all responses made to the PTY through the indirection of the
UI event loop, making sure that the writes to the PTY are in the same
order as the original requests.

This just delays all escape sequences by forcing them through the event
loop, ideally all responses which are not asynchronous (like a clipboard
read) would be made immediately. However since some escapes require
feedback from the UI to mutable structures like the config (e.g. color
query escapes), this would require additional locking.

Fixes #4872.

alacritty/src/event.rs

@@ -1222,6 +1222,7 @@ impl<N: Notify + OnResize> Processor<N> {
                         let text = format(processor.ctx.display.colors[index]);
                         processor.ctx.write_to_pty(text.into_bytes());
                     },
+                    TerminalEvent::PtyWrite(text) => processor.ctx.write_to_pty(text.into_bytes()),
                     TerminalEvent::MouseCursorDirty => processor.reset_mouse_cursor(),
                     TerminalEvent::Exit => (),
                     TerminalEvent::CursorBlinkingChange(_) => {

alacritty_terminal/src/ansi.rs

@@ -2,7 +2,7 @@
 
 use std::convert::TryFrom;
 use std::time::{Duration, Instant};
-use std::{io, iter, str};
+use std::{iter, str};
 
 use log::{debug, trace};
 use serde::{Deserialize, Serialize};
@@ -153,29 +153,27 @@ impl Processor {
 
     /// Process a new byte from the PTY.
     #[inline]
-    pub fn advance<H, W>(&mut self, handler: &mut H, byte: u8, writer: &mut W)
+    pub fn advance<H>(&mut self, handler: &mut H, byte: u8)
     where
         H: Handler,
-        W: io::Write,
     {
         if self.state.sync_state.timeout.is_none() {
-            let mut performer = Performer::new(&mut self.state, handler, writer);
+            let mut performer = Performer::new(&mut self.state, handler);
             self.parser.advance(&mut performer, byte);
         } else {
-            self.advance_sync(handler, byte, writer);
+            self.advance_sync(handler, byte);
         }
     }
 
     /// End a synchronized update.
-    pub fn stop_sync<H, W>(&mut self, handler: &mut H, writer: &mut W)
+    pub fn stop_sync<H>(&mut self, handler: &mut H)
     where
         H: Handler,
-        W: io::Write,
     {
         // Process all synchronized bytes.
         for i in 0..self.state.sync_state.buffer.len() {
             let byte = self.state.sync_state.buffer[i];
-            let mut performer = Performer::new(&mut self.state, handler, writer);
+            let mut performer = Performer::new(&mut self.state, handler);
             self.parser.advance(&mut performer, byte);
         }
 
@@ -198,16 +196,15 @@ impl Processor {
 
     /// Process a new byte during a synchronized update.
     #[cold]
-    fn advance_sync<H, W>(&mut self, handler: &mut H, byte: u8, writer: &mut W)
+    fn advance_sync<H>(&mut self, handler: &mut H, byte: u8)
     where
         H: Handler,
-        W: io::Write,
     {
         self.state.sync_state.buffer.push(byte);
 
         // Handle sync DCS escape sequences.
         match self.state.sync_state.pending_dcs {
-            Some(_) => self.advance_sync_dcs_end(handler, byte, writer),
+            Some(_) => self.advance_sync_dcs_end(handler, byte),
             None => self.advance_sync_dcs_start(),
         }
     }
@@ -228,10 +225,9 @@ impl Processor {
     }
 
     /// Parse the DCS termination sequence for synchronized updates.
-    fn advance_sync_dcs_end<H, W>(&mut self, handler: &mut H, byte: u8, writer: &mut W)
+    fn advance_sync_dcs_end<H>(&mut self, handler: &mut H, byte: u8)
     where
         H: Handler,
-        W: io::Write,
     {
         match byte {
             // Ignore DCS passthrough characters.
@@ -243,7 +239,7 @@ impl Processor {
                 Some(Dcs::SyncStart) => {
                     self.state.sync_state.timeout = Some(Instant::now() + SYNC_UPDATE_TIMEOUT);
                 },
-                Some(Dcs::SyncEnd) => self.stop_sync(handler, writer),
+                Some(Dcs::SyncEnd) => self.stop_sync(handler),
                 None => (),
             },
         }
@@ -254,21 +250,16 @@ impl Processor {
 ///
 /// Processor creates a Performer when running advance and passes the Performer
 /// to `vte::Parser`.
-struct Performer<'a, H: Handler, W: io::Write> {
+struct Performer<'a, H: Handler> {
     state: &'a mut ProcessorState,
     handler: &'a mut H,
-    writer: &'a mut W,
 }
 
-impl<'a, H: Handler + 'a, W: io::Write> Performer<'a, H, W> {
+impl<'a, H: Handler + 'a> Performer<'a, H> {
     /// Create a performer.
     #[inline]
-    pub fn new<'b>(
-        state: &'b mut ProcessorState,
-        handler: &'b mut H,
-        writer: &'b mut W,
-    ) -> Performer<'b, H, W> {
-        Performer { state, handler, writer }
+    pub fn new<'b>(state: &'b mut ProcessorState, handler: &'b mut H) -> Performer<'b, H> {
+        Performer { state, handler }
     }
 }
 
@@ -308,10 +299,10 @@ pub trait Handler {
     fn move_down(&mut self, _: usize) {}
 
     /// Identify the terminal (should write back to the pty stream).
-    fn identify_terminal<W: io::Write>(&mut self, _: &mut W, _intermediate: Option<char>) {}
+    fn identify_terminal(&mut self, _intermediate: Option<char>) {}
 
     /// Report device status.
-    fn device_status<W: io::Write>(&mut self, _: &mut W, _: usize) {}
+    fn device_status(&mut self, _: usize) {}
 
     /// Move cursor forward `cols`.
     fn move_forward(&mut self, _: Column) {}
@@ -461,10 +452,10 @@ pub trait Handler {
     fn pop_title(&mut self) {}
 
     /// Report text area size in pixels.
-    fn text_area_size_pixels<W: io::Write>(&mut self, _: &mut W) {}
+    fn text_area_size_pixels(&mut self) {}
 
     /// Report text area size in characters.
-    fn text_area_size_chars<W: io::Write>(&mut self, _: &mut W) {}
+    fn text_area_size_chars(&mut self) {}
 }
 
 /// Terminal cursor configuration.
@@ -883,10 +874,9 @@ impl StandardCharset {
     }
 }
 
-impl<'a, H, W> vte::Perform for Performer<'a, H, W>
+impl<'a, H> vte::Perform for Performer<'a, H>
 where
     H: Handler + 'a,
-    W: io::Write + 'a,
 {
     #[inline]
     fn print(&mut self, c: char) {
@@ -1115,7 +1105,6 @@ where
 
         let mut params_iter = params.iter();
         let handler = &mut self.handler;
-        let writer = &mut self.writer;
 
         let mut next_param_or = |default: u16| {
             params_iter.next().map(|param| param[0]).filter(|&param| param != 0).unwrap_or(default)
@@ -1136,7 +1125,7 @@ where
             },
             ('C', []) | ('a', []) => handler.move_forward(Column(next_param_or(1) as usize)),
             ('c', intermediates) if next_param_or(0) == 0 => {
-                handler.identify_terminal(writer, intermediates.get(0).map(|&i| i as char))
+                handler.identify_terminal(intermediates.get(0).map(|&i| i as char))
             },
             ('D', []) => handler.move_backward(Column(next_param_or(1) as usize)),
             ('d', []) => handler.goto_line(Line(next_param_or(1) as i32 - 1)),
@@ -1218,7 +1207,7 @@ where
                     }
                 }
             },
-            ('n', []) => handler.device_status(writer, next_param_or(0) as usize),
+            ('n', []) => handler.device_status(next_param_or(0) as usize),
             ('P', []) => handler.delete_chars(next_param_or(1) as usize),
             ('q', [b' ']) => {
                 // DECSCUSR (CSI Ps SP q) -- Set Cursor Style.
@@ -1249,8 +1238,8 @@ where
             ('s', []) => handler.save_cursor_position(),
             ('T', []) => handler.scroll_down(next_param_or(1) as usize),
             ('t', []) => match next_param_or(1) as usize {
-                14 => handler.text_area_size_pixels(writer),
-                18 => handler.text_area_size_chars(writer),
+                14 => handler.text_area_size_pixels(),
+                18 => handler.text_area_size_chars(),
                 22 => handler.push_title(),
                 23 => handler.pop_title(),
                 _ => unhandled!(),
@@ -1298,7 +1287,7 @@ where
             },
             (b'H', []) => self.handler.set_horizontal_tabstop(),
             (b'M', []) => self.handler.reverse_index(),
-            (b'Z', []) => self.handler.identify_terminal(self.writer, None),
+            (b'Z', []) => self.handler.identify_terminal(None),
             (b'c', []) => self.handler.reset_state(),
             (b'0', intermediates) => {
                 configure_charset!(StandardCharset::SpecialCharacterAndLineDrawing, intermediates)
@@ -1494,11 +1483,9 @@ pub mod C0 {
 // Byte sequences used in these tests are recording of pty stdout.
 #[cfg(test)]
 mod tests {
-    use super::{
-        parse_number, xparse_color, Attr, CharsetIndex, Color, Handler, Processor, StandardCharset,
-    };
+    use super::*;
+
     use crate::term::color::Rgb;
-    use std::io;
 
     struct MockHandler {
         index: CharsetIndex,
@@ -1521,7 +1508,7 @@ mod tests {
             self.index = index;
         }
 
-        fn identify_terminal<W: io::Write>(&mut self, _: &mut W, _intermediate: Option<char>) {
+        fn identify_terminal(&mut self, _intermediate: Option<char>) {
             self.identity_reported = true;
         }
 
@@ -1549,7 +1536,7 @@ mod tests {
         let mut handler = MockHandler::default();
 
         for byte in BYTES {
-            parser.advance(&mut handler, *byte, &mut io::sink());
+            parser.advance(&mut handler, *byte);
         }
 
         assert_eq!(handler.attr, Some(Attr::Bold));
@@ -1563,7 +1550,7 @@ mod tests {
         let mut handler = MockHandler::default();
 
         for byte in bytes {
-            parser.advance(&mut handler, *byte, &mut io::sink());
+            parser.advance(&mut handler, *byte);
         }
 
         assert!(!handler.identity_reported);
@@ -1572,7 +1559,7 @@ mod tests {
         let bytes: &[u8] = &[0x1b, b'[', b'c'];
 
         for byte in bytes {
-            parser.advance(&mut handler, *byte, &mut io::sink());
+            parser.advance(&mut handler, *byte);
         }
 
         assert!(handler.identity_reported);
@@ -1581,7 +1568,7 @@ mod tests {
         let bytes: &[u8] = &[0x1b, b'[', b'0', b'c'];
 
         for byte in bytes {
-            parser.advance(&mut handler, *byte, &mut io::sink());
+            parser.advance(&mut handler, *byte);
         }
 
         assert!(handler.identity_reported);
@@ -1595,7 +1582,7 @@ mod tests {
         let mut handler = MockHandler::default();
 
         for byte in bytes {
-            parser.advance(&mut handler, *byte, &mut io::sink());
+            parser.advance(&mut handler, *byte);
         }
 
         assert!(handler.identity_reported);
@@ -1607,7 +1594,7 @@ mod tests {
         let mut handler = MockHandler::default();
 
         for byte in bytes {
-            parser.advance(&mut handler, *byte, &mut io::sink());
+            parser.advance(&mut handler, *byte);
         }
 
         assert!(!handler.identity_reported);
@@ -1625,7 +1612,7 @@ mod tests {
         let mut handler = MockHandler::default();
 
         for byte in BYTES {
-            parser.advance(&mut handler, *byte, &mut io::sink());
+            parser.advance(&mut handler, *byte);
         }
 
         let spec = Rgb { r: 128, g: 66, b: 255 };
@@ -1656,7 +1643,7 @@ mod tests {
         let mut parser = Processor::new();
 
         for byte in BYTES {
-            parser.advance(&mut handler, *byte, &mut io::sink());
+            parser.advance(&mut handler, *byte);
         }
     }
 
@@ -1667,7 +1654,7 @@ mod tests {
         let mut handler = MockHandler::default();
 
         for byte in BYTES {
-            parser.advance(&mut handler, *byte, &mut io::sink());
+            parser.advance(&mut handler, *byte);
         }
 
         assert_eq!(handler.index, CharsetIndex::G0);
@@ -1681,14 +1668,14 @@ mod tests {
         let mut handler = MockHandler::default();
 
         for byte in &BYTES[..3] {
-            parser.advance(&mut handler, *byte, &mut io::sink());
+            parser.advance(&mut handler, *byte);
         }
 
         assert_eq!(handler.index, CharsetIndex::G1);
         assert_eq!(handler.charset, StandardCharset::SpecialCharacterAndLineDrawing);
 
         let mut handler = MockHandler::default();
-        parser.advance(&mut handler, BYTES[3], &mut io::sink());
+        parser.advance(&mut handler, BYTES[3]);
 
         assert_eq!(handler.index, CharsetIndex::G1);
     }

alacritty_terminal/src/event.rs

@@ -35,6 +35,9 @@ pub enum Event {
     /// expected escape sequence format.
     ColorRequest(usize, Arc<dyn Fn(Rgb) -> String + Sync + Send + 'static>),
 
+    /// Write some text to the PTY.
+    PtyWrite(String),
+
     /// Cursor blinking state has changed.
     CursorBlinkingChange(bool),
 
@@ -57,6 +60,7 @@ impl Debug for Event {
             Event::ClipboardStore(ty, text) => write!(f, "ClipboardStore({:?}, {})", ty, text),
             Event::ClipboardLoad(ty, _) => write!(f, "ClipboardLoad({:?})", ty),
             Event::ColorRequest(index, _) => write!(f, "ColorRequest({})", index),
+            Event::PtyWrite(text) => write!(f, "PtyWrite({})", text),
             Event::Wakeup => write!(f, "Wakeup"),
             Event::Bell => write!(f, "Bell"),
             Event::Exit => write!(f, "Exit"),

alacritty_terminal/src/event_loop.rs

@@ -240,7 +240,7 @@ where
 
                     // Run the parser.
                     for byte in &buf[..got] {
-                        state.parser.advance(&mut **terminal, *byte, &mut self.pty.writer());
+                        state.parser.advance(&mut **terminal, *byte);
                     }
 
                     // Exit if we've processed enough bytes.
@@ -334,7 +334,7 @@ where
 
                 // Handle synchronized update timeout.
                 if events.is_empty() {
-                    state.parser.stop_sync(&mut *self.terminal.lock(), &mut self.pty.writer());
+                    state.parser.stop_sync(&mut *self.terminal.lock());
                     self.event_proxy.send_event(Event::Wakeup);
                     continue;
                 }

alacritty_terminal/src/term/mod.rs

@@ -3,7 +3,7 @@
 use std::cmp::{max, min};
 use std::ops::{Index, IndexMut, Range};
 use std::sync::Arc;
-use std::{io, mem, ptr, str};
+use std::{mem, ptr, str};
 
 use bitflags::bitflags;
 use log::{debug, trace};
@@ -968,32 +968,35 @@ impl<T: EventListener> Handler for Term<T> {
     }
 
     #[inline]
-    fn identify_terminal<W: io::Write>(&mut self, writer: &mut W, intermediate: Option<char>) {
+    fn identify_terminal(&mut self, intermediate: Option<char>) {
         match intermediate {
             None => {
                 trace!("Reporting primary device attributes");
-                let _ = writer.write_all(b"\x1b[?6c");
+                let text = String::from("\x1b[?6c");
+                self.event_proxy.send_event(Event::PtyWrite(text));
             },
             Some('>') => {
                 trace!("Reporting secondary device attributes");
                 let version = version_number(env!("CARGO_PKG_VERSION"));
-                let _ = writer.write_all(format!("\x1b[>0;{};1c", version).as_bytes());
+                let text = format!("\x1b[>0;{};1c", version);
+                self.event_proxy.send_event(Event::PtyWrite(text));
             },
             _ => debug!("Unsupported device attributes intermediate"),
         }
     }
 
     #[inline]
-    fn device_status<W: io::Write>(&mut self, writer: &mut W, arg: usize) {
+    fn device_status(&mut self, arg: usize) {
         trace!("Reporting device status: {}", arg);
         match arg {
             5 => {
-                let _ = writer.write_all(b"\x1b[0n");
+                let text = String::from("\x1b[0n");
+                self.event_proxy.send_event(Event::PtyWrite(text));
             },
             6 => {
                 let pos = self.grid.cursor.point;
-                let response = format!("\x1b[{};{}R", pos.line + 1, pos.column + 1);
-                let _ = writer.write_all(response.as_bytes());
+                let text = format!("\x1b[{};{}R", pos.line + 1, pos.column + 1);
+                self.event_proxy.send_event(Event::PtyWrite(text));
             },
             _ => debug!("unknown device status query: {}", arg),
         };
@@ -1687,15 +1690,17 @@ impl<T: EventListener> Handler for Term<T> {
     }
 
     #[inline]
-    fn text_area_size_pixels<W: io::Write>(&mut self, writer: &mut W) {
+    fn text_area_size_pixels(&mut self) {
         let width = self.cell_width * self.columns();
         let height = self.cell_height * self.screen_lines();
-        let _ = write!(writer, "\x1b[4;{};{}t", height, width);
+        let text = format!("\x1b[4;{};{}t", height, width);
+        self.event_proxy.send_event(Event::PtyWrite(text));
     }
 
     #[inline]
-    fn text_area_size_chars<W: io::Write>(&mut self, writer: &mut W) {
-        let _ = write!(writer, "\x1b[8;{};{}t", self.screen_lines(), self.columns());
+    fn text_area_size_chars(&mut self) {
+        let text = format!("\x1b[8;{};{}t", self.screen_lines(), self.columns());
+        self.event_proxy.send_event(Event::PtyWrite(text));
     }
 }
 

alacritty_terminal/tests/ref.rs

@@ -2,7 +2,7 @@ use serde::Deserialize;
 use serde_json as json;
 
 use std::fs::{self, File};
-use std::io::{self, Read};
+use std::io::Read;
 use std::path::Path;
 
 use alacritty_terminal::ansi;
@@ -108,7 +108,7 @@ fn ref_test(dir: &Path) {
     let mut parser = ansi::Processor::new();
 
     for byte in recording {
-        parser.advance(&mut terminal, byte, &mut io::sink());
+        parser.advance(&mut terminal, byte);
     }
 
     // Truncate invisible lines from the grid.