fix(unix): forkpty => openpty (#830)

* fix(unix): forkpty => openpty

* style(fmt): make rustfmt happy

6 files changed, 123 insertions, 123 deletions

diff --git a/Cargo.lock b/Cargo.lock
index 39d449fe..f092f778 100644
--- a/Cargo.lock
+++ b/Cargo.lock
@@ -352,6 +352,16 @@ dependencies = [
 ]
 
 [[package]]
+name = "close_fds"
+version = "0.3.2"
+source = "registry+https://github.com/rust-lang/crates.io-index"
+checksum = "3bc416f33de9d59e79e57560f450d21ff8393adcf1cdfc3e6d8fb93d5f88a2ed"
+dependencies = [
+ "cfg-if 1.0.0",
+ "libc",
+]
+
+[[package]]
 name = "colored"
 version = "2.0.0"
 source = "registry+https://github.com/rust-lang/crates.io-index"
@@ -2857,6 +2867,7 @@ dependencies = [
  "byteorder",
  "cassowary",
  "chrono",
+ "close_fds",
  "daemonize",
  "darwin-libproc",
  "highway",
diff --git a/zellij-server/Cargo.toml b/zellij-server/Cargo.toml
index 9d7c075e..ea72f2d0 100644
--- a/zellij-server/Cargo.toml
+++ b/zellij-server/Cargo.toml
@@ -25,6 +25,7 @@ zellij-utils = { path = "../zellij-utils/", version = "0.20.0" }
 log = "0.4.14"
 typetag = "0.1.7"
 chrono = "0.4.19"
+close_fds = "0.3.2"
 
 [target.'cfg(target_os = "macos")'.dependencies]
 darwin-libproc = "0.2.0"
diff --git a/zellij-server/src/os_input_output.rs b/zellij-server/src/os_input_output.rs
index 3ca50ed6..3e19ca5a 100644
--- a/zellij-server/src/os_input_output.rs
+++ b/zellij-server/src/os_input_output.rs
@@ -1,5 +1,7 @@
 use std::collections::HashMap;
 
+use crate::panes::PaneId;
+
 #[cfg(target_os = "macos")]
 use darwin_libproc;
 
@@ -10,7 +12,7 @@ use std::env;
 use std::os::unix::io::RawFd;
 use std::os::unix::process::CommandExt;
 use std::path::PathBuf;
-use std::process::{Child, Command};
+use std::process::{Child, Command, Stdio};
 use std::sync::{Arc, Mutex};
 
 use zellij_utils::{async_std, interprocess, libc, nix, signal_hook, zellij_tile};
@@ -18,7 +20,8 @@ use zellij_utils::{async_std, interprocess, libc, nix, signal_hook, zellij_tile}
 use async_std::fs::File as AsyncFile;
 use async_std::os::unix::io::FromRawFd;
 use interprocess::local_socket::LocalSocketStream;
-use nix::pty::{forkpty, ForkptyResult, Winsize};
+use nix::fcntl::{fcntl, FcntlArg, FdFlag};
+use nix::pty::{forkpty, openpty, ForkptyResult, OpenptyResult, Winsize};
 use nix::sys::signal::{kill, Signal};
 use nix::sys::termios;
 use nix::sys::wait::waitpid;
@@ -99,96 +102,63 @@ fn handle_command_exit(mut child: Child) {
     }
 }
 
-fn handle_fork_pty(
-    fork_pty_res: ForkptyResult,
+fn handle_openpty(
+    open_pty_res: OpenptyResult,
     cmd: RunCommand,
-    parent_fd: RawFd,
-    child_fd: RawFd,
-) -> (RawFd, ChildId) {
-    let pid_primary = fork_pty_res.master;
-    let (pid_secondary, pid_shell) = match fork_pty_res.fork_result {
-        ForkResult::Parent { child } => {
-            let pid_shell = read_from_pipe(parent_fd, child_fd);
-            (child, pid_shell)
+    quit_cb: Box<dyn Fn(PaneId) + Send>,
+) -> (RawFd, RawFd) {
+    // primary side of pty and child fd
+    let pid_primary = open_pty_res.master;
+    let pid_secondary = open_pty_res.slave;
+
+    let mut child = unsafe {
+        let command = &mut Command::new(cmd.command);
+        if let Some(current_dir) = cmd.cwd {
+            command.current_dir(current_dir);
         }
-        ForkResult::Child => {
-            let child = unsafe {
-                let command = &mut Command::new(cmd.command);
-                if let Some(current_dir) = cmd.cwd {
-                    command.current_dir(current_dir);
+        command
+            .args(&cmd.args)
+            .pre_exec(move || -> std::io::Result<()> {
+                if libc::login_tty(pid_secondary) != 0 {
+                    panic!("failed to set controlling terminal");
                 }
-                command
-                    .args(&cmd.args)
-                    .pre_exec(|| -> std::io::Result<()> {
-                        // this is the "unsafe" part, for more details please see:
-                        // https://doc.rust-lang.org/std/os/unix/process/trait.CommandExt.html#notes-and-safety
-                        unistd::setpgid(Pid::from_raw(0), Pid::from_raw(0))
-                            .expect("failed to create a new process group");
-                        Ok(())
-                    })
-                    .spawn()
-                    .expect("failed to spawn")
-            };
-            unistd::tcsetpgrp(0, Pid::from_raw(child.id() as i32))
-                .expect("faled to set child's forceground process group");
-            write_to_pipe(child.id(), parent_fd, child_fd);
-            handle_command_exit(child);
-            ::std::process::exit(0);
-        }
+                close_fds::close_open_fds(3, &[]);
+                Ok(())
+            })
+            .spawn()
+            .expect("failed to spawn")
     };
 
-    (
-        pid_primary,
-        ChildId {
-            primary: pid_secondary,
-            shell: pid_shell.map(|pid| Pid::from_raw(pid as i32)),
-        },
-    )
+    let child_id = child.id();
+    std::thread::spawn(move || {
+        child.wait().unwrap();
+        handle_command_exit(child);
+        let _ = nix::unistd::close(pid_primary);
+        let _ = nix::unistd::close(pid_secondary);
+        quit_cb(PaneId::Terminal(pid_primary));
+    });
+
+    (pid_primary, child_id as RawFd)
 }
 
 /// Spawns a new terminal from the parent terminal with [`termios`](termios::Termios)
 /// `orig_termios`.
 ///
-fn handle_terminal(cmd: RunCommand, orig_termios: termios::Termios) -> (RawFd, ChildId) {
+fn handle_terminal(
+    cmd: RunCommand,
+    orig_termios: termios::Termios,
+    quit_cb: Box<dyn Fn(PaneId) + Send>,
+) -> (RawFd, RawFd) {
     // Create a pipe to allow the child the communicate the shell's pid to it's
     // parent.
-    let (parent_fd, child_fd) = unistd::pipe().expect("failed to create pipe");
-    match forkpty(None, Some(&orig_termios)) {
-        Ok(fork_pty_res) => handle_fork_pty(fork_pty_res, cmd, parent_fd, child_fd),
+    match openpty(None, Some(&orig_termios)) {
+        Ok(open_pty_res) => handle_openpty(open_pty_res, cmd, quit_cb),
         Err(e) => {
-            panic!("failed to fork {:?}", e);
+            panic!("failed to start pty{:?}", e);
         }
     }
 }
 
-/// Write to a pipe given both file descriptors
-fn write_to_pipe(data: u32, parent_fd: RawFd, child_fd: RawFd) {
-    let mut buff = [0; 4];
-    BigEndian::write_u32(&mut buff, data);
-    if unistd::close(parent_fd).is_err() {
-        return;
-    }
-    if unistd::write(child_fd, &buff).is_err() {
-        return;
-    }
-    unistd::close(child_fd).unwrap_or_default();
-}
-
-/// Read from a pipe given both file descriptors
-fn read_from_pipe(parent_fd: RawFd, child_fd: RawFd) -> Option<u32> {
-    let mut buffer = [0; 4];
-    if unistd::close(child_fd).is_err() {
-        return None;
-    }
-    if unistd::read(parent_fd, &mut buffer).is_err() {
-        return None;
-    }
-    if unistd::close(parent_fd).is_err() {
-        return None;
-    }
-    Some(u32::from_be_bytes(buffer))
-}
-
 /// If a [`TerminalAction::OpenFile(file)`] is given, the text editor specified by environment variable `EDITOR`
 /// (or `VISUAL`, if `EDITOR` is not set) will be started in the new terminal, with the given
 /// file open.
@@ -204,7 +174,8 @@ fn read_from_pipe(parent_fd: RawFd, child_fd: RawFd) -> Option<u32> {
 pub fn spawn_terminal(
     terminal_action: TerminalAction,
     orig_termios: termios::Termios,
-) -> (RawFd, ChildId) {
+    quit_cb: Box<dyn Fn(PaneId) + Send>,
+) -> (RawFd, RawFd) {
     let cmd = match terminal_action {
         TerminalAction::OpenFile(file_to_open) => {
             if env::var("EDITOR").is_err() && env::var("VISUAL").is_err() {
@@ -226,7 +197,7 @@ pub fn spawn_terminal(
         TerminalAction::RunCommand(command) => command,
     };
 
-    handle_terminal(cmd, orig_termios)
+    handle_terminal(cmd, orig_termios, quit_cb)
 }
 
 #[derive(Clone)]
@@ -271,7 +242,11 @@ pub trait ServerOsApi: Send + Sync {
     /// Spawn a new terminal, with a terminal action. The returned tuple contains the master file
     /// descriptor of the forked psuedo terminal and a [ChildId] struct containing process id's for
     /// the forked child process.
-    fn spawn_terminal(&self, terminal_action: TerminalAction) -> (RawFd, ChildId);
+    fn spawn_terminal(
+        &self,
+        terminal_action: TerminalAction,
+        quit_cb: Box<dyn Fn(PaneId) + Send>,
+    ) -> (RawFd, RawFd);
     /// Read bytes from the standard output of the virtual terminal referred to by `fd`.
     fn read_from_tty_stdout(&self, fd: RawFd, buf: &mut [u8]) -> Result<usize, nix::Error>;
     /// Creates an `AsyncReader` that can be used to read from `fd` in an async context
@@ -304,9 +279,13 @@ impl ServerOsApi for ServerOsInputOutput {
             set_terminal_size_using_fd(fd, cols, rows);
         }
     }
-    fn spawn_terminal(&self, terminal_action: TerminalAction) -> (RawFd, ChildId) {
+    fn spawn_terminal(
+        &self,
+        terminal_action: TerminalAction,
+        quit_cb: Box<dyn Fn(PaneId) + Send>,
+    ) -> (RawFd, RawFd) {
         let orig_termios = self.orig_termios.lock().unwrap();
-        spawn_terminal(terminal_action, orig_termios.clone())
+        spawn_terminal(terminal_action, orig_termios.clone(), quit_cb)
     }
     fn read_from_tty_stdout(&self, fd: RawFd, buf: &mut [u8]) -> Result<usize, nix::Error> {
         unistd::read(fd, buf)
@@ -324,8 +303,7 @@ impl ServerOsApi for ServerOsInputOutput {
         Box::new((*self).clone())
     }
     fn kill(&self, pid: Pid) -> Result<(), nix::Error> {
-        kill(pid, Some(Signal::SIGTERM)).unwrap();
-        waitpid(pid, None).unwrap();
+        let _ = kill(pid, Some(Signal::SIGTERM));
         Ok(())
     }
     fn force_kill(&self, pid: Pid) -> Result<(), nix::Error> {
diff --git a/zellij-server/src/pty.rs b/zellij-server/src/pty.rs
index 277f572c..93699a32 100644
--- a/zellij-server/src/pty.rs
+++ b/zellij-server/src/pty.rs
@@ -1,5 +1,5 @@
 use crate::{
-    os_input_output::{AsyncReader, ChildId, ServerOsApi},
+    os_input_output::{AsyncReader, ServerOsApi},
     panes::PaneId,
     screen::ScreenInstruction,
     thread_bus::{Bus, ThreadSenders},
@@ -17,6 +17,7 @@ use std::{
     path::PathBuf,
     time::{Duration, Instant},
 };
+use zellij_utils::nix::unistd::Pid;
 use zellij_utils::{
     async_std,
     errors::{get_current_ctx, ContextType, PtyContext},
@@ -66,7 +67,7 @@ impl From<&PtyInstruction> for PtyContext {
 pub(crate) struct Pty {
     pub active_panes: HashMap<ClientId, PaneId>,
     pub bus: Bus<PtyInstruction>,
-    pub id_to_child_pid: HashMap<RawFd, ChildId>,
+    pub id_to_child_pid: HashMap<RawFd, RawFd>, // pty_primary => child raw fd
     debug_to_file: bool,
     task_handles: HashMap<RawFd, JoinHandle<()>>,
 }
@@ -252,15 +253,6 @@ fn stream_terminal_bytes(
                 }
             }
             async_send_to_screen(senders.clone(), ScreenInstruction::Render).await;
-
-            // we send ClosePane here so that the screen knows to close this tab if the process
-            // inside the terminal exited on its own (eg. the user typed "exit<ENTER>" inside a
-            // bash shell)
-            async_send_to_screen(
-                senders,
-                ScreenInstruction::ClosePane(PaneId::Terminal(pid), None),
-            )
-            .await;
         }
     })
 }
@@ -283,9 +275,14 @@ impl Pty {
                 .and_then(|client_id| self.active_panes.get(&client_id))
                 .and_then(|pane| match pane {
                     PaneId::Plugin(..) => None,
-                    PaneId::Terminal(id) => self.id_to_child_pid.get(id).and_then(|id| id.shell),
+                    PaneId::Terminal(id) => self.id_to_child_pid.get(id),
+                })
+                .and_then(|id| {
+                    self.bus
+                        .os_input
+                        .as_ref()
+                        .map(|input| input.get_cwd(Pid::from_raw(*id)))
                 })
-                .and_then(|id| self.bus.os_input.as_ref().map(|input| input.get_cwd(id)))
                 .flatten(),
         })
     }
@@ -302,12 +299,18 @@ impl Pty {
                 terminal_action.unwrap_or_else(|| self.get_default_terminal(None))
             }
         };
-        let (pid_primary, child_id): (RawFd, ChildId) = self
+        let quit_cb = Box::new({
+            let senders = self.bus.senders.clone();
+            move |pane_id| {
+                let _ = senders.send_to_screen(ScreenInstruction::ClosePane(pane_id, None));
+            }
+        });
+        let (pid_primary, child_fd): (RawFd, RawFd) = self
             .bus
             .os_input
             .as_mut()
             .unwrap()
-            .spawn_terminal(terminal_action);
+            .spawn_terminal(terminal_action, quit_cb);
         let task_handle = stream_terminal_bytes(
             pid_primary,
             self.bus.senders.clone(),
@@ -315,7 +318,7 @@ impl Pty {
             self.debug_to_file,
         );
         self.task_handles.insert(pid_primary, task_handle);
-        self.id_to_child_pid.insert(pid_primary, child_id);
+        self.id_to_child_pid.insert(pid_primary, child_fd);
         pid_primary
     }
     pub fn spawn_terminals_for_layout(
@@ -329,22 +332,32 @@ impl Pty {
         let extracted_run_instructions = layout.extract_run_instructions();
         let mut new_pane_pids = vec![];
         for run_instruction in extracted_run_instructions {
+            let quit_cb = Box::new({
+                let senders = self.bus.senders.clone();
+                move |pane_id| {
+                    let _ = senders.send_to_screen(ScreenInstruction::ClosePane(pane_id, None));
+                }
+            });
             match run_instruction {
                 Some(Run::Command(command)) => {
                     let cmd = TerminalAction::RunCommand(command);
-                    let (pid_primary, child_id): (RawFd, ChildId) =
-                        self.bus.os_input.as_mut().unwrap().spawn_terminal(cmd);
-                    self.id_to_child_pid.insert(pid_primary, child_id);
+                    let (pid_primary, child_fd): (RawFd, RawFd) = self
+                        .bus
+                        .os_input
+                        .as_mut()
+                        .unwrap()
+                        .spawn_terminal(cmd, quit_cb);
+                    self.id_to_child_pid.insert(pid_primary, child_fd);
                     new_pane_pids.push(pid_primary);
                 }
                 None => {
-                    let (pid_primary, child_id): (RawFd, ChildId) = self
+                    let (pid_primary, child_fd): (RawFd, RawFd) = self
                         .bus
                         .os_input
                         .as_mut()
                         .unwrap()
-                        .spawn_terminal(default_shell.clone());
-                    self.id_to_child_pid.insert(pid_primary, child_id);
+                        .spawn_terminal(default_shell.clone(), quit_cb);
+                    self.id_to_child_pid.insert(pid_primary, child_fd);
                     new_pane_pids.push(pid_primary);
                 }
                 // Investigate moving plugin loading to here.
@@ -372,27 +385,15 @@ impl Pty {
     pub fn close_pane(&mut self, id: PaneId) {
         match id {
             PaneId::Terminal(id) => {
-                let pids = self.id_to_child_pid.remove(&id).unwrap();
-                let handle = self.task_handles.remove(&id).unwrap();
+                let child_fd = self.id_to_child_pid.remove(&id).unwrap();
+                self.task_handles.remove(&id).unwrap();
                 task::block_on(async {
                     self.bus
                         .os_input
                         .as_mut()
                         .unwrap()
-                        .kill(pids.primary)
+                        .kill(Pid::from_raw(child_fd))
                         .unwrap();
-                    let timeout = Duration::from_millis(100);
-                    match async_timeout(timeout, handle.cancel()).await {
-                        Ok(_) => {}
-                        _ => {
-                            self.bus
-                                .os_input
-                                .as_mut()
-                                .unwrap()
-                                .force_kill(pids.primary)
-                                .unwrap();
-                        }
-                    };
                 });
             }
             PaneId::Plugin(pid) => drop(
diff --git a/zellij-server/src/unit/screen_tests.rs b/zellij-server/src/unit/screen_tests.rs
index b4d367e4..f9f63195 100644
--- a/zellij-server/src/unit/screen_tests.rs
+++ b/zellij-server/src/unit/screen_tests.rs
@@ -1,7 +1,8 @@
 use super::{Screen, ScreenInstruction};
+use crate::panes::PaneId;
 use crate::zellij_tile::data::{ModeInfo, Palette};
 use crate::{
-    os_input_output::{AsyncReader, ChildId, Pid, ServerOsApi},
+    os_input_output::{AsyncReader, Pid, ServerOsApi},
     thread_bus::Bus,
     ClientId,
 };
@@ -29,7 +30,11 @@ impl ServerOsApi for FakeInputOutput {
     fn set_terminal_size_using_fd(&self, _fd: RawFd, _cols: u16, _rows: u16) {
         // noop
     }
-    fn spawn_terminal(&self, _file_to_open: TerminalAction) -> (RawFd, ChildId) {
+    fn spawn_terminal(
+        &self,
+        _file_to_open: TerminalAction,
+        _quit_db: Box<dyn Fn(PaneId) + Send>,
+    ) -> (RawFd, RawFd) {
         unimplemented!()
     }
     fn read_from_tty_stdout(&self, _fd: RawFd, _buf: &mut [u8]) -> Result<usize, nix::Error> {
diff --git a/zellij-server/src/unit/tab_tests.rs b/zellij-server/src/unit/tab_tests.rs
index 41e243cd..3d177ab8 100644
--- a/zellij-server/src/unit/tab_tests.rs
+++ b/zellij-server/src/unit/tab_tests.rs
@@ -1,7 +1,7 @@
 use super::Tab;
 use crate::zellij_tile::data::{ModeInfo, Palette};
 use crate::{
-    os_input_output::{AsyncReader, ChildId, Pid, ServerOsApi},
+    os_input_output::{AsyncReader, Pid, ServerOsApi},
     panes::PaneId,
     thread_bus::ThreadSenders,
     ClientId,
@@ -29,7 +29,11 @@ impl ServerOsApi for FakeInputOutput {
     fn set_terminal_size_using_fd(&self, _fd: RawFd, _cols: u16, _rows: u16) {
         // noop
     }
-    fn spawn_terminal(&self, _file_to_open: TerminalAction) -> (RawFd, ChildId) {
+    fn spawn_terminal(
+        &self,
+        _file_to_open: TerminalAction,
+        _quit_cb: Box<dyn Fn(PaneId) + Send>,
+    ) -> (RawFd, RawFd) {
         unimplemented!()
     }
     fn read_from_tty_stdout(&self, _fd: RawFd, _buf: &mut [u8]) -> Result<usize, nix::Error> {