From f1499d1d4518674c6c3c5c859a7028709bdd741d Mon Sep 17 00:00:00 2001 From: Joe Wilm Date: Sat, 24 Sep 2016 16:11:50 -0700 Subject: [PATCH] Use evented I/O for the pty This was largely an experiment to see whether writing and reading from a separate thread was causing terminal state corruption as described in https://github.com/jwilm/alacritty/issues/9. Although this doesn't seem to fix that particular issue. Keeping this because it generally seems more correct than reading/writing from separate locations. --- Cargo.lock | 50 ++++++++++ Cargo.toml | 1 + src/event.rs | 29 +++--- src/input.rs | 28 +++++- src/main.rs | 254 ++++++++++++++++++++++++++++++++++++++++++++------- src/tty.rs | 13 +++ 6 files changed, 322 insertions(+), 53 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index e6cd8da2..c93ffe72 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -9,6 +9,7 @@ dependencies = [ "gl_generator 0.5.2 (registry+https://github.com/rust-lang/crates.io-index)", "glutin 0.6.1 (git+https://github.com/jwilm/glutin?rev=16ed800f68022a37203434bd3b542ff5400c1403)", "libc 0.2.16 (registry+https://github.com/rust-lang/crates.io-index)", + "mio 0.6.0 (registry+https://github.com/rust-lang/crates.io-index)", "notify 2.6.3 (registry+https://github.com/rust-lang/crates.io-index)", "parking_lot 0.3.2 (registry+https://github.com/rust-lang/crates.io-index)", "serde 0.8.8 (registry+https://github.com/rust-lang/crates.io-index)", @@ -370,6 +371,11 @@ name = "lazy_static" version = "0.2.1" source = "registry+https://github.com/rust-lang/crates.io-index" +[[package]] +name = "lazycell" +version = "0.4.0" +source = "registry+https://github.com/rust-lang/crates.io-index" + [[package]] name = "libc" version = "0.2.16" @@ -436,6 +442,22 @@ dependencies = [ "winapi 0.2.8 (registry+https://github.com/rust-lang/crates.io-index)", ] +[[package]] +name = "mio" +version = "0.6.0" +source = "registry+https://github.com/rust-lang/crates.io-index" +dependencies = [ + "kernel32-sys 0.2.2 (registry+https://github.com/rust-lang/crates.io-index)", + "lazycell 0.4.0 (registry+https://github.com/rust-lang/crates.io-index)", + "libc 0.2.16 (registry+https://github.com/rust-lang/crates.io-index)", + "log 0.3.6 (registry+https://github.com/rust-lang/crates.io-index)", + "miow 0.1.3 (registry+https://github.com/rust-lang/crates.io-index)", + "net2 0.2.26 (registry+https://github.com/rust-lang/crates.io-index)", + "nix 0.6.0 (registry+https://github.com/rust-lang/crates.io-index)", + "slab 0.3.0 (registry+https://github.com/rust-lang/crates.io-index)", + "winapi 0.2.8 (registry+https://github.com/rust-lang/crates.io-index)", +] + [[package]] name = "miow" version = "0.1.3" @@ -468,6 +490,19 @@ dependencies = [ "libc 0.2.16 (registry+https://github.com/rust-lang/crates.io-index)", ] +[[package]] +name = "nix" +version = "0.6.0" +source = "registry+https://github.com/rust-lang/crates.io-index" +dependencies = [ + "bitflags 0.4.0 (registry+https://github.com/rust-lang/crates.io-index)", + "cfg-if 0.1.0 (registry+https://github.com/rust-lang/crates.io-index)", + "libc 0.2.16 (registry+https://github.com/rust-lang/crates.io-index)", + "rustc_version 0.1.7 (registry+https://github.com/rust-lang/crates.io-index)", + "semver 0.1.20 (registry+https://github.com/rust-lang/crates.io-index)", + "void 1.0.2 (registry+https://github.com/rust-lang/crates.io-index)", +] + [[package]] name = "notify" version = "2.6.3" @@ -737,6 +772,11 @@ name = "slab" version = "0.1.3" source = "registry+https://github.com/rust-lang/crates.io-index" +[[package]] +name = "slab" +version = "0.3.0" +source = "registry+https://github.com/rust-lang/crates.io-index" + [[package]] name = "smallvec" version = "0.1.8" @@ -786,6 +826,11 @@ name = "utf8parse" version = "0.1.0" source = "registry+https://github.com/rust-lang/crates.io-index" +[[package]] +name = "void" +version = "1.0.2" +source = "registry+https://github.com/rust-lang/crates.io-index" + [[package]] name = "vte" version = "0.1.2" @@ -940,6 +985,7 @@ source = "registry+https://github.com/rust-lang/crates.io-index" "checksum khronos_api 1.0.0 (registry+https://github.com/rust-lang/crates.io-index)" = "09c9d3760673c427d46f91a0350f0a84a52e6bc5a84adf26dc610b6c52436630" "checksum lazy_static 0.1.16 (registry+https://github.com/rust-lang/crates.io-index)" = "cf186d1a8aa5f5bee5fd662bc9c1b949e0259e1bcc379d1f006847b0080c7417" "checksum lazy_static 0.2.1 (registry+https://github.com/rust-lang/crates.io-index)" = "49247ec2a285bb3dcb23cbd9c35193c025e7251bfce77c1d5da97e6362dffe7f" +"checksum lazycell 0.4.0 (registry+https://github.com/rust-lang/crates.io-index)" = "ce12306c4739d86ee97c23139f3a34ddf0387bbf181bc7929d287025a8c3ef6b" "checksum libc 0.2.16 (registry+https://github.com/rust-lang/crates.io-index)" = "408014cace30ee0f767b1c4517980646a573ec61a57957aeeabcac8ac0a02e8d" "checksum libloading 0.2.4 (registry+https://github.com/rust-lang/crates.io-index)" = "eceb2637ee9a27c7f19764048a9f377e40e3a70a322722f348e6bc7704d565f2" "checksum libz-sys 1.0.6 (registry+https://github.com/rust-lang/crates.io-index)" = "40f2df7730b5d29426c3e44ce4d088d8c5def6471c2c93ba98585b89fb201ce6" @@ -947,9 +993,11 @@ source = "registry+https://github.com/rust-lang/crates.io-index" "checksum malloc_buf 0.0.6 (registry+https://github.com/rust-lang/crates.io-index)" = "62bb907fe88d54d8d9ce32a3cceab4218ed2f6b7d35617cafe9adf84e43919cb" "checksum memmap 0.2.3 (registry+https://github.com/rust-lang/crates.io-index)" = "f20f72ed93291a72e22e8b16bb18762183bb4943f0f483da5b8be1a9e8192752" "checksum mio 0.5.1 (registry+https://github.com/rust-lang/crates.io-index)" = "a637d1ca14eacae06296a008fa7ad955347e34efcb5891cfd8ba05491a37907e" +"checksum mio 0.6.0 (registry+https://github.com/rust-lang/crates.io-index)" = "2dadd39d4b47343e10513ac2a731c979517a4761224ecb6bbd243602300c9537" "checksum miow 0.1.3 (registry+https://github.com/rust-lang/crates.io-index)" = "d5bfc6782530ac8ace97af10a540054a37126b63b0702ddaaa243b73b5745b9a" "checksum net2 0.2.26 (registry+https://github.com/rust-lang/crates.io-index)" = "5edf9cb6be97212423aed9413dd4729d62b370b5e1c571750e882cebbbc1e3e2" "checksum nix 0.5.1 (registry+https://github.com/rust-lang/crates.io-index)" = "bfb3ddedaa14746434a02041940495bf11325c22f6d36125d3bdd56090d50a79" +"checksum nix 0.6.0 (registry+https://github.com/rust-lang/crates.io-index)" = "7a7bb1da2be7da3cbffda73fc681d509ffd9e665af478d2bee1907cee0bc64b2" "checksum notify 2.6.3 (registry+https://github.com/rust-lang/crates.io-index)" = "4e0e7eec936337952c4228b023007528a33b2fa039d96c2e8f32d764221a9c07" "checksum num 0.1.35 (registry+https://github.com/rust-lang/crates.io-index)" = "5a9699207fab8b02bd0e56f8f06fee3f26d640303130de548898b4c9704f6d01" "checksum num-bigint 0.1.35 (registry+https://github.com/rust-lang/crates.io-index)" = "88b14378471f7c2adc5262f05b4701ef53e8da376453a8d8fee48e51db745e49" @@ -982,12 +1030,14 @@ source = "registry+https://github.com/rust-lang/crates.io-index" "checksum shared_library 0.1.5 (registry+https://github.com/rust-lang/crates.io-index)" = "fb04126b6fcfd2710fb5b6d18f4207b6c535f2850a7e1a43bcd526d44f30a79a" "checksum shell32-sys 0.1.1 (registry+https://github.com/rust-lang/crates.io-index)" = "72f20b8f3c060374edb8046591ba28f62448c369ccbdc7b02075103fb3a9e38d" "checksum slab 0.1.3 (registry+https://github.com/rust-lang/crates.io-index)" = "d807fd58c4181bbabed77cb3b891ba9748241a552bcc5be698faaebefc54f46e" +"checksum slab 0.3.0 (registry+https://github.com/rust-lang/crates.io-index)" = "17b4fcaed89ab08ef143da37bc52adbcc04d4a69014f4c1208d6b51f0c47bc23" "checksum smallvec 0.1.8 (registry+https://github.com/rust-lang/crates.io-index)" = "fcc8d19212aacecf95e4a7a2179b26f7aeb9732a915cf01f05b0d3e044865410" "checksum target_build_utils 0.1.1 (registry+https://github.com/rust-lang/crates.io-index)" = "7a1be18d4d908e4e5697908de04fdd5099505463fc8eaf1ceb8133ae486936aa" "checksum tempfile 2.1.4 (registry+https://github.com/rust-lang/crates.io-index)" = "9270837a93bad1b1dac18fe67e786b3c960513af86231f6f4f57fddd594ff0c8" "checksum time 0.1.35 (registry+https://github.com/rust-lang/crates.io-index)" = "3c7ec6d62a20df54e07ab3b78b9a3932972f4b7981de295563686849eb3989af" "checksum user32-sys 0.1.2 (registry+https://github.com/rust-lang/crates.io-index)" = "6717129de5ac253f5642fc78a51d0c7de6f9f53d617fc94e9bae7f6e71cf5504" "checksum utf8parse 0.1.0 (registry+https://github.com/rust-lang/crates.io-index)" = "a15ea87f3194a3a454c78d79082b4f5e85f6956ddb6cb86bbfbe4892aa3c0323" +"checksum void 1.0.2 (registry+https://github.com/rust-lang/crates.io-index)" = "6a02e4885ed3bc0f2de90ea6dd45ebcbb66dacffe03547fadbb0eeae2770887d" "checksum vte 0.1.2 (registry+https://github.com/rust-lang/crates.io-index)" = "182acba7edb46d25ab5ddfc9f390b8c6cc1dcec7a4efc2798f0a8918f07d8c8a" "checksum walkdir 0.1.8 (registry+https://github.com/rust-lang/crates.io-index)" = "c66c0b9792f0a765345452775f3adbd28dde9d33f30d13e5dcc5ae17cf6f3780" "checksum wayland-client 0.5.12 (registry+https://github.com/rust-lang/crates.io-index)" = "ced3094c157b5cc0a08d40530e1a627d9f88b9a436971338d2646439128a559e" diff --git a/Cargo.toml b/Cargo.toml index a8067ccb..56a5b71b 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -18,6 +18,7 @@ serde = "0.8" serde_yaml = "0.4" serde_macros = "0.8" vte = "0.1.2" +mio = "0.6" [build-dependencies] gl_generator = "0.5" diff --git a/src/event.rs b/src/event.rs index 303c6d8e..f44fd86c 100644 --- a/src/event.rs +++ b/src/event.rs @@ -1,4 +1,5 @@ //! Process window events +use std::io; use std::sync::{Arc, mpsc}; use std; @@ -9,27 +10,25 @@ use sync::FairMutex; use term::Term; /// The event processor -pub struct Processor<'a, W: 'a> { - writer: &'a mut W, +pub struct Processor { + notifier: N, input_processor: input::Processor, terminal: Arc>, resize_tx: mpsc::Sender<(u32, u32)>, } -impl<'a, W> Processor<'a, W> - where W: std::io::Write -{ +impl Processor { /// 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(writer: &mut W, - terminal: Arc>, - resize_tx: mpsc::Sender<(u32, u32)>) - -> Processor - { + pub fn new( + notifier: N, + terminal: Arc>, + resize_tx: mpsc::Sender<(u32, u32)> + ) -> Processor { Processor { - writer: writer, + notifier: notifier, terminal: terminal, input_processor: input::Processor::new(), resize_tx: resize_tx, @@ -47,7 +46,7 @@ impl<'a, W> Processor<'a, W> '\u{f700}' | '\u{f701}' | '\u{f702}' | '\u{f703}' => (), _ => { let encoded = c.encode_utf8(); - self.writer.write(encoded.as_slice()).unwrap(); + self.notifier.notify(encoded.as_slice().to_vec()); } } }, @@ -60,10 +59,10 @@ impl<'a, W> Processor<'a, W> glutin::Event::KeyboardInput(state, _code, key, mods) => { // Acquire term lock let terminal = self.terminal.lock(); + let processor = &mut self.input_processor; + let notifier = &mut self.notifier; - self.input_processor.process(state, key, mods, - &mut input::WriteNotifier(self.writer), - *terminal.mode()); + processor.process(state, key, mods, notifier, *terminal.mode()); }, _ => (), } diff --git a/src/input.rs b/src/input.rs index 757256d2..829312e7 100644 --- a/src/input.rs +++ b/src/input.rs @@ -23,6 +23,7 @@ //! APIs //! //! TODO handling xmodmap would be good +use std::borrow::Cow; use std::io::Write; use glutin::{ElementState, VirtualKeyCode}; @@ -42,15 +43,34 @@ pub struct Processor; /// Types that are notified of escape sequences from the input::Processor. pub trait Notify { /// Notify that an escape sequence should be written to the pty - fn notify(&mut self, &str); + /// + /// TODO this needs to be able to error somehow + fn notify>>(&mut self, B); } /// A notifier type that simply writes bytes to the provided `Write` type pub struct WriteNotifier<'a, W: Write + 'a>(pub &'a mut W); impl<'a, W: Write> Notify for WriteNotifier<'a, W> { - fn notify(&mut self, message: &str) { - self.0.write_all(message.as_bytes()).unwrap(); + fn notify(&mut self, bytes: B) + where B: Into> + { + let message = bytes.into(); + self.0.write_all(&message[..]).unwrap(); + } +} + +pub struct LoopNotifier(pub ::mio::channel::Sender<::EventLoopMessage>); + +impl Notify for LoopNotifier { + fn notify(&mut self, bytes: B) + where B: Into> + { + let bytes = bytes.into(); + match self.0.send(::EventLoopMessage::Input(bytes)) { + Ok(_) => (), + Err(_) => panic!("expected send event loop msg"), + } } } @@ -277,7 +297,7 @@ impl Processor { // Modifier keys if binding.mods.is_all() || mods.intersects(binding.mods) { // everything matches - notifier.notify(binding.send); + notifier.notify(binding.send.as_bytes()); break; } } diff --git a/src/main.rs b/src/main.rs index e520b08d..c6694ce9 100644 --- a/src/main.rs +++ b/src/main.rs @@ -28,6 +28,7 @@ extern crate errno; extern crate font; extern crate glutin; extern crate libc; +extern crate mio; extern crate notify; extern crate parking_lot; extern crate serde; @@ -54,6 +55,7 @@ mod util; mod sync; use std::sync::{mpsc, Arc}; +use std::fs::File; use std::sync::atomic::{AtomicBool, Ordering}; use parking_lot::{MutexGuard}; @@ -172,9 +174,7 @@ fn main() { println!("Cell Size: ({} x {})", cell_width, cell_height); let terminal = Term::new(width as f32, height as f32, cell_width as f32, cell_height as f32); - - let reader = terminal.tty().reader(); - let mut writer = terminal.tty().writer(); + let pty_io = terminal.tty().reader(); let (tx, rx) = mpsc::channel(); unsafe { @@ -183,16 +183,20 @@ fn main() { let signal_flag = Flag::new(false); + let terminal = Arc::new(FairMutex::new(terminal)); let window = Arc::new(window); - let pty_reader = PtyReader::spawn( + let event_loop = EventLoop::new( terminal.clone(), - reader, window.create_window_proxy(), - signal_flag.clone() + signal_flag.clone(), + pty_io, ); + let loop_tx = event_loop.channel(); + let event_loop_handle = event_loop.spawn(); + // Wraps a renderer and gives simple draw() api. let mut display = Display::new( window.clone(), @@ -203,7 +207,11 @@ fn main() { ); // Event processor - let mut processor = event::Processor::new(&mut writer, terminal.clone(), tx); + let mut processor = event::Processor::new( + input::LoopNotifier(loop_tx), + terminal.clone(), + tx + ); // Main loop loop { @@ -223,43 +231,221 @@ fn main() { } // shutdown - pty_reader.join().ok(); + event_loop_handle.join().ok(); println!("Goodbye"); } -struct PtyReader; +struct EventLoop { + poll: mio::Poll, + pty: File, + rx: mio::channel::Receiver, + tx: mio::channel::Sender, + terminal: Arc>, + proxy: ::glutin::WindowProxy, + signal_flag: Flag, +} + +const CHANNEL: mio::Token = mio::Token(0); +const PTY: mio::Token = mio::Token(1); + +#[derive(Debug)] +pub enum EventLoopMessage { + Input(::std::borrow::Cow<'static, [u8]>), +} + +impl EventLoop { + pub fn new( + terminal: Arc>, + proxy: ::glutin::WindowProxy, + signal_flag: Flag, + pty: File, + ) -> EventLoop { + let (tx, rx) = ::mio::channel::channel(); + EventLoop { + poll: mio::Poll::new().expect("create mio Poll"), + pty: pty, + tx: tx, + rx: rx, + terminal: terminal, + proxy: proxy, + signal_flag: signal_flag + } + } + + pub fn channel(&self) -> mio::channel::Sender { + self.tx.clone() + } + + pub fn spawn(mut self) -> std::thread::JoinHandle<()> { + use mio::{Events, PollOpt, Ready}; + use mio::unix::EventedFd; + use std::borrow::Cow; + use std::os::unix::io::AsRawFd; + use std::io::{Read, Write, ErrorKind}; + + struct Writing { + source: Cow<'static, [u8]>, + written: usize, + } + + impl Writing { + #[inline] + fn new(c: Cow<'static, [u8]>) -> Writing { + Writing { source: c, written: 0 } + } + + #[inline] + fn advance(&mut self, n: usize) { + self.written += n; + } + + #[inline] + fn remaining_bytes(&self) -> &[u8] { + &self.source[self.written..] + } + + #[inline] + fn finished(&self) -> bool { + self.written >= self.source.len() + } + } -impl PtyReader { - pub fn spawn(terminal: Arc>, - mut pty: R, - proxy: ::glutin::WindowProxy, - signal_flag: Flag) - -> std::thread::JoinHandle<()> - where R: std::io::Read + Send + 'static - { thread::spawn_named("pty reader", move || { + + let EventLoop { mut poll, mut pty, rx, terminal, proxy, signal_flag, .. } = self; + + let mut buf = [0u8; 4096]; let mut pty_parser = ansi::Processor::new(); + let fd = pty.as_raw_fd(); + let fd = EventedFd(&fd); - loop { - if let Ok(got) = pty.read(&mut buf[..]) { - let mut terminal = terminal.lock(); + poll.register(&rx, CHANNEL, Ready::readable(), PollOpt::edge() | PollOpt::oneshot()) + .unwrap(); + poll.register(&fd, PTY, Ready::readable(), PollOpt::edge() | PollOpt::oneshot()) + .unwrap(); - for byte in &buf[..got] { - pty_parser.advance(&mut *terminal, *byte); + let mut events = Events::with_capacity(1024); + let mut write_list = ::std::collections::VecDeque::new(); + let mut writing = None; + + 'event_loop: loop { + poll.poll(&mut events, None).expect("poll ok"); + + for event in events.iter() { + match event.token() { + CHANNEL => { + while let Ok(msg) = rx.try_recv() { + match msg { + EventLoopMessage::Input(input) => { + write_list.push_back(input); + } + } + } + + poll.reregister( + &rx, CHANNEL, + Ready::readable(), + PollOpt::edge() | PollOpt::oneshot() + ).expect("reregister channel"); + + if writing.is_some() || !write_list.is_empty() { + poll.reregister( + &fd, + PTY, + Ready::readable() | Ready::writable(), + PollOpt::edge() | PollOpt::oneshot() + ).expect("reregister fd after channel recv"); + } + }, + PTY => { + let kind = event.kind(); + + if kind.is_readable() { + loop { + match pty.read(&mut buf[..]) { + Ok(0) => break, + Ok(got) => { + let mut terminal = terminal.lock(); + for byte in &buf[..got] { + pty_parser.advance(&mut *terminal, *byte); + } + + terminal.dirty = true; + + // Only wake up the event loop if it hasn't already been + // signaled. This is a really important optimization + // because waking up the event loop redundantly burns *a + // lot* of cycles. + if !signal_flag.get() { + proxy.wakeup_event_loop(); + signal_flag.set(true); + } + }, + Err(err) => { + match err.kind() { + ErrorKind::WouldBlock => break, + _ => panic!("unexpected read err: {:?}", err), + } + } + } + } + } + + if kind.is_writable() { + if writing.is_none() { + writing = write_list + .pop_front() + .map(|c| Writing::new(c)); + } + + 'write_list_loop: while let Some(mut write_now) = writing.take() { + loop { + let start = write_now.written; + match pty.write(write_now.remaining_bytes()) { + Ok(0) => { + writing = Some(write_now); + break 'write_list_loop; + }, + Ok(n) => { + write_now.advance(n); + if write_now.finished() { + writing = write_list + .pop_front() + .map(|next| Writing::new(next)); + + break; + } else { + } + }, + Err(err) => { + writing = Some(write_now); + match err.kind() { + ErrorKind::WouldBlock => break 'write_list_loop, + // TODO + _ => panic!("unexpected err: {:?}", err), + } + } + } + + } + } + } + + if kind.is_hup() { + break 'event_loop; + } + + let mut interest = Ready::readable(); + if writing.is_some() || !write_list.is_empty() { + interest.insert(Ready::writable()); + } + + poll.reregister(&fd, PTY, interest, PollOpt::edge() | PollOpt::oneshot()) + .expect("register fd after read/write"); + }, + _ => (), } - - terminal.dirty = true; - - // Only wake up the event loop if it hasn't already been signaled. This is a - // really important optimization because waking up the event loop redundantly - // burns *a lot* of cycles. - if !signal_flag.get() { - proxy.wakeup_event_loop(); - signal_flag.set(true); - } - } else { - break; } } diff --git a/src/tty.rs b/src/tty.rs index accf0226..988db37e 100644 --- a/src/tty.rs +++ b/src/tty.rs @@ -274,6 +274,12 @@ pub fn new(rows: u8, cols: u8) -> Tty { libc::close(slave); } + unsafe { + // Maybe this should be done outside of this function so nonblocking + // isn't forced upon consumers. Although maybe it should be? + set_nonblocking(master); + } + Tty { fd: master } } } @@ -320,6 +326,13 @@ impl Tty { } } +unsafe fn set_nonblocking(fd: c_int) { + use libc::{fcntl, F_SETFL, F_GETFL, O_NONBLOCK}; + + let res = fcntl(fd, F_SETFL, fcntl(fd, F_GETFL, 0) | O_NONBLOCK); + assert_eq!(res, 0); +} + #[test] fn test_get_pw_entry() { let mut buf: [i8; 1024] = [0; 1024];