From 429e415ecca4948563d6e5b81f7270a98963f6bb Mon Sep 17 00:00:00 2001 From: Aram Drevekenin Date: Thu, 19 Nov 2020 18:24:34 +0100 Subject: [PATCH] fix(atomicity): block commands before executing others (#59) --- src/command_is_executing.rs | 52 ++++++++ src/layout.rs | 6 +- src/main.rs | 230 +++++++++++++++++++----------------- src/pty_bus.rs | 3 +- src/screen.rs | 4 +- src/tests/fakes.rs | 43 +++++-- 6 files changed, 217 insertions(+), 121 deletions(-) create mode 100644 src/command_is_executing.rs diff --git a/src/command_is_executing.rs b/src/command_is_executing.rs new file mode 100644 index 00000000..0cf47c05 --- /dev/null +++ b/src/command_is_executing.rs @@ -0,0 +1,52 @@ +use std::sync::{Arc, Condvar, Mutex}; + +#[derive(Clone)] +pub struct CommandIsExecuting { + opening_new_pane: Arc<(Mutex, Condvar)>, + closing_pane: Arc<(Mutex, Condvar)>, +} + +impl CommandIsExecuting { + pub fn new() -> Self { + CommandIsExecuting { + opening_new_pane: Arc::new((Mutex::new(false), Condvar::new())), + closing_pane: Arc::new((Mutex::new(false), Condvar::new())), + } + } + pub fn closing_pane(&mut self) { + let (lock, _cvar) = &*self.closing_pane; + let mut closing_pane = lock.lock().unwrap(); + *closing_pane = true; + } + pub fn done_closing_pane(&mut self) { + let (lock, cvar) = &*self.closing_pane; + let mut closing_pane = lock.lock().unwrap(); + *closing_pane = false; + cvar.notify_one(); + } + pub fn opening_new_pane(&mut self) { + let (lock, _cvar) = &*self.opening_new_pane; + let mut opening_new_pane = lock.lock().unwrap(); + *opening_new_pane = true; + } + pub fn done_opening_new_pane(&mut self) { + let (lock, cvar) = &*self.opening_new_pane; + let mut opening_new_pane = lock.lock().unwrap(); + *opening_new_pane = false; + cvar.notify_one(); + } + pub fn wait_until_pane_is_closed(&self) { + let (lock, cvar) = &*self.closing_pane; + let mut closing_pane = lock.lock().unwrap(); + while *closing_pane { + closing_pane = cvar.wait(closing_pane).unwrap(); + } + } + pub fn wait_until_new_pane_is_opened(&self) { + let (lock, cvar) = &*self.opening_new_pane; + let mut opening_new_pane = lock.lock().unwrap(); + while *opening_new_pane { + opening_new_pane = cvar.wait(opening_new_pane).unwrap(); + } + } +} diff --git a/src/layout.rs b/src/layout.rs index f4297bfd..d557a17c 100644 --- a/src/layout.rs +++ b/src/layout.rs @@ -87,18 +87,18 @@ fn split_space(space_to_split: &PositionAndSize, layout: &Layout) -> Vec, opts: Opt) { let mut active_threads = vec![]; + let command_is_executing = CommandIsExecuting::new(); + delete_log_dir().unwrap(); delete_log_file().unwrap(); @@ -128,6 +133,7 @@ pub fn start(mut os_input: Box, opts: Opt) { thread::Builder::new() .name("pty".to_string()) .spawn({ + let mut command_is_executing = command_is_executing.clone(); move || { match opts.layout { Some(layout_path) => { @@ -166,6 +172,7 @@ pub fn start(mut os_input: Box, opts: Opt) { } PtyInstruction::ClosePane(id) => { pty_bus.close_pane(id); + command_is_executing.done_closing_pane(); } PtyInstruction::Quit => { break; @@ -181,6 +188,7 @@ pub fn start(mut os_input: Box, opts: Opt) { thread::Builder::new() .name("screen".to_string()) .spawn({ + let mut command_is_executing = command_is_executing.clone(); move || loop { let event = screen .receiver @@ -195,12 +203,15 @@ pub fn start(mut os_input: Box, opts: Opt) { } ScreenInstruction::NewPane(pid) => { screen.new_pane(pid); + command_is_executing.done_opening_new_pane(); } ScreenInstruction::HorizontalSplit(pid) => { screen.horizontal_split(pid); + command_is_executing.done_opening_new_pane(); } ScreenInstruction::VerticalSplit(pid) => { screen.vertical_split(pid); + command_is_executing.done_opening_new_pane(); } ScreenInstruction::WriteCharacter(bytes) => { screen.write_to_active_terminal(bytes); @@ -306,113 +317,120 @@ pub fn start(mut os_input: Box, opts: Opt) { }) .unwrap(); - let _stdin_thread = thread::Builder::new() - .name("ipc_server".to_string()) - .spawn({ - let send_screen_instructions = send_screen_instructions.clone(); - let send_pty_instructions = send_pty_instructions.clone(); - let send_app_instructions = send_app_instructions.clone(); - let os_input = os_input.clone(); - move || { - let mut stdin = os_input.get_stdin_reader(); - loop { - let mut buffer = [0; 10]; // TODO: more accurately - stdin.read(&mut buffer).expect("failed to read stdin"); - // uncomment this to print the entered character to a log file (/tmp/mosaic-log.txt) for debugging - //crate::utils::logging::debug_log_to_file(format!("buffer {:?}", buffer)); - match buffer { - [10, 0, 0, 0, 0, 0, 0, 0, 0, 0] => { - // ctrl-j - send_screen_instructions - .send(ScreenInstruction::ResizeDown) - .unwrap(); - } - [11, 0, 0, 0, 0, 0, 0, 0, 0, 0] => { - // ctrl-k - send_screen_instructions - .send(ScreenInstruction::ResizeUp) - .unwrap(); - } - [16, 0, 0, 0, 0, 0, 0, 0, 0, 0] => { - // ctrl-p - send_screen_instructions - .send(ScreenInstruction::MoveFocus) - .unwrap(); - } - [8, 0, 0, 0, 0, 0, 0, 0, 0, 0] => { - // ctrl-h - send_screen_instructions - .send(ScreenInstruction::ResizeLeft) - .unwrap(); - } - [12, 0, 0, 0, 0, 0, 0, 0, 0, 0] => { - // ctrl-l - send_screen_instructions - .send(ScreenInstruction::ResizeRight) - .unwrap(); - } - [26, 0, 0, 0, 0, 0, 0, 0, 0, 0] => { - // ctrl-z - send_pty_instructions - .send(PtyInstruction::SpawnTerminal(None)) - .unwrap(); - } - [14, 0, 0, 0, 0, 0, 0, 0, 0, 0] => { - // ctrl-n - send_pty_instructions - .send(PtyInstruction::SpawnTerminalVertically(None)) - .unwrap(); - } - [2, 0, 0, 0, 0, 0, 0, 0, 0, 0] => { - // ctrl-b - send_pty_instructions - .send(PtyInstruction::SpawnTerminalHorizontally(None)) - .unwrap(); - } - [17, 0, 0, 0, 0, 0, 0, 0, 0, 0] => { - // ctrl-q - let _ = send_screen_instructions.send(ScreenInstruction::Quit); - let _ = send_pty_instructions.send(PtyInstruction::Quit); - let _ = send_app_instructions.send(AppInstruction::Exit); - break; - } - [27, 91, 53, 94, 0, 0, 0, 0, 0, 0] => { - // ctrl-PgUp - send_screen_instructions - .send(ScreenInstruction::ScrollUp) - .unwrap(); - } - [27, 91, 54, 94, 0, 0, 0, 0, 0, 0] => { - // ctrl-PgDown - send_screen_instructions - .send(ScreenInstruction::ScrollDown) - .unwrap(); - } - [24, 0, 0, 0, 0, 0, 0, 0, 0, 0] => { - // ctrl-x - send_screen_instructions - .send(ScreenInstruction::CloseFocusedPane) - .unwrap(); - // ::std::thread::sleep(::std::time::Duration::from_millis(10)); - } - [5, 0, 0, 0, 0, 0, 0, 0, 0, 0] => { - // ctrl-e - send_screen_instructions - .send(ScreenInstruction::ToggleActiveTerminalFullscreen) - .unwrap(); - } - _ => { - send_screen_instructions - .send(ScreenInstruction::ClearScroll) - .unwrap(); - send_screen_instructions - .send(ScreenInstruction::WriteCharacter(buffer)) - .unwrap(); - } + let _stdin_thread = thread::Builder::new().name("stdin".to_string()).spawn({ + let send_screen_instructions = send_screen_instructions.clone(); + let send_pty_instructions = send_pty_instructions.clone(); + let send_app_instructions = send_app_instructions.clone(); + let os_input = os_input.clone(); + + let mut command_is_executing = command_is_executing.clone(); + move || { + let mut stdin = os_input.get_stdin_reader(); + loop { + let mut buffer = [0; 10]; // TODO: more accurately + stdin.read(&mut buffer).expect("failed to read stdin"); + // uncomment this to print the entered character to a log file (/tmp/mosaic-log.txt) for debugging + //crate::utils::logging::debug_log_to_file(format!("buffer {:?}", buffer)); + match buffer { + [10, 0, 0, 0, 0, 0, 0, 0, 0, 0] => { + // ctrl-j + send_screen_instructions + .send(ScreenInstruction::ResizeDown) + .unwrap(); + } + [11, 0, 0, 0, 0, 0, 0, 0, 0, 0] => { + // ctrl-k + send_screen_instructions + .send(ScreenInstruction::ResizeUp) + .unwrap(); + } + [16, 0, 0, 0, 0, 0, 0, 0, 0, 0] => { + // ctrl-p + send_screen_instructions + .send(ScreenInstruction::MoveFocus) + .unwrap(); + } + [8, 0, 0, 0, 0, 0, 0, 0, 0, 0] => { + // ctrl-h + send_screen_instructions + .send(ScreenInstruction::ResizeLeft) + .unwrap(); + } + [12, 0, 0, 0, 0, 0, 0, 0, 0, 0] => { + // ctrl-l + send_screen_instructions + .send(ScreenInstruction::ResizeRight) + .unwrap(); + } + [26, 0, 0, 0, 0, 0, 0, 0, 0, 0] => { + // ctrl-z + command_is_executing.opening_new_pane(); + send_pty_instructions + .send(PtyInstruction::SpawnTerminal(None)) + .unwrap(); + command_is_executing.wait_until_new_pane_is_opened(); + } + [14, 0, 0, 0, 0, 0, 0, 0, 0, 0] => { + // ctrl-n + command_is_executing.opening_new_pane(); + send_pty_instructions + .send(PtyInstruction::SpawnTerminalVertically(None)) + .unwrap(); + command_is_executing.wait_until_new_pane_is_opened(); + } + [2, 0, 0, 0, 0, 0, 0, 0, 0, 0] => { + // ctrl-b + command_is_executing.opening_new_pane(); + send_pty_instructions + .send(PtyInstruction::SpawnTerminalHorizontally(None)) + .unwrap(); + command_is_executing.wait_until_new_pane_is_opened(); + } + [17, 0, 0, 0, 0, 0, 0, 0, 0, 0] => { + // ctrl-q + let _ = send_screen_instructions.send(ScreenInstruction::Quit); + let _ = send_pty_instructions.send(PtyInstruction::Quit); + let _ = send_app_instructions.send(AppInstruction::Exit); + break; + } + [27, 91, 53, 94, 0, 0, 0, 0, 0, 0] => { + // ctrl-PgUp + send_screen_instructions + .send(ScreenInstruction::ScrollUp) + .unwrap(); + } + [27, 91, 54, 94, 0, 0, 0, 0, 0, 0] => { + // ctrl-PgDown + send_screen_instructions + .send(ScreenInstruction::ScrollDown) + .unwrap(); + } + [24, 0, 0, 0, 0, 0, 0, 0, 0, 0] => { + // ctrl-x + command_is_executing.closing_pane(); + send_screen_instructions + .send(ScreenInstruction::CloseFocusedPane) + .unwrap(); + command_is_executing.wait_until_pane_is_closed(); + } + [5, 0, 0, 0, 0, 0, 0, 0, 0, 0] => { + // ctrl-e + send_screen_instructions + .send(ScreenInstruction::ToggleActiveTerminalFullscreen) + .unwrap(); + } + _ => { + send_screen_instructions + .send(ScreenInstruction::ClearScroll) + .unwrap(); + send_screen_instructions + .send(ScreenInstruction::WriteCharacter(buffer)) + .unwrap(); } } } - }); + } + }); loop { let app_instruction = receive_app_instructions diff --git a/src/pty_bus.rs b/src/pty_bus.rs index 49334736..ee6d8247 100644 --- a/src/pty_bus.rs +++ b/src/pty_bus.rs @@ -60,7 +60,7 @@ impl Stream for ReadFromPid { } } -#[derive(Debug)] +#[derive(Debug, Clone)] pub enum VteEvent { // TODO: try not to allocate Vecs Print(char), @@ -141,6 +141,7 @@ impl vte::Perform for VteEventSender { } } +#[derive(Clone, Debug)] pub enum PtyInstruction { SpawnTerminal(Option), SpawnTerminalVertically(Option), diff --git a/src/screen.rs b/src/screen.rs index 8466eb0c..10c4a2cc 100644 --- a/src/screen.rs +++ b/src/screen.rs @@ -48,7 +48,7 @@ fn split_horizontally_with_gap(rect: &PositionAndSize) -> (PositionAndSize, Posi (first_rect, second_rect) } -#[derive(Debug)] +#[derive(Debug, Clone)] pub enum ScreenInstruction { Pty(RawFd, VteEvent), Render, @@ -1469,10 +1469,10 @@ impl Screen { } pub fn close_focused_pane(&mut self) { if let Some(active_terminal_id) = self.get_active_terminal_id() { + self.close_pane(active_terminal_id); self.send_pty_instructions .send(PtyInstruction::ClosePane(active_terminal_id)) .unwrap(); - self.close_pane(active_terminal_id); } } pub fn scroll_active_terminal_up(&mut self) { diff --git a/src/tests/fakes.rs b/src/tests/fakes.rs index 79a57f86..778a77ab 100644 --- a/src/tests/fakes.rs +++ b/src/tests/fakes.rs @@ -4,11 +4,13 @@ use ::std::io::{Read, Write}; use ::std::os::unix::io::RawFd; use ::std::path::PathBuf; use ::std::sync::{Arc, Mutex}; -use ::std::time::Duration; +use ::std::time::{Duration, Instant}; use crate::os_input_output::OsApi; use crate::tests::possible_tty_inputs::{get_possible_tty_inputs, Bytes}; +const MIN_TIME_BETWEEN_SNAPSHOTS: Duration = Duration::from_millis(50); + #[derive(Clone)] pub enum IoEvent { Kill(RawFd), @@ -21,23 +23,29 @@ pub enum IoEvent { pub struct FakeStdinReader { pub input_chars: Vec<[u8; 10]>, pub read_position: usize, + last_snapshot_time: Arc>, } impl FakeStdinReader { - pub fn new(input_chars: Vec<[u8; 10]>) -> Self { + pub fn new(input_chars: Vec<[u8; 10]>, last_snapshot_time: Arc>) -> Self { FakeStdinReader { input_chars, read_position: 0, + last_snapshot_time, } } } impl Read for FakeStdinReader { fn read(&mut self, buf: &mut [u8]) -> Result { - // ideally, we shouldn't have to sleep here - // stdin should be buffered and handled in the app itself - ::std::thread::sleep(Duration::from_millis(50)); - // ::std::thread::sleep(Duration::from_millis(100)); + loop { + let last_snapshot_time = { *self.last_snapshot_time.lock().unwrap() }; + if last_snapshot_time.elapsed() > MIN_TIME_BETWEEN_SNAPSHOTS { + break; + } else { + ::std::thread::sleep(MIN_TIME_BETWEEN_SNAPSHOTS - last_snapshot_time.elapsed()); + } + } let read_position = self.read_position; let bytes_to_read = self.input_chars.get(read_position).unwrap(); for (i, byte) in bytes_to_read.iter().enumerate() { @@ -48,10 +56,21 @@ impl Read for FakeStdinReader { } } -#[derive(Clone, Default)] +#[derive(Clone)] pub struct FakeStdoutWriter { output_buffer: Arc>>, pub output_frames: Arc>>>, + last_snapshot_time: Arc>, +} + +impl FakeStdoutWriter { + pub fn new(last_snapshot_time: Arc>) -> Self { + FakeStdoutWriter { + output_buffer: Arc::new(Mutex::new(Vec::new())), + output_frames: Arc::new(Mutex::new(Vec::new())), + last_snapshot_time, + } + } } impl Write for FakeStdoutWriter { @@ -69,6 +88,8 @@ impl Write for FakeStdoutWriter { let mut output_frames = self.output_frames.lock().unwrap(); let new_frame = output_buffer.drain(..).collect(); output_frames.push(new_frame); + let mut last_snapshot_time = self.last_snapshot_time.lock().unwrap(); + *last_snapshot_time = Instant::now(); Ok(()) } } @@ -82,17 +103,21 @@ pub struct FakeInputOutput { io_events: Arc>>, win_sizes: Arc>>, possible_tty_inputs: HashMap, + last_snapshot_time: Arc>, } impl FakeInputOutput { pub fn new(winsize: PositionAndSize) -> Self { let mut win_sizes = HashMap::new(); + let last_snapshot_time = Arc::new(Mutex::new(Instant::now())); + let stdout_writer = FakeStdoutWriter::new(last_snapshot_time.clone()); win_sizes.insert(0, winsize); // 0 is the current terminal FakeInputOutput { read_buffers: Arc::new(Mutex::new(HashMap::new())), stdin_writes: Arc::new(Mutex::new(HashMap::new())), input_to_add: Arc::new(Mutex::new(None)), - stdout_writer: FakeStdoutWriter::default(), + stdout_writer, + last_snapshot_time, io_events: Arc::new(Mutex::new(vec![])), win_sizes: Arc::new(Mutex::new(win_sizes)), possible_tty_inputs: get_possible_tty_inputs(), @@ -205,7 +230,7 @@ impl OsApi for FakeInputOutput { } } input_chars.push([17, 0, 0, 0, 0, 0, 0, 0, 0, 0]); // ctrl-q (quit) - let reader = FakeStdinReader::new(input_chars); + let reader = FakeStdinReader::new(input_chars, self.last_snapshot_time.clone()); Box::new(reader) } fn get_stdout_writer(&self) -> Box {