YaLTeR / wl-clipboard-rs

A safe Rust crate for working with the Wayland clipboard.
Apache License 2.0
223 stars 16 forks source link

Wait for copy_data to avoid zombie cat processes #26

Closed dnut closed 2 years ago

dnut commented 2 years ago

paste::get_contents always leaves a zombie process of cat. I tracked this down to the utils::copy_data function which is used with wait=false in DataSourceHandler::send. There is no straightforward way to clean it up it because the pid is dropped from memory in this function without being reaped.

This is a problem if you read from the clipboard frequently. I have an app that polls multiple times per second so I end up generating hundreds of zombie processes per minute that never go away. Also I have seen an error for too many clients which seemed related, but I haven't been able to reliably reproduce this issue. To reap the zombies, I was forced to put this ugly code in my app. This code would not be necessary if the change from this pr is integrated.

use anyhow::Result as AnyResult;
use std::{process::Command, thread, time};

pub fn spawn_reaper() {
    thread::spawn(|| {
        let delay = time::Duration::from_secs(1);
        let my_pid = std::process::id();
        loop {
            reap_children(my_pid).unwrap_or_else(|err| eprintln!("{:?}", err));
            thread::sleep(delay);
        }
    });
}

pub fn reap_children(parent_pid: u32) -> AnyResult<()> {
    for pid in get_children(parent_pid)? {
        let _ = nix::sys::wait::waitpid(
            nix::unistd::Pid::from_raw(pid as i32),
            Some(nix::sys::wait::WaitPidFlag::WNOHANG),
        )?;
    }

    Ok(())
}

pub fn get_children(pid: u32) -> AnyResult<Vec<u32>> {
    let stdout = Command::new("pgrep")
        .arg("-P")
        .arg(pid.to_string())
        .output()?
        .stdout;

    Ok(String::from_utf8_lossy(&stdout)
        .trim()
        .to_string()
        .split("\n")
        .into_iter()
        .map(|s| s.parse::<u32>())
        .filter(|r| r.is_ok())
        .map(|r| r.unwrap())
        .collect::<Vec<u32>>())
}

This PR is a very small change, it just sets wait=true so the process is reaped immediately rather than being left forever as a zombie.

dnut commented 2 years ago

@YaLTeR any thoughts on this tiny change?

YaLTeR commented 2 years ago

Sorry, I lost this somehow. I'll take a look at this within a day.

YaLTeR commented 2 years ago

Thanks! This makes sense, and it seems that wl-clipboard also does this.