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.
This commit is contained in:
Christian Duerr 2021-12-18 22:18:42 +00:00 committed by GitHub
parent f1802c1cda
commit 6d1a63ef28
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
5 changed files with 123 additions and 86 deletions

View File

@ -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.
/// Start a new process in the background.
#[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)
}
#[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)
}

View File

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

View File

@ -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();

View File

@ -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,

View File

@ -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());