Send PTY resize messages through event loop

This allows us to clean up the Arcs on windows, as well as tidy up
the code on unix a little too.

Fixes #3086.

alacritty/src/event.rs

@@ -308,20 +308,18 @@ pub struct Processor<N> {
     suppress_chars: bool,
     modifiers: ModifiersState,
     config: Config,
-    pty_resize_handle: Box<dyn OnResize>,
     message_buffer: MessageBuffer,
     display: Display,
     font_size: Size,
 }
 
-impl<N: Notify> Processor<N> {
+impl<N: Notify + OnResize> Processor<N> {
     /// Create a new event processor
     ///
     /// Takes a writer which is expected to be hooked up to the write end of a
     /// pty.
     pub fn new(
         notifier: N,
-        pty_resize_handle: Box<dyn OnResize>,
         message_buffer: MessageBuffer,
         config: Config,
         display: Display,
@@ -334,7 +332,6 @@ impl<N: Notify> Processor<N> {
             modifiers: Default::default(),
             font_size: config.font.size,
             config,
-            pty_resize_handle,
             message_buffer,
             display,
         }
@@ -405,7 +402,7 @@ impl<N: Notify> Processor<N> {
             if !display_update_pending.is_empty() {
                 self.display.handle_update(
                     &mut terminal,
-                    self.pty_resize_handle.as_mut(),
+                    &mut self.notifier,
                     &self.message_buffer,
                     &self.config,
                     display_update_pending,

alacritty/src/main.rs

@@ -27,8 +27,6 @@ use std::env;
 use std::error::Error;
 use std::fs;
 use std::io::{self, Write};
-#[cfg(not(windows))]
-use std::os::unix::io::AsRawFd;
 use std::sync::Arc;
 
 #[cfg(target_os = "macos")]
@@ -170,16 +168,6 @@ fn run(window_event_loop: GlutinEventLoop<Event>, config: Config) -> Result<(),
     #[cfg(any(target_os = "macos", windows))]
     let pty = tty::new(&config, &display.size_info, None);
 
-    // Create PTY resize handle
-    //
-    // This exists because rust doesn't know the interface is thread-safe
-    // and we need to be able to resize the PTY from the main thread while the IO
-    // thread owns the EventedRW object.
-    #[cfg(windows)]
-    let resize_handle = pty.resize_handle();
-    #[cfg(not(windows))]
-    let resize_handle = pty.fd.as_raw_fd();
-
     // Create the pseudoterminal I/O loop
     //
     // pty I/O is ran on another thread as to not occupy cycles used by the
@@ -204,15 +192,8 @@ fn run(window_event_loop: GlutinEventLoop<Event>, config: Config) -> Result<(),
     let message_buffer = MessageBuffer::new();
 
     // Event processor
-    //
+    let mut processor =
-    // Need the Rc<RefCell<_>> here since a ref is shared in the resize callback
+        Processor::new(event_loop::Notifier(loop_tx.clone()), message_buffer, config, display);
-    let mut processor = Processor::new(
-        event_loop::Notifier(loop_tx.clone()),
-        Box::new(resize_handle),
-        message_buffer,
-        config,
-        display,
-    );
 
     // Kick off the I/O thread
     let io_thread = event_loop.spawn();

alacritty_terminal/src/event_loop.rs

@@ -16,7 +16,7 @@ use crate::ansi;
 use crate::config::Config;
 use crate::event::{self, Event, EventListener};
 use crate::sync::FairMutex;
-use crate::term::Term;
+use crate::term::{SizeInfo, Term};
 use crate::tty;
 use crate::util::thread;
 
@@ -31,6 +31,9 @@ pub enum Msg {
 
     /// Indicates that the `EventLoop` should shut down, as Alacritty is shutting down
     Shutdown,
+
+    /// Instruction to resize the pty
+    Resize(SizeInfo),
 }
 
 /// The main event!.. loop.
@@ -76,9 +79,14 @@ impl event::Notify for Notifier {
         if bytes.len() == 0 {
             return;
         }
-        if self.0.send(Msg::Input(bytes)).is_err() {
+
-            panic!("expected send event loop msg");
+        self.0.send(Msg::Input(bytes)).expect("send event loop msg");
-        }
+    }
+}
+
+impl event::OnResize for Notifier {
+    fn on_resize(&mut self, size: &SizeInfo) {
+        self.0.send(Msg::Resize(*size)).expect("expected send event loop msg");
     }
 }
 
@@ -141,7 +149,7 @@ impl Writing {
 
 impl<T, U> EventLoop<T, U>
 where
-    T: tty::EventedPty + Send + 'static,
+    T: tty::EventedPty + event::OnResize + Send + 'static,
     U: EventListener + Send + 'static,
 {
     /// Create a new event loop
@@ -171,11 +179,12 @@ where
     // Drain the channel
     //
     // Returns `false` when a shutdown message was received.
-    fn drain_recv_channel(&self, state: &mut State) -> bool {
+    fn drain_recv_channel(&mut self, state: &mut State) -> bool {
         while let Ok(msg) = self.rx.try_recv() {
             match msg {
                 Msg::Input(input) => state.write_list.push_back(input),
                 Msg::Shutdown => return false,
+                Msg::Resize(size) => self.pty.on_resize(&size),
             }
         }
 

alacritty_terminal/src/tty/unix.rs

@@ -135,7 +135,7 @@ fn get_pw_entry(buf: &mut [i8; 1024]) -> Passwd<'_> {
 
 pub struct Pty {
     child: Child,
-    pub fd: File,
+    fd: File,
     token: mio::Token,
     signals: Signals,
     signals_token: mio::Token,
@@ -226,14 +226,14 @@ pub fn new<C>(config: &Config<C>, size: &SizeInfo, window_id: Option<usize>) ->
                 set_nonblocking(master);
             }
 
-            let pty = Pty {
+            let mut pty = Pty {
                 child,
                 fd: unsafe { File::from_raw_fd(master) },
                 token: mio::Token::from(0),
                 signals,
                 signals_token: mio::Token::from(0),
             };
-            pty.fd.as_raw_fd().on_resize(size);
+            pty.on_resize(size);
             pty
         },
         Err(err) => die!("Failed to spawn command '{}': {}", shell.program, err),
@@ -350,7 +350,7 @@ impl<'a> ToWinsize for &'a SizeInfo {
     }
 }
 
-impl OnResize for i32 {
+impl OnResize for Pty {
     /// Resize the pty
     ///
     /// Tells the kernel that the window size changed with the new pixel
@@ -358,7 +358,7 @@ impl OnResize for i32 {
     fn on_resize(&mut self, size: &SizeInfo) {
         let win = size.to_winsize();
 
-        let res = unsafe { libc::ioctl(*self, libc::TIOCSWINSZ, &win as *const _) };
+        let res = unsafe { libc::ioctl(self.fd.as_raw_fd(), libc::TIOCSWINSZ, &win as *const _) };
 
         if res < 0 {
             die!("ioctl TIOCSWINSZ failed: {}", io::Error::last_os_error());

alacritty_terminal/src/tty/windows/conpty.rs

@@ -17,7 +17,6 @@ use std::io::Error;
 use std::mem;
 use std::os::windows::io::IntoRawHandle;
 use std::ptr;
-use std::sync::Arc;
 
 use dunce::canonicalize;
 use mio_anonymous_pipes::{EventedAnonRead, EventedAnonWrite};
@@ -85,9 +84,6 @@ pub struct Conpty {
     api: ConptyApi,
 }
 
-/// Handle can be cloned freely and moved between threads.
-pub type ConptyHandle = Arc<Conpty>;
-
 impl Drop for Conpty {
     fn drop(&mut self) {
         // XXX: This will block until the conout pipe is drained. Will cause a deadlock if the
@@ -98,9 +94,8 @@ impl Drop for Conpty {
     }
 }
 
-// The Conpty API can be accessed from multiple threads.
+// The Conpty handle can be sent between threads.
 unsafe impl Send for Conpty {}
-unsafe impl Sync for Conpty {}
 
 pub fn new<C>(config: &Config<C>, size: &SizeInfo, _window_id: Option<usize>) -> Option<Pty> {
     if !config.enable_experimental_conpty_backend {
@@ -244,10 +239,10 @@ pub fn new<C>(config: &Config<C>, size: &SizeInfo, _window_id: Option<usize>) ->
     let conout = EventedAnonRead::new(conout);
 
     let child_watcher = ChildExitWatcher::new(proc_info.hProcess).unwrap();
-    let agent = Conpty { handle: pty_handle, api };
+    let conpty = Conpty { handle: pty_handle, api };
 
     Some(Pty {
-        handle: super::PtyHandle::Conpty(ConptyHandle::new(agent)),
+        backend: super::PtyBackend::Conpty(conpty),
         conout: super::EventedReadablePipe::Anonymous(conout),
         conin: super::EventedWritablePipe::Anonymous(conin),
         read_token: 0.into(),
@@ -262,7 +257,7 @@ fn panic_shell_spawn() {
     panic!("Unable to spawn shell: {}", Error::last_os_error());
 }
 
-impl OnResize for ConptyHandle {
+impl OnResize for Conpty {
     fn on_resize(&mut self, sizeinfo: &SizeInfo) {
         if let Some(coord) = coord_from_sizeinfo(sizeinfo) {
             let result = unsafe { (self.api.ResizePseudoConsole)(self.handle, coord) };

alacritty_terminal/src/tty/windows/mod.rs

@@ -38,16 +38,15 @@ pub fn is_conpty() -> bool {
     IS_CONPTY.load(Ordering::Relaxed)
 }
 
-#[derive(Clone)]
+enum PtyBackend {
-pub enum PtyHandle {
+    Winpty(winpty::Agent),
-    Winpty(winpty::WinptyHandle),
+    Conpty(conpty::Conpty),
-    Conpty(conpty::ConptyHandle),
 }
 
 pub struct Pty {
-    // XXX: Handle is required to be the first field, to ensure correct drop order. Dropping
+    // XXX: Backend is required to be the first field, to ensure correct drop order. Dropping
-    // `conout` before `handle` will cause a deadlock.
+    // `conout` before `backend` will cause a deadlock.
-    handle: PtyHandle,
+    backend: PtyBackend,
     // TODO: It's on the roadmap for the Conpty API to support Overlapped I/O.
     // See https://github.com/Microsoft/console/issues/262
     // When support for that lands then it should be possible to use
@@ -60,12 +59,6 @@ pub struct Pty {
     child_watcher: ChildExitWatcher,
 }
 
-impl Pty {
-    pub fn resize_handle(&self) -> impl OnResize {
-        self.handle.clone()
-    }
-}
-
 pub fn new<C>(config: &Config<C>, size: &SizeInfo, window_id: Option<usize>) -> Pty {
     if let Some(pty) = conpty::new(config, size, window_id) {
         info!("Using Conpty agent");
@@ -189,18 +182,6 @@ impl Write for EventedWritablePipe {
     }
 }
 
-impl OnResize for PtyHandle {
-    fn on_resize(&mut self, sizeinfo: &SizeInfo) {
-        match self {
-            PtyHandle::Winpty(w) => w.resize(sizeinfo),
-            PtyHandle::Conpty(c) => {
-                let mut handle = c.clone();
-                handle.on_resize(sizeinfo)
-            },
-        }
-    }
-}
-
 impl EventedReadWrite for Pty {
     type Reader = EventedReadablePipe;
     type Writer = EventedWritablePipe;
@@ -308,3 +289,12 @@ impl EventedPty for Pty {
         }
     }
 }
+
+impl OnResize for Pty {
+    fn on_resize(&mut self, size: &SizeInfo) {
+        match &mut self.backend {
+            PtyBackend::Winpty(w) => w.on_resize(size),
+            PtyBackend::Conpty(c) => c.on_resize(size),
+        }
+    }
+}

alacritty_terminal/src/tty/windows/winpty.rs

@@ -16,7 +16,6 @@ use std::fs::OpenOptions;
 use std::io;
 use std::os::windows::fs::OpenOptionsExt;
 use std::os::windows::io::{FromRawHandle, IntoRawHandle};
-use std::sync::Arc;
 use std::u16;
 
 use dunce::canonicalize;
@@ -31,45 +30,7 @@ use crate::term::SizeInfo;
 use crate::tty::windows::child::ChildExitWatcher;
 use crate::tty::windows::Pty;
 
-// We store a raw pointer because we need mutable access to call
+pub use winpty::Winpty as Agent;
-// on_resize from a separate thread. Winpty internally uses a mutex
-// so this is safe, despite outwards appearance.
-pub struct Agent {
-    winpty: *mut Winpty,
-}
-
-/// Handle can be cloned freely and moved between threads.
-pub type WinptyHandle = Arc<Agent>;
-
-// Because Winpty has a mutex, we can do this.
-unsafe impl Send for Agent {}
-unsafe impl Sync for Agent {}
-
-impl Agent {
-    pub fn new(winpty: Winpty) -> Self {
-        Self { winpty: Box::into_raw(Box::new(winpty)) }
-    }
-
-    /// Get immutable access to Winpty.
-    pub fn winpty(&self) -> &Winpty {
-        unsafe { &*self.winpty }
-    }
-
-    pub fn resize(&self, size: &SizeInfo) {
-        // This is safe since Winpty uses a mutex internally.
-        unsafe {
-            (&mut *self.winpty).on_resize(size);
-        }
-    }
-}
-
-impl Drop for Agent {
-    fn drop(&mut self) {
-        unsafe {
-            Box::from_raw(self.winpty);
-        }
-    }
-}
 
 /// How long the winpty agent should wait for any RPC request
 /// This is a placeholder value until we see how often long responses happen
@@ -84,8 +45,8 @@ pub fn new<C>(config: &Config<C>, size: &SizeInfo, _window_id: Option<usize>) ->
     wconfig.set_agent_timeout(AGENT_TIMEOUT);
 
     // Start agent
-    let mut winpty = Winpty::open(&wconfig).unwrap();
+    let mut agent = Winpty::open(&wconfig).unwrap();
-    let (conin, conout) = (winpty.conin_name(), winpty.conout_name());
+    let (conin, conout) = (agent.conin_name(), agent.conout_name());
 
     // Get process commandline
     let default_shell = &Shell::new("powershell");
@@ -134,13 +95,12 @@ pub fn new<C>(config: &Config<C>, size: &SizeInfo, _window_id: Option<usize>) ->
     }
     assert!(conin_pipe.take_error().unwrap().is_none());
 
-    winpty.spawn(&spawnconfig).unwrap();
+    agent.spawn(&spawnconfig).unwrap();
 
-    let child_watcher = ChildExitWatcher::new(winpty.raw_handle()).unwrap();
+    let child_watcher = ChildExitWatcher::new(agent.raw_handle()).unwrap();
-    let agent = Agent::new(winpty);
 
     Pty {
-        handle: super::PtyHandle::Winpty(WinptyHandle::new(agent)),
+        backend: super::PtyBackend::Winpty(agent),
         conout: super::EventedReadablePipe::Named(conout_pipe),
         conin: super::EventedWritablePipe::Named(conin_pipe),
         read_token: 0.into(),
@@ -150,7 +110,7 @@ pub fn new<C>(config: &Config<C>, size: &SizeInfo, _window_id: Option<usize>) ->
     }
 }
 
-impl OnResize for Winpty {
+impl OnResize for Agent {
     fn on_resize(&mut self, sizeinfo: &SizeInfo) {
         let (cols, lines) = (sizeinfo.cols().0, sizeinfo.lines().0);
         if cols > 0 && cols <= u16::MAX as usize && lines > 0 && lines <= u16::MAX as usize {