From 6d1a63ef28d18168ed4ca0d6a8c3413cb4621ca5 Mon Sep 17 00:00:00 2001 From: Christian Duerr Date: Sat, 18 Dec 2021 22:18:42 +0000 Subject: 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. --- alacritty/src/daemon.rs | 94 ++++++++++++++++++++--------------------- alacritty/src/event.rs | 36 +++++++++++++--- alacritty/src/input.rs | 11 ++++- alacritty/src/window_context.rs | 19 +++++++++ 4 files changed, 104 insertions(+), 56 deletions(-) (limited to 'alacritty/src') 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(program: &str, args: I) -where - I: IntoIterator + Debug + Copy, - S: AsRef, -{ - 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(program: &str, args: I) -> io::Result<()> +pub fn spawn_daemon(program: &str, args: I) -> io::Result<()> where I: IntoIterator + Copy, S: AsRef, @@ -55,37 +44,21 @@ where .map(|_| ()) } -/// Get working directory of controlling process. -#[cfg(not(windows))] -pub fn foreground_process_path() -> Result> { - 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(program: &str, args: I) -> io::Result<()> +pub fn spawn_daemon( + program: &str, + args: I, + master_fd: RawFd, + shell_pid: u32, +) -> io::Result<()> where I: IntoIterator + Copy, S: AsRef, { 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> { + 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 for ActionContext<'a, N, T> { @@ -367,12 +374,13 @@ impl<'a, N: Notify + 'a, T: EventListener> input::ActionContext 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 for ActionCon let _ = self.event_proxy.send_event(Event::new(EventType::CreateWindow(None), None)); } + fn spawn_daemon(&self, program: &str, args: I) + where + I: IntoIterator + Debug + Copy, + S: AsRef, + { + #[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 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> { // 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 { fn trigger_hint(&mut self, _hint: &HintMatch) {} fn expand_selection(&mut self) {} fn paste(&mut self, _text: &str) {} + fn spawn_daemon(&self, _program: &str, _args: I) + where + I: IntoIterator + Debug + Copy, + S: AsRef, + { + } } impl Action { @@ -139,7 +146,7 @@ impl Execute 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, -- cgit