diff options
author | Christian Duerr <contact@christianduerr.com> | 2021-12-18 22:18:42 +0000 |
---|---|---|
committer | GitHub <noreply@github.com> | 2021-12-19 01:18:42 +0300 |
commit | 6d1a63ef28d18168ed4ca0d6a8c3413cb4621ca5 (patch) | |
tree | 8fa922ffd541f6d4da24f677f2eab4934692f511 | |
parent | f1802c1cda7b9922f23d872705653beb6b053f2e (diff) | |
download | alacritty-6d1a63ef28d18168ed4ca0d6a8c3413cb4621ca5.tar.gz alacritty-6d1a63ef28d18168ed4ca0d6a8c3413cb4621ca5.zip |
Remove shared PID/FD variables
The existing PID/FD atomics in alacritty_terminal/src/tty/unix.rs were
shared across all Alacritty windows, causing problem with the new
multiwindow feature.
Instead of sharing these between the different windows, the master FD
and shell PID are now stored on the `window_context`.
Unfortunately this makes spawning new daemons a little more complicated,
having to pass through additional parameters. To ease this a little bit
the helper method `spawn_daemon` has been defined on the
`ActionContext`, making it accessible from most parts of Alacritty's
event loop.
Fixes #5700.
-rw-r--r-- | alacritty/src/daemon.rs | 94 | ||||
-rw-r--r-- | alacritty/src/event.rs | 36 | ||||
-rw-r--r-- | alacritty/src/input.rs | 11 | ||||
-rw-r--r-- | alacritty/src/window_context.rs | 19 | ||||
-rw-r--r-- | alacritty_terminal/src/tty/unix.rs | 49 |
5 files changed, 123 insertions, 86 deletions
diff --git a/alacritty/src/daemon.rs b/alacritty/src/daemon.rs index 4b95cb6b..730a2cc7 100644 --- a/alacritty/src/daemon.rs +++ b/alacritty/src/daemon.rs @@ -1,42 +1,31 @@ -#[cfg(not(windows))] -use std::error::Error; use std::ffi::OsStr; -use std::fmt::Debug; #[cfg(not(any(target_os = "macos", windows)))] use std::fs; use std::io; -#[cfg(not(windows))] -use std::os::unix::process::CommandExt; #[cfg(windows)] use std::os::windows::process::CommandExt; -#[cfg(not(windows))] -use std::path::PathBuf; use std::process::{Command, Stdio}; -use log::{debug, warn}; -#[cfg(windows)] -use winapi::um::winbase::{CREATE_NEW_PROCESS_GROUP, CREATE_NO_WINDOW}; +#[rustfmt::skip] +#[cfg(not(windows))] +use { + std::error::Error, + std::os::unix::process::CommandExt, + std::os::unix::io::RawFd, + std::path::PathBuf, +}; #[cfg(not(windows))] -use alacritty_terminal::tty; +use libc::pid_t; +#[cfg(windows)] +use winapi::um::winbase::{CREATE_NEW_PROCESS_GROUP, CREATE_NO_WINDOW}; #[cfg(target_os = "macos")] use crate::macos; -/// Start the daemon and log error on failure. -pub fn start_daemon<I, S>(program: &str, args: I) -where - I: IntoIterator<Item = S> + Debug + Copy, - S: AsRef<OsStr>, -{ - match spawn_daemon(program, args) { - Ok(_) => debug!("Launched {} with args {:?}", program, args), - Err(_) => warn!("Unable to launch {} with args {:?}", program, args), - } -} - +/// Start a new process in the background. #[cfg(windows)] -fn spawn_daemon<I, S>(program: &str, args: I) -> io::Result<()> +pub fn spawn_daemon<I, S>(program: &str, args: I) -> io::Result<()> where I: IntoIterator<Item = S> + Copy, S: AsRef<OsStr>, @@ -55,37 +44,21 @@ where .map(|_| ()) } -/// Get working directory of controlling process. -#[cfg(not(windows))] -pub fn foreground_process_path() -> Result<PathBuf, Box<dyn Error>> { - let mut pid = unsafe { libc::tcgetpgrp(tty::master_fd()) }; - if pid < 0 { - pid = tty::child_pid(); - } - - #[cfg(not(any(target_os = "macos", target_os = "freebsd")))] - let link_path = format!("/proc/{}/cwd", pid); - #[cfg(target_os = "freebsd")] - let link_path = format!("/compat/linux/proc/{}/cwd", pid); - - #[cfg(not(target_os = "macos"))] - let cwd = fs::read_link(link_path)?; - - #[cfg(target_os = "macos")] - let cwd = macos::proc::cwd(pid)?; - - Ok(cwd) -} - +/// Start a new process in the background. #[cfg(not(windows))] -fn spawn_daemon<I, S>(program: &str, args: I) -> io::Result<()> +pub fn spawn_daemon<I, S>( + program: &str, + args: I, + master_fd: RawFd, + shell_pid: u32, +) -> io::Result<()> where I: IntoIterator<Item = S> + Copy, S: AsRef<OsStr>, { let mut command = Command::new(program); command.args(args).stdin(Stdio::null()).stdout(Stdio::null()).stderr(Stdio::null()); - if let Ok(cwd) = foreground_process_path() { + if let Ok(cwd) = foreground_process_path(master_fd, shell_pid) { command.current_dir(cwd); } unsafe { @@ -108,3 +81,28 @@ where .map(|_| ()) } } + +/// Get working directory of controlling process. +#[cfg(not(windows))] +pub fn foreground_process_path( + master_fd: RawFd, + shell_pid: u32, +) -> Result<PathBuf, Box<dyn Error>> { + let mut pid = unsafe { libc::tcgetpgrp(master_fd) }; + if pid < 0 { + pid = shell_pid as pid_t; + } + + #[cfg(not(any(target_os = "macos", target_os = "freebsd")))] + let link_path = format!("/proc/{}/cwd", pid); + #[cfg(target_os = "freebsd")] + let link_path = format!("/compat/linux/proc/{}/cwd", pid); + + #[cfg(not(target_os = "macos"))] + let cwd = fs::read_link(link_path)?; + + #[cfg(target_os = "macos")] + let cwd = macos::proc::cwd(pid)?; + + Ok(cwd) +} diff --git a/alacritty/src/event.rs b/alacritty/src/event.rs index 7fb54b39..8ce3b2e0 100644 --- a/alacritty/src/event.rs +++ b/alacritty/src/event.rs @@ -4,7 +4,10 @@ use std::borrow::Cow; use std::cmp::{max, min}; use std::collections::{HashMap, VecDeque}; use std::error::Error; +use std::ffi::OsStr; use std::fmt::Debug; +#[cfg(not(windows))] +use std::os::unix::io::RawFd; use std::path::PathBuf; use std::time::{Duration, Instant}; use std::{env, f32, mem}; @@ -16,7 +19,7 @@ use glutin::platform::run_return::EventLoopExtRunReturn; #[cfg(all(feature = "wayland", not(any(target_os = "macos", windows))))] use glutin::platform::unix::EventLoopWindowTargetExtUnix; use glutin::window::WindowId; -use log::{error, info}; +use log::{debug, error, info, warn}; #[cfg(all(feature = "wayland", not(any(target_os = "macos", windows))))] use wayland_client::{Display as WaylandDisplay, EventQueue}; @@ -37,7 +40,7 @@ use crate::config::ui_config::{HintAction, HintInternalAction}; use crate::config::{self, UiConfig}; #[cfg(not(windows))] use crate::daemon::foreground_process_path; -use crate::daemon::start_daemon; +use crate::daemon::spawn_daemon; use crate::display::hint::HintMatch; use crate::display::window::Window; use crate::display::{self, Display}; @@ -183,6 +186,10 @@ pub struct ActionContext<'a, N, T> { pub search_state: &'a mut SearchState, pub font_size: &'a mut Size, pub dirty: &'a mut bool, + #[cfg(not(windows))] + pub master_fd: RawFd, + #[cfg(not(windows))] + pub shell_pid: u32, } impl<'a, N: Notify + 'a, T: EventListener> input::ActionContext<T> for ActionContext<'a, N, T> { @@ -367,12 +374,13 @@ impl<'a, N: Notify + 'a, T: EventListener> input::ActionContext<T> for ActionCon args.push(arg); } - start_daemon(&alacritty, &args); + self.spawn_daemon(&alacritty, &args); } #[cfg(not(windows))] fn create_new_window(&mut self) { - let options = if let Ok(working_directory) = foreground_process_path() { + let cwd = foreground_process_path(self.master_fd, self.shell_pid); + let options = if let Ok(working_directory) = cwd { let mut options = TerminalCliOptions::new(); options.working_directory = Some(working_directory); Some(options) @@ -388,6 +396,22 @@ impl<'a, N: Notify + 'a, T: EventListener> input::ActionContext<T> for ActionCon let _ = self.event_proxy.send_event(Event::new(EventType::CreateWindow(None), None)); } + fn spawn_daemon<I, S>(&self, program: &str, args: I) + where + I: IntoIterator<Item = S> + Debug + Copy, + S: AsRef<OsStr>, + { + #[cfg(not(windows))] + let result = spawn_daemon(program, args, self.master_fd, self.shell_pid); + #[cfg(windows)] + let result = spawn_daemon(program, args); + + match result { + Ok(_) => debug!("Launched {} with args {:?}", program, args), + Err(_) => warn!("Unable to launch {} with args {:?}", program, args), + } + } + fn change_font_size(&mut self, delta: f32) { *self.font_size = max(*self.font_size + delta, Size::new(FONT_SIZE_STEP)); let font = self.config.font.clone().with_size(*self.font_size); @@ -655,7 +679,7 @@ impl<'a, N: Notify + 'a, T: EventListener> input::ActionContext<T> for ActionCon let text = self.terminal.bounds_to_string(*hint.bounds.start(), *hint.bounds.end()); let mut args = command.args().to_vec(); args.push(text); - start_daemon(command.program(), &args); + self.spawn_daemon(command.program(), &args); }, // Copy the text to the clipboard. HintAction::Action(HintInternalAction::Copy) => { @@ -1039,7 +1063,7 @@ impl input::Processor<EventProxy, ActionContext<'_, Notifier, EventProxy>> { // Execute bell command. if let Some(bell_command) = &self.ctx.config.bell.command { - start_daemon(bell_command.program(), bell_command.args()); + self.ctx.spawn_daemon(bell_command.program(), bell_command.args()); } }, TerminalEvent::ClipboardStore(clipboard_type, content) => { diff --git a/alacritty/src/input.rs b/alacritty/src/input.rs index 07b3154c..40b18ca2 100644 --- a/alacritty/src/input.rs +++ b/alacritty/src/input.rs @@ -7,6 +7,8 @@ use std::borrow::Cow; use std::cmp::{max, min, Ordering}; +use std::ffi::OsStr; +use std::fmt::Debug; use std::marker::PhantomData; use std::time::{Duration, Instant}; @@ -30,7 +32,6 @@ use alacritty_terminal::vi_mode::ViMotion; use crate::clipboard::Clipboard; use crate::config::{Action, BindingMode, Key, MouseAction, SearchAction, UiConfig, ViAction}; -use crate::daemon::start_daemon; use crate::display::hint::HintMatch; use crate::display::window::Window; use crate::display::Display; @@ -107,6 +108,12 @@ pub trait ActionContext<T: EventListener> { fn trigger_hint(&mut self, _hint: &HintMatch) {} fn expand_selection(&mut self) {} fn paste(&mut self, _text: &str) {} + fn spawn_daemon<I, S>(&self, _program: &str, _args: I) + where + I: IntoIterator<Item = S> + Debug + Copy, + S: AsRef<OsStr>, + { + } } impl Action { @@ -139,7 +146,7 @@ impl<T: EventListener> Execute<T> for Action { ctx.scroll(Scroll::Bottom); ctx.write_to_pty(s.clone().into_bytes()) }, - Action::Command(program) => start_daemon(program.program(), program.args()), + Action::Command(program) => ctx.spawn_daemon(program.program(), program.args()), Action::Hint(hint) => { ctx.display().hint_state.start(hint.clone()); ctx.mark_dirty(); diff --git a/alacritty/src/window_context.rs b/alacritty/src/window_context.rs index f0b111b5..d7a6b41e 100644 --- a/alacritty/src/window_context.rs +++ b/alacritty/src/window_context.rs @@ -5,6 +5,8 @@ use std::error::Error; use std::fs::File; use std::io::Write; use std::mem; +#[cfg(not(windows))] +use std::os::unix::io::{AsRawFd, RawFd}; #[cfg(not(any(target_os = "macos", windows)))] use std::sync::atomic::Ordering; use std::sync::Arc; @@ -49,6 +51,10 @@ pub struct WindowContext { font_size: Size, mouse: Mouse, dirty: bool, + #[cfg(not(windows))] + master_fd: RawFd, + #[cfg(not(windows))] + shell_pid: u32, } impl WindowContext { @@ -97,6 +103,11 @@ impl WindowContext { .unwrap_or(Cow::Borrowed(&config.terminal_config.pty_config)); let pty = tty::new(&pty_config, &display.size_info, display.window.x11_window_id())?; + #[cfg(not(windows))] + let master_fd = pty.file().as_raw_fd(); + #[cfg(not(windows))] + let shell_pid = pty.child().id(); + // Create the pseudoterminal I/O loop. // // PTY I/O is ran on another thread as to not occupy cycles used by the @@ -129,6 +140,10 @@ impl WindowContext { notifier: Notifier(loop_tx), terminal, display, + #[cfg(not(windows))] + master_fd, + #[cfg(not(windows))] + shell_pid, suppress_chars: Default::default(), message_buffer: Default::default(), received_count: Default::default(), @@ -246,6 +261,10 @@ impl WindowContext { mouse: &mut self.mouse, dirty: &mut self.dirty, terminal: &mut terminal, + #[cfg(not(windows))] + master_fd: self.master_fd, + #[cfg(not(windows))] + shell_pid: self.shell_pid, event_proxy, event_loop, clipboard, diff --git a/alacritty_terminal/src/tty/unix.rs b/alacritty_terminal/src/tty/unix.rs index 0d123b8c..206dbe8d 100644 --- a/alacritty_terminal/src/tty/unix.rs +++ b/alacritty_terminal/src/tty/unix.rs @@ -11,9 +11,8 @@ use std::os::unix::io::{AsRawFd, FromRawFd, RawFd}; use std::os::unix::process::CommandExt; use std::process::{Child, Command, Stdio}; use std::ptr; -use std::sync::atomic::{AtomicI32, AtomicUsize, Ordering}; -use libc::{self, c_int, pid_t, winsize, TIOCSCTTY}; +use libc::{self, c_int, winsize, TIOCSCTTY}; use log::error; use mio::unix::EventedFd; use nix::pty::openpty; @@ -28,14 +27,6 @@ use crate::grid::Dimensions; use crate::term::SizeInfo; use crate::tty::{ChildEvent, EventedPty, EventedReadWrite}; -/// Process ID of child process. -/// -/// Necessary to put this in static storage for `SIGCHLD` to have access. -static PID: AtomicUsize = AtomicUsize::new(0); - -/// File descriptor of terminal master. -static FD: AtomicI32 = AtomicI32::new(-1); - macro_rules! die { ($($arg:tt)*) => {{ error!($($arg)*); @@ -43,14 +34,6 @@ macro_rules! die { }} } -pub fn child_pid() -> pid_t { - PID.load(Ordering::Relaxed) as pid_t -} - -pub fn master_fd() -> RawFd { - FD.load(Ordering::Relaxed) as RawFd -} - /// Get raw fds for master/slave ends of a new PTY. fn make_pty(size: winsize) -> (RawFd, RawFd) { let mut win_size = size; @@ -124,12 +107,22 @@ fn get_pw_entry(buf: &mut [i8; 1024]) -> Passwd<'_> { pub struct Pty { child: Child, - fd: File, + file: File, token: mio::Token, signals: Signals, signals_token: mio::Token, } +impl Pty { + pub fn child(&self) -> &Child { + &self.child + } + + pub fn file(&self) -> &File { + &self.file + } +} + #[cfg(target_os = "macos")] fn default_shell(pw: &Passwd<'_>) -> Program { let shell_name = pw.shell.rsplit('/').next().unwrap(); @@ -223,10 +216,6 @@ pub fn new(config: &PtyConfig, size: &SizeInfo, window_id: Option<usize>) -> Res match builder.spawn() { Ok(child) => { - // Remember master FD and child PID so other modules can use it. - PID.store(child.id() as usize, Ordering::Relaxed); - FD.store(master, Ordering::Relaxed); - unsafe { // Maybe this should be done outside of this function so nonblocking // isn't forced upon consumers. Although maybe it should be? @@ -235,7 +224,7 @@ pub fn new(config: &PtyConfig, size: &SizeInfo, window_id: Option<usize>) -> Res let mut pty = Pty { child, - fd: unsafe { File::from_raw_fd(master) }, + file: unsafe { File::from_raw_fd(master) }, token: mio::Token::from(0), signals, signals_token: mio::Token::from(0), @@ -273,7 +262,7 @@ impl EventedReadWrite for Pty { poll_opts: mio::PollOpt, ) -> Result<()> { self.token = token.next().unwrap(); - poll.register(&EventedFd(&self.fd.as_raw_fd()), self.token, interest, poll_opts)?; + poll.register(&EventedFd(&self.file.as_raw_fd()), self.token, interest, poll_opts)?; self.signals_token = token.next().unwrap(); poll.register( @@ -291,7 +280,7 @@ impl EventedReadWrite for Pty { interest: mio::Ready, poll_opts: mio::PollOpt, ) -> Result<()> { - poll.reregister(&EventedFd(&self.fd.as_raw_fd()), self.token, interest, poll_opts)?; + poll.reregister(&EventedFd(&self.file.as_raw_fd()), self.token, interest, poll_opts)?; poll.reregister( &self.signals, @@ -303,13 +292,13 @@ impl EventedReadWrite for Pty { #[inline] fn deregister(&mut self, poll: &mio::Poll) -> Result<()> { - poll.deregister(&EventedFd(&self.fd.as_raw_fd()))?; + poll.deregister(&EventedFd(&self.file.as_raw_fd()))?; poll.deregister(&self.signals) } #[inline] fn reader(&mut self) -> &mut File { - &mut self.fd + &mut self.file } #[inline] @@ -319,7 +308,7 @@ impl EventedReadWrite for Pty { #[inline] fn writer(&mut self) -> &mut File { - &mut self.fd + &mut self.file } #[inline] @@ -361,7 +350,7 @@ impl OnResize for Pty { fn on_resize(&mut self, size: &SizeInfo) { let win = size.to_winsize(); - let res = unsafe { libc::ioctl(self.fd.as_raw_fd(), libc::TIOCSWINSZ, &win as *const _) }; + let res = unsafe { libc::ioctl(self.file.as_raw_fd(), libc::TIOCSWINSZ, &win as *const _) }; if res < 0 { die!("ioctl TIOCSWINSZ failed: {}", Error::last_os_error()); |