diff --git a/docs/changelog.md b/docs/changelog.md index 5344ff418..bb9f3eddf 100644 --- a/docs/changelog.md +++ b/docs/changelog.md @@ -33,6 +33,8 @@ As features stabilize some brief notes about them will accumulate here. a non UTF-8, non-ASCII name. #3390 * mux: `--config` related command line options passed to `wezterm-mux-server` were not propagated when using `--daemonize`. #3397 #2686 +* mux: server would lock and then unlock the pid/lock file when it re-executed, + rendering it useless. ### 20230326-111934-3666303c diff --git a/wezterm-mux-server/src/daemonize.rs b/wezterm-mux-server/src/daemonize.rs index e2602baf2..c661750b5 100644 --- a/wezterm-mux-server/src/daemonize.rs +++ b/wezterm-mux-server/src/daemonize.rs @@ -2,7 +2,7 @@ use anyhow::Context; use libc::pid_t; use std::io::Write; -use std::os::unix::io::AsRawFd; +use std::os::unix::io::{AsRawFd, IntoRawFd, RawFd}; enum Fork { Child(pid_t), @@ -63,7 +63,7 @@ fn lock_pid_file(config: &config::ConfigHandle) -> anyhow::Result Ok(file) } -pub fn daemonize(config: &config::ConfigHandle) -> anyhow::Result<()> { +pub fn daemonize(config: &config::ConfigHandle) -> anyhow::Result> { let pid_file = if !config::running_under_wsl() { // pid file locking is only partly functional when running under // WSL 1; it is possible for the pid file to exist after a reboot @@ -96,16 +96,40 @@ pub fn daemonize(config: &config::ConfigHandle) -> anyhow::Result<()> { Fork::Child(_) => {} } - if let Some(mut pid_file) = pid_file { + let pid_file_fd = pid_file.map(|mut pid_file| { writeln!(pid_file, "{}", unsafe { libc::getpid() }).ok(); // Leak it so that the descriptor remains open for the duration // of the process runtime - std::mem::forget(pid_file); - } + let fd = pid_file.into_raw_fd(); + + // Since we will always re-exec, we need to clear FD_CLOEXEC + // in order for the pidfile to be inherited in our newly + // exec'd self + set_cloexec(fd, false); + + fd + }); unsafe { libc::dup2(devnull.as_raw_fd(), libc::STDIN_FILENO) }; unsafe { libc::dup2(stdout.as_raw_fd(), libc::STDOUT_FILENO) }; unsafe { libc::dup2(stderr.as_raw_fd(), libc::STDERR_FILENO) }; - Ok(()) + Ok(pid_file_fd) +} + +pub fn set_cloexec(fd: RawFd, enable: bool) { + unsafe { + let flags = libc::fcntl(fd, libc::F_GETFD); + if flags == -1 { + return; + } + + let flags = if enable { + flags | libc::FD_CLOEXEC + } else { + flags & !libc::FD_CLOEXEC + }; + + libc::fcntl(fd, libc::F_SETFD, flags); + } } diff --git a/wezterm-mux-server/src/main.rs b/wezterm-mux-server/src/main.rs index 433f16308..b54667770 100644 --- a/wezterm-mux-server/src/main.rs +++ b/wezterm-mux-server/src/main.rs @@ -51,6 +51,10 @@ struct Opt { #[arg(long = "cwd", value_parser, value_hint=ValueHint::DirPath)] cwd: Option, + #[cfg(unix)] + #[arg(long, hide = true)] + pid_file_fd: Option, + /// Instead of executing your shell, run PROG. /// For example: `wezterm start -- bash -l` will spawn bash /// as if it were a login shell. @@ -75,6 +79,16 @@ fn run() -> anyhow::Result<()> { let _saver = umask::UmaskSaver::new(); let opts = Opt::parse(); + + #[cfg(unix)] + { + // Ensure that we set CLOEXEC on the inherited lock file + // before we have an opportunity to spawn any child processes. + if let Some(fd) = opts.pid_file_fd { + daemonize::set_cloexec(fd, true); + } + } + config::common_init( opts.config_file.as_ref(), &opts.config_override, @@ -85,10 +99,13 @@ fn run() -> anyhow::Result<()> { config.update_ulimit()?; + #[cfg(unix)] + let mut pid_file = None; + #[cfg(unix)] { if opts.daemonize { - daemonize::daemonize(&config)?; + pid_file = daemonize::daemonize(&config)?; // When we reach this line, we are in a forked child process, // and the fork will have broken the async-io/reactor state // of the smol runtime. @@ -103,6 +120,17 @@ fn run() -> anyhow::Result<()> { // On Unix, forking breaks the global state maintained by `smol`, // so we need to re-exec ourselves to start things back up properly. let mut cmd = Command::new(std::env::current_exe().unwrap()); + + #[cfg(unix)] + { + // Inform the new version of ourselves that we already + // locked the pidfile so that it can prevent it from + // being propagated to its children when they spawn + if let Some(fd) = pid_file { + cmd.arg("--pid-file-fd"); + cmd.arg(&fd.to_string()); + } + } if opts.skip_config { cmd.arg("-n"); }