Use new libproc functionality to list group pids

Replace darwin-libproc use with libproc::processes::pids_by_type(),
which is cross-platform. This returns pids _other than the group leader_,
so adjust the expected counts in tests down by one.
This commit is contained in:
Martijn Pieters 2023-02-08 16:31:38 +00:00
parent 86ff623288
commit aa6ec4beaa
No known key found for this signature in database
7 changed files with 36 additions and 93 deletions

View File

@ -12,8 +12,7 @@ The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.0.0/),
### Fix
- Point to a new patched fork of `darwin-libproc`, as the original has been deleted.
This fixes the development builts for pueue on Apple platforms.
- Switched the test suite on MacOS to use the new `libproc::processes::pids_by_type()` API to enumerate PIDs in a program group, removing the need to depend on the unmaintained darwing-librproc library. [#409](https://github.com/Nukesor/pueue/issues/409).
## [3.0.1] - 2022-12-31

25
Cargo.lock generated
View File

@ -111,9 +111,9 @@ dependencies = [
[[package]]
name = "bindgen"
version = "0.59.2"
version = "0.64.0"
source = "registry+https://github.com/rust-lang/crates.io-index"
checksum = "2bd2a9a458e8f4304c52c43ebb0cfbd520289f8379a52e329a38afda99bf8eb8"
checksum = "c4243e6031260db77ede97ad86c27e501d646a27ab57b59a574f725d98ab1fb4"
dependencies = [
"bitflags",
"cexpr",
@ -126,6 +126,7 @@ dependencies = [
"regex",
"rustc-hash",
"shlex",
"syn",
]
[[package]]
@ -446,21 +447,6 @@ dependencies = [
"syn",
]
[[package]]
name = "darwin-libproc"
version = "0.2.0"
source = "git+https://github.com/Nukesor/darwin-libproc.git?branch=master#d783719c2f61a347a409ffe4ef1b2d893eec555a"
dependencies = [
"darwin-libproc-sys",
"libc",
"memchr",
]
[[package]]
name = "darwin-libproc-sys"
version = "0.2.0"
source = "git+https://github.com/Nukesor/darwin-libproc.git?branch=master#d783719c2f61a347a409ffe4ef1b2d893eec555a"
[[package]]
name = "diff"
version = "0.1.13"
@ -870,9 +856,9 @@ dependencies = [
[[package]]
name = "libproc"
version = "0.12.0"
version = "0.13.0"
source = "registry+https://github.com/rust-lang/crates.io-index"
checksum = "0b799ad155d75ce914c467ee5627b62247c20d4aedbd446f821484cebf3cded7"
checksum = "8b18cbf29f8ff3542ba22bdce9ac610fcb75d74bb4e2b306b2a2762242025b4f"
dependencies = [
"bindgen",
"errno",
@ -1280,7 +1266,6 @@ dependencies = [
"byteorder",
"chrono",
"command-group",
"darwin-libproc",
"dirs",
"libproc",
"log",

View File

@ -62,17 +62,9 @@ winapi = { version = "0.3", features = [
# Unix
[target.'cfg(unix)'.dependencies]
libproc = "0.13.0"
whoami = "1"
# Linux only
[target.'cfg(target_os = "linux")'.dependencies]
procfs = { version = "0.14.2", default-features = false }
# Apple only
[target.'cfg(target_vendor = "apple")'.dependencies]
libproc = "0.12.0"
[target.'cfg(target_vendor = "apple")'.dev-dependencies]
# Fork that fixes a version conflict, see heim-rs/darwin-libproc#3
# only used as a test dependency.
darwin-libproc = { git = "https://github.com/Nukesor/darwin-libproc.git", branch = "master" }

View File

@ -4,21 +4,3 @@ use libproc::libproc::{proc_pid, task_info};
pub fn process_exists(pid: u32) -> bool {
proc_pid::pidinfo::<task_info::TaskInfo>(pid.try_into().unwrap(), 0).is_ok()
}
#[cfg(test)]
pub mod tests {
// TODO: swap darwin_libproc out for libproc when that project supports listing pids by
// group id.
use darwin_libproc;
use log::warn;
pub fn get_process_group_pids(pgrp: i32) -> Vec<i32> {
match darwin_libproc::pgrp_only_pids(pgrp) {
Err(error) => {
warn!("Failed to get list of processes in process group {pgrp}: {error}");
Vec::new()
}
Ok(processes) => processes,
}
}
}

View File

@ -10,32 +10,3 @@ pub fn process_exists(pid: u32) -> bool {
},
}
}
#[cfg(test)]
pub mod tests {
use log::warn;
use super::process;
/// Get all processes in a process group
pub fn get_process_group_pids(pgrp: i32) -> Vec<i32> {
let all_processes = match process::all_processes() {
Err(error) => {
warn!("Failed to get full process list: {error}");
return Vec::new();
}
Ok(processes) => processes,
};
// Get all processes whose `stat` can be accessed without any errors.
// If the stat() result matches the process group, use the process PID.
all_processes
.into_iter()
.filter_map(|result| result.ok())
.filter_map(|process| match process.stat() {
Ok(stat) if stat.pgrp == pgrp => Some(process.pid),
_ => None,
})
.collect()
}
}

View File

@ -20,16 +20,12 @@ use command_group::Signal;
mod linux;
#[cfg(target_os = "linux")]
pub use self::linux::process_exists;
#[cfg(all(test, target_os = "linux"))]
use self::linux::tests;
// Apple specific process support
#[cfg(target_vendor = "apple")]
mod apple;
#[cfg(target_vendor = "apple")]
pub use self::apple::process_exists;
#[cfg(all(test, target_vendor = "apple"))]
use self::apple::tests;
// Windows specific process handling
#[cfg(any(target_os = "windows"))]

View File

@ -39,16 +39,34 @@ pub fn kill_child(task_id: usize, child: &mut GroupChild) -> std::io::Result<()>
#[cfg(test)]
mod tests {
use log::warn;
use std::thread::sleep;
use std::time::Duration;
use anyhow::Result;
use command_group::CommandGroup;
use libproc::processes::{pids_by_type, ProcFilter};
use pretty_assertions::assert_eq;
use super::*;
use crate::process_helper::process_exists;
use crate::process_helper::tests::get_process_group_pids;
/// List all PIDs that are part of the process group
pub fn get_process_group_pids(pgrp: u32) -> Vec<u32> {
match pids_by_type(ProcFilter::ByProgramGroup { pgrpid: pgrp }) {
Err(error) => {
warn!("Failed to get list of processes in process group {pgrp}: {error}");
Vec::new()
}
Ok(mut processes) => {
// MacOS doesn't list the main process in this group
if !processes.iter().any(|pid| pid == &pgrp) && !process_is_gone(pgrp) {
processes.push(pgrp)
}
processes
}
}
}
/// Assert that certain process id no longer exists
fn process_is_gone(pid: u32) -> bool {
@ -72,7 +90,7 @@ mod tests {
let mut child = compile_shell_command("sleep 60 & sleep 60 && echo 'this is a test'")
.group_spawn()
.expect("Failed to spawn echo");
let pid: i32 = child.id().try_into().unwrap();
let pid = child.id();
// Sleep a little to give everything a chance to spawn.
sleep(Duration::from_millis(500));
@ -90,7 +108,7 @@ mod tests {
child.try_wait().unwrap_or_default();
// Assert that the direct child (sh -c) has been killed.
assert!(process_is_gone(pid as u32));
assert!(process_is_gone(pid));
// Assert that all child processes have been killed.
assert_eq!(get_process_group_pids(pid).len(), 0);
@ -105,7 +123,7 @@ mod tests {
let mut child = compile_shell_command("sleep 60 & sleep 60 && echo 'this is a test'")
.group_spawn()
.expect("Failed to spawn echo");
let pid: i32 = child.id().try_into().unwrap();
let pid = child.id();
// Sleep a little to give everything a chance to spawn.
sleep(Duration::from_millis(500));
@ -123,7 +141,7 @@ mod tests {
child.try_wait().unwrap_or_default();
// Assert that the direct child (sh -c) has been killed.
assert!(process_is_gone(pid as u32));
assert!(process_is_gone(pid));
// Assert that all child processes have been killed.
assert_eq!(get_process_group_pids(pid).len(), 0);
@ -138,7 +156,7 @@ mod tests {
let mut child = compile_shell_command("bash -c 'sleep 60 && sleep 60' && sleep 60")
.group_spawn()
.expect("Failed to spawn echo");
let pid: i32 = child.id().try_into().unwrap();
let pid = child.id();
// Sleep a little to give everything a chance to spawn.
sleep(Duration::from_millis(500));
@ -156,7 +174,7 @@ mod tests {
child.try_wait().unwrap_or_default();
// Assert that the direct child (sh -c) has been killed.
assert!(process_is_gone(pid as u32));
assert!(process_is_gone(pid));
// Assert that all child processes have been killed.
assert_eq!(get_process_group_pids(pid).len(), 0);
@ -171,7 +189,7 @@ mod tests {
.arg("60")
.group_spawn()
.expect("Failed to spawn echo");
let pid: i32 = child.id().try_into().unwrap();
let pid = child.id();
// Sleep a little to give everything a chance to spawn.
sleep(Duration::from_millis(500));
@ -187,7 +205,7 @@ mod tests {
// collect the exit status; otherwise the child process hangs around as a zombie.
child.try_wait().unwrap_or_default();
assert!(process_is_gone(pid as u32));
assert!(process_is_gone(pid));
Ok(())
}
@ -201,7 +219,7 @@ mod tests {
.arg("sleep 60 & sleep 60 && sleep 60")
.group_spawn()
.expect("Failed to spawn echo");
let pid: i32 = child.id().try_into().unwrap();
let pid = child.id();
// Sleep a little to give everything a chance to spawn.
sleep(Duration::from_millis(500));
@ -219,7 +237,7 @@ mod tests {
child.try_wait().unwrap_or_default();
// Assert that the direct child (sh -c) has been killed.
assert!(process_is_gone(pid as u32));
assert!(process_is_gone(pid));
// Assert that all child processes have been killed.
assert_eq!(get_process_group_pids(pid).len(), 0);