fix(atomicity): block commands before executing others (#59)

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<bool>, Condvar)>,
+    closing_pane: Arc<(Mutex<bool>, 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();
+        }
+    }
+}

src/layout.rs

@@ -87,18 +87,18 @@ fn split_space(space_to_split: &PositionAndSize, layout: &Layout) -> Vec<Positio
     pane_positions
 }
 
-#[derive(Debug, Serialize, Deserialize)]
+#[derive(Debug, Serialize, Deserialize, Clone)]
 pub enum Direction {
     Horizontal,
     Vertical,
 }
 
-#[derive(Debug, Serialize, Deserialize)]
+#[derive(Debug, Serialize, Deserialize, Clone)]
 pub enum SplitSize {
     Percent(u8), // 1 to 100
 }
 
-#[derive(Debug, Serialize, Deserialize)]
+#[derive(Debug, Serialize, Deserialize, Clone)]
 pub struct Layout {
     pub direction: Direction,
     #[serde(default, skip_serializing_if = "Vec::is_empty")]

src/main.rs

@@ -1,11 +1,13 @@
+#[cfg(test)]
+mod tests;
+
 mod boundaries;
+mod command_is_executing;
 mod layout;
 mod os_input_output;
 mod pty_bus;
 mod screen;
 mod terminal_pane;
-#[cfg(test)]
-mod tests;
 mod utils;
 
 use std::io::{Read, Write};
@@ -18,6 +20,7 @@ use serde::{Deserialize, Serialize};
 use serde_yaml;
 use structopt::StructOpt;
 
+use crate::command_is_executing::CommandIsExecuting;
 use crate::layout::Layout;
 use crate::os_input_output::{get_os_input, OsApi};
 use crate::pty_bus::{PtyBus, PtyInstruction, VteEvent};
@@ -92,6 +95,8 @@ pub enum AppInstruction {
 pub fn start(mut os_input: Box<dyn OsApi>, 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<dyn OsApi>, 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<dyn OsApi>, 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<dyn OsApi>, 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<dyn OsApi>, 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,13 +317,13 @@ pub fn start(mut os_input: Box<dyn OsApi>, opts: Opt) {
         })
         .unwrap();
 
-    let _stdin_thread = thread::Builder::new()
-        .name("ipc_server".to_string())
-        .spawn({
+    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 {
@@ -353,21 +364,27 @@ pub fn start(mut os_input: Box<dyn OsApi>, opts: Opt) {
                     }
                     [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
@@ -390,10 +407,11 @@ pub fn start(mut os_input: Box<dyn OsApi>, opts: Opt) {
                     }
                     [24, 0, 0, 0, 0, 0, 0, 0, 0, 0] => {
                         // ctrl-x
+                        command_is_executing.closing_pane();
                         send_screen_instructions
                             .send(ScreenInstruction::CloseFocusedPane)
                             .unwrap();
-                            // ::std::thread::sleep(::std::time::Duration::from_millis(10));
+                        command_is_executing.wait_until_pane_is_closed();
                     }
                     [5, 0, 0, 0, 0, 0, 0, 0, 0, 0] => {
                         // ctrl-e

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<PathBuf>),
     SpawnTerminalVertically(Option<PathBuf>),

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) {

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<Mutex<Instant>>,
 }
 
 impl FakeStdinReader {
-    pub fn new(input_chars: Vec<[u8; 10]>) -> Self {
+    pub fn new(input_chars: Vec<[u8; 10]>, last_snapshot_time: Arc<Mutex<Instant>>) -> Self {
         FakeStdinReader {
             input_chars,
             read_position: 0,
+            last_snapshot_time,
         }
     }
 }
 
 impl Read for FakeStdinReader {
     fn read(&mut self, buf: &mut [u8]) -> Result<usize, std::io::Error> {
-        // 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<Mutex<Vec<u8>>>,
     pub output_frames: Arc<Mutex<Vec<Vec<u8>>>>,
+    last_snapshot_time: Arc<Mutex<Instant>>,
+}
+
+impl FakeStdoutWriter {
+    pub fn new(last_snapshot_time: Arc<Mutex<Instant>>) -> 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<Mutex<Vec<IoEvent>>>,
     win_sizes: Arc<Mutex<HashMap<RawFd, PositionAndSize>>>,
     possible_tty_inputs: HashMap<u16, Bytes>,
+    last_snapshot_time: Arc<Mutex<Instant>>,
 }
 
 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<dyn Write> {