From a9db5664be1625a177e2dddc9f7d0e1b0a7fbf4c Mon Sep 17 00:00:00 2001 From: Teddy Tennant Date: Wed, 12 Aug 2026 06:26:33 -0400 Subject: [PATCH] fix(clipboard): don't kill copy_command processes that own the selection (#5458) * fix(clipboard): don't kill copy_command processes that own the selection The reaping added in #4298 kills the copy_command process if it is still running one second after the copied text was written to its stdin. X11 and Wayland clipboard helpers are supposed to keep running: they own the selection and serve it to whoever pastes it, so killing them empties the clipboard. Wait for the process instead of polling it with a deadline. Commands that exit on their own are still reaped as soon as they do, so the zombies #4298 was about don't come back. * docs(changelog): add PR --- CHANGELOG.md | 1 + zellij-server/src/tab/copy_command.rs | 120 ++++++++++++++++++++------ 2 files changed, 95 insertions(+), 26 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index da4f7b9bf..4f8b582ff 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -28,6 +28,7 @@ The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.0.0/) * fix: clear existing pane name when entering rename input (https://github.com/zellij-org/zellij/pull/5369) * fix: occasional input spam when pressing ESC and holding the mouse over slow SSH connections (https://github.com/zellij-org/zellij/pull/5323) * chore: switch cassowary deps to maintained fork kasuari (https://github.com/zellij-org/zellij/pull/5416) +* fix: don't kill the `copy_command` process while it owns the system clipboard (https://github.com/zellij-org/zellij/pull/5458) * chore: switch unmaintained daemonize with maintained fork daemonix (https://github.com/zellij-org/zellij/pull/4512) * feat: handle nested Zellij sessions (https://github.com/zellij-org/zellij/pull/5417) * chore: remove outdated manpage and some build fixes (https://github.com/zellij-org/zellij/pull/5426) diff --git a/zellij-server/src/tab/copy_command.rs b/zellij-server/src/tab/copy_command.rs index eead0bd75..911ef0fcc 100644 --- a/zellij-server/src/tab/copy_command.rs +++ b/zellij-server/src/tab/copy_command.rs @@ -1,5 +1,6 @@ use std::io::prelude::*; -use std::process::{Command, Stdio}; +use std::process::{Child, Command, Stdio}; +use std::thread::JoinHandle; use anyhow::{Context, Result}; @@ -30,32 +31,99 @@ impl CopyCommand { .write_all(value.as_bytes()) .with_context(|| format!("couldn't write to {} stdin", self.command))?; - // reap process with a 1 second timeout - std::thread::spawn(move || { - let timeout = std::time::Duration::from_secs(1); - let start = std::time::Instant::now(); - - loop { - match process.try_wait() { - Ok(Some(_)) => { - return; // Process finished normally - }, - Ok(None) => { - if start.elapsed() > timeout { - let _ = process.kill(); - log::error!("Copy operation times out after 1 second"); - return; - } - std::thread::sleep(std::time::Duration::from_millis(50)); - }, - Err(e) => { - log::error!("Clipboard failure: {}", e); - return; - }, - } - } - }); + reap(process); Ok(()) } } + +/// Wait for the copy command's process in the background, so that it does not linger as a zombie +/// once it exits. +/// +/// The process is deliberately not killed if it outlives the copy operation: X11 and Wayland +/// clipboard helpers (eg. `xsel`, `xclip` or `wl-copy`) keep running for as long as they own the +/// selection, and killing them clears the clipboard. Copy commands that do exit by themselves (eg. +/// `pbcopy`) are reaped as soon as they do. +fn reap(mut process: Child) -> JoinHandle<()> { + std::thread::spawn(move || { + if let Err(e) = process.wait() { + log::error!("Clipboard failure: {}", e); + } + }) +} + +#[cfg(all(test, unix))] +mod tests { + use super::*; + use nix::errno::Errno; + use nix::sys::wait::{waitpid, WaitPidFlag}; + use nix::unistd::Pid; + use std::path::Path; + use std::time::{Duration, Instant}; + + // stands in for an x11/wayland clipboard helper: consumes stdin and then keeps running the way + // `xsel` or `xclip` do while they own the selection + fn fake_clipboard_helper(dir: &Path, received: &Path, still_alive: &Path) -> String { + let script = dir.join("fake-clipboard-helper"); + std::fs::write( + &script, + format!( + "cat > '{}'\nsleep 2\ntouch '{}'\n", + received.display(), + still_alive.display() + ), + ) + .unwrap(); + // run it through `sh` rather than making it executable, so that a concurrent fork/exec + // elsewhere in the test binary can't make this spawn fail with ETXTBSY + format!("sh {}", script.display()) + } + + #[test] + fn copy_command_is_not_killed_when_it_outlives_the_copy() { + let dir = tempfile::tempdir().unwrap(); + let received = dir.path().join("received"); + let still_alive = dir.path().join("still-alive"); + let copy_command = fake_clipboard_helper(dir.path(), &received, &still_alive); + + CopyCommand::new(copy_command) + .set("some copied text".to_owned()) + .unwrap(); + + // poll (bounded to ~15s) for the helper to outlive the copy operation + let deadline = Instant::now() + Duration::from_secs(15); + while !still_alive.exists() && Instant::now() < deadline { + std::thread::sleep(Duration::from_millis(20)); + } + + assert_eq!( + std::fs::read_to_string(&received).unwrap(), + "some copied text", + "the copy command should be fed the copied text on its stdin" + ); + assert!( + still_alive.exists(), + "the copy command should not be killed for outliving the copy operation" + ); + } + + #[test] + fn copy_command_is_reaped_once_it_exits() { + let process = Command::new("sh") + .args(["-c", "exit 0"]) + .stdin(Stdio::piped()) + .spawn() + .unwrap(); + let pid = Pid::from_raw(process.id() as i32); + + reap(process).join().unwrap(); + + // having been waited for, the process is gone from the process table rather than left + // behind as a zombie + assert_eq!( + waitpid(pid, Some(WaitPidFlag::WNOHANG)).err(), + Some(Errno::ECHILD), + "the copy command's process should have been reaped" + ); + } +}