From bc8f7f732380c9945a6ced7345b38624bf1d8218 Mon Sep 17 00:00:00 2001 From: Guillaume Gomez Date: Sat, 6 Apr 2024 21:20:07 +0200 Subject: [PATCH 1/5] Update `Process::parent` at every refresh --- src/unix/apple/macos/process.rs | 18 ++++++++++++++---- src/unix/linux/process.rs | 5 +++++ src/windows/process.rs | 2 +- src/windows/system.rs | 13 ++++++++----- 4 files changed, 28 insertions(+), 10 deletions(-) diff --git a/src/unix/apple/macos/process.rs b/src/unix/apple/macos/process.rs index 7712166b9..9b06b3307 100644 --- a/src/unix/apple/macos/process.rs +++ b/src/unix/apple/macos/process.rs @@ -335,6 +335,13 @@ unsafe fn get_bsd_info(pid: Pid) -> Option { } } +fn get_parent(info: &libc::proc_bsdinfo) -> Option { + match info.pbi_ppid as i32 { + 0 => None, + p => Some(Pid(p)), + } +} + unsafe fn create_new_process( pid: Pid, now: u64, @@ -353,10 +360,8 @@ unsafe fn create_new_process( return Err(()); } }; - let parent = match info.pbi_ppid as i32 { - 0 => None, - p => Some(Pid(p)), - }; + + let parent = get_parent(&info); let start_time = info.pbi_start_tvsec; let run_time = now.saturating_sub(start_time); @@ -642,6 +647,11 @@ pub(crate) fn update_process( // The owner of this PID changed. return create_new_process(pid, now, refresh_kind, Some(info)); } + let parent = get_parent(&info); + // Update the parent if it changed. + if p.parent != parent { + p.parent = parent; + } } if !get_process_infos(p, refresh_kind) { diff --git a/src/unix/linux/process.rs b/src/unix/linux/process.rs index ef324acd7..abd2016d0 100644 --- a/src/unix/linux/process.rs +++ b/src/unix/linux/process.rs @@ -510,6 +510,11 @@ pub(crate) fn _get_process_data( let parts = parse_stat_file(&data).ok_or(())?; let start_time_without_boot_time = compute_start_time_without_boot_time(&parts, info); + // Update the parent if it changed. + if entry.parent != parent_pid { + entry.parent = parent_pid; + } + // It's possible that a new process took this same PID when the "original one" terminated. // If the start time differs, then it means it's not the same process anymore and that we // need to get all its information, hence why we check it here. diff --git a/src/windows/process.rs b/src/windows/process.rs index b95b1cbe8..2896012c1 100644 --- a/src/windows/process.rs +++ b/src/windows/process.rs @@ -208,7 +208,7 @@ pub(crate) struct ProcessInner { root: Option, pub(crate) memory: u64, pub(crate) virtual_memory: u64, - parent: Option, + pub(crate) parent: Option, status: ProcessStatus, handle: Option>, cpu_calc_values: CPUsageCalculationValues, diff --git a/src/windows/system.rs b/src/windows/system.rs index 74d8eca75..37730725d 100644 --- a/src/windows/system.rs +++ b/src/windows/system.rs @@ -309,6 +309,11 @@ impl SystemInner { // as above, read_unaligned is necessary let pi = ptr::read_unaligned(pi.0); let pid = Pid(pi.UniqueProcessId as _); + let parent = if pi.InheritedFromUniqueProcessId as usize != 0 { + Some(Pid(pi.InheritedFromUniqueProcessId as _)) + } else { + None + }; if let Some(proc_) = (*process_list.0.get()).get_mut(&pid) { let proc_ = &mut proc_.inner; if proc_ @@ -321,6 +326,8 @@ impl SystemInner { proc_.virtual_memory = pi.VirtualSize as _; } proc_.update(refresh_kind, nb_cpus, now); + // Update the parent in case it changed. + proc_.parent = parent; return None; } // If the PID owner changed, we need to recompute the whole process. @@ -334,11 +341,7 @@ impl SystemInner { }; let mut p = ProcessInner::new_full( pid, - if pi.InheritedFromUniqueProcessId as usize != 0 { - Some(Pid(pi.InheritedFromUniqueProcessId as _)) - } else { - None - }, + parent, memory, virtual_memory, name, From e342557f0919df779b80f0cccd7190ddb9ff5a9e Mon Sep 17 00:00:00 2001 From: Guillaume Gomez Date: Sat, 6 Apr 2024 21:20:22 +0200 Subject: [PATCH 2/5] Add regression test for `Process::parent` update --- test_bin/main.rs | 22 +++++++++++++++++++++- tests/process.rs | 47 +++++++++++++++++++++++++++++++++++++++++++++++ 2 files changed, 68 insertions(+), 1 deletion(-) diff --git a/test_bin/main.rs b/test_bin/main.rs index d15425064..8957b0037 100644 --- a/test_bin/main.rs +++ b/test_bin/main.rs @@ -1,5 +1,25 @@ // Does nothing and just exits after waiting for 30 seconds. +use std::process::{Child, Command}; + +fn maybe_start_child(last: String, args: &[String]) -> Option { + if last == "1" { + let mut cmd = Command::new(&args[0]); + for arg in &args[1..] { + cmd.arg(arg); + } + Some(cmd.spawn().expect("failed to run command")) + } else { + None + } +} + fn main() { - std::thread::sleep(std::time::Duration::from_secs(30)); + let mut args: Vec = std::env::args().collect(); + let child = args.pop().and_then(|last| maybe_start_child(last, &args)); + if child.is_some() { + std::thread::sleep(std::time::Duration::from_secs(3)); + } else { + std::thread::sleep(std::time::Duration::from_secs(30)); + } } diff --git a/tests/process.rs b/tests/process.rs index 132f8dc78..b48a4ad18 100644 --- a/tests/process.rs +++ b/tests/process.rs @@ -801,3 +801,50 @@ fn test_process_run_time() { run_time ); } + +// Test that if the parent of a process is removed, then the child PID will be +// updated as well. +#[test] +fn test_parent_change() { + if !sysinfo::IS_SUPPORTED_SYSTEM || cfg!(feature = "apple-sandbox") { + return; + } + + build_test_binary(); + let mut p = std::process::Command::new("./target/test_binary") + .arg("1") + .spawn() + .unwrap(); + + std::thread::sleep(std::time::Duration::from_secs(1)); + + let pid = Pid::from_u32(p.id() as _); + let mut s = System::new(); + s.refresh_processes(); + + assert_eq!( + s.process(pid).expect("process was not created").parent(), + sysinfo::get_current_pid().ok(), + ); + + let child_pid = s + .processes() + .iter() + .find(|(_, proc_)| proc_.parent() == Some(pid)) + .map(|(pid, _)| *pid) + .expect("failed to get child process"); + + // Waiting for the parent process to stop. + p.wait().expect("wait failed"); + + s.refresh_processes(); + // Parent should not be around anymore. + assert!(s.process(pid).is_none()); + + let child = s.process(child_pid).expect("child is dead"); + // Child should have a different parent now. + assert_ne!(child.parent(), Some(pid)); + + // We kill the child to clean up. + child.kill(); +} From edfbe928e7c68267e6e73aab7fba478c18b2ece7 Mon Sep 17 00:00:00 2001 From: Guillaume Gomez Date: Sat, 6 Apr 2024 22:17:52 +0200 Subject: [PATCH 3/5] Fix linux `Process::parent` retrieval --- src/unix/linux/network.rs | 2 +- src/unix/linux/process.rs | 28 +++++++++++++++------------- 2 files changed, 16 insertions(+), 14 deletions(-) diff --git a/src/unix/linux/network.rs b/src/unix/linux/network.rs index a428cc276..ca90cfa94 100644 --- a/src/unix/linux/network.rs +++ b/src/unix/linux/network.rs @@ -1,9 +1,9 @@ // Take a look at the license at the top of the repository in the LICENSE file. use std::collections::{hash_map, HashMap}; +use std::fs::File; use std::io::Read; use std::path::Path; -use std::{fs::File, u8}; use crate::common::{IpNetwork, MacAddr}; use crate::network::refresh_networks_addresses; diff --git a/src/unix/linux/process.rs b/src/unix/linux/process.rs index abd2016d0..cb55993ab 100644 --- a/src/unix/linux/process.rs +++ b/src/unix/linux/process.rs @@ -397,12 +397,15 @@ fn refresh_user_group_ids( #[allow(clippy::too_many_arguments)] fn update_proc_info( p: &mut ProcessInner, + parent_pid: Option, refresh_kind: ProcessRefreshKind, proc_path: &mut PathHandler, str_parts: &[&str], uptime: u64, info: &SystemInfo, ) { + update_parent_pid(p, parent_pid, str_parts); + get_status(p, str_parts[ProcIndex::State as usize]); refresh_user_group_ids(p, proc_path, refresh_kind); @@ -431,6 +434,16 @@ fn update_proc_info( } } +fn update_parent_pid(p: &mut ProcessInner, parent_pid: Option, str_parts: &[&str]) { + p.parent = match parent_pid { + Some(parent_pid) if parent_pid.0 != 0 => Some(parent_pid), + _ => match Pid::from_str(str_parts[ProcIndex::ParentPid as usize]) { + Ok(p) if p.0 != 0 => Some(p), + _ => None, + }, + }; +} + fn retrieve_all_new_process_info( pid: Pid, parent_pid: Option, @@ -444,14 +457,6 @@ fn retrieve_all_new_process_info( let mut proc_path = PathHandler::new(path); let name = parts.short_exe; - p.parent = match parent_pid { - Some(parent_pid) if parent_pid.0 != 0 => Some(parent_pid), - _ => match Pid::from_str(parts.str_parts[ProcIndex::ParentPid as usize]) { - Ok(p) if p.0 != 0 => Some(p), - _ => None, - }, - }; - p.start_time_without_boot_time = compute_start_time_without_boot_time(parts, info); p.start_time = p .start_time_without_boot_time @@ -469,6 +474,7 @@ fn retrieve_all_new_process_info( update_proc_info( &mut p, + parent_pid, refresh_kind, &mut proc_path, &parts.str_parts, @@ -510,11 +516,6 @@ pub(crate) fn _get_process_data( let parts = parse_stat_file(&data).ok_or(())?; let start_time_without_boot_time = compute_start_time_without_boot_time(&parts, info); - // Update the parent if it changed. - if entry.parent != parent_pid { - entry.parent = parent_pid; - } - // It's possible that a new process took this same PID when the "original one" terminated. // If the start time differs, then it means it's not the same process anymore and that we // need to get all its information, hence why we check it here. @@ -523,6 +524,7 @@ pub(crate) fn _get_process_data( update_proc_info( entry, + parent_pid, refresh_kind, &mut proc_path, &parts.str_parts, From 38bceab57714f0bcbf76bc56dbbbaf2a3597d411 Mon Sep 17 00:00:00 2001 From: Guillaume Gomez Date: Sat, 6 Apr 2024 22:43:50 +0200 Subject: [PATCH 4/5] Prevent dual compilation of `test_binary` --- tests/process.rs | 14 ++++++++------ 1 file changed, 8 insertions(+), 6 deletions(-) diff --git a/tests/process.rs b/tests/process.rs index b48a4ad18..a97f98c8b 100644 --- a/tests/process.rs +++ b/tests/process.rs @@ -81,11 +81,11 @@ fn test_cmd() { } } -fn build_test_binary() { +fn build_test_binary(file_name: &str) { std::process::Command::new("rustc") .arg("test_bin/main.rs") .arg("-o") - .arg("target/test_binary") + .arg(file_name) .stdout(std::process::Stdio::null()) .spawn() .unwrap() @@ -98,8 +98,9 @@ fn test_environ() { if !sysinfo::IS_SUPPORTED_SYSTEM || cfg!(feature = "apple-sandbox") { return; } - build_test_binary(); - let mut p = std::process::Command::new("./target/test_binary") + let file_name = "target/test_binary"; + build_test_binary(file_name); + let mut p = std::process::Command::new(format!("./{file_name}")) .env("FOO", "BAR") .env("OTHER", "VALUE") .spawn() @@ -810,8 +811,9 @@ fn test_parent_change() { return; } - build_test_binary(); - let mut p = std::process::Command::new("./target/test_binary") + let file_name = "target/test_binary2"; + build_test_binary(file_name); + let mut p = std::process::Command::new(format!("./{file_name}")) .arg("1") .spawn() .unwrap(); From 3fd2df9b8b9ee41fc4917fda904992f9a253969d Mon Sep 17 00:00:00 2001 From: Guillaume Gomez Date: Sun, 7 Apr 2024 01:44:51 +0200 Subject: [PATCH 5/5] Improve Windows code --- src/common.rs | 2 +- src/windows/disk.rs | 36 ++------ src/windows/process.rs | 205 ++++++++++++++--------------------------- src/windows/system.rs | 24 ++--- src/windows/utils.rs | 50 +++++++++- tests/process.rs | 3 +- 6 files changed, 134 insertions(+), 186 deletions(-) diff --git a/src/common.rs b/src/common.rs index 630885af1..d4fb53e57 100644 --- a/src/common.rs +++ b/src/common.rs @@ -1627,7 +1627,7 @@ impl UpdateKind { /// /// When all refresh are ruled out, a [`Process`] will still retrieve the following information: /// * Process ID ([`Pid`]) -/// * Parent process ID +/// * Parent process ID (on Windows it never changes though) /// * Process name /// * Start time /// diff --git a/src/windows/disk.rs b/src/windows/disk.rs index 440743b3d..16c7eafdb 100644 --- a/src/windows/disk.rs +++ b/src/windows/disk.rs @@ -1,5 +1,6 @@ // Take a look at the license at the top of the repository in the LICENSE file. +use crate::sys::utils::HandleWrapper; use crate::{Disk, DiskKind}; use std::ffi::{c_void, OsStr, OsString}; @@ -8,11 +9,10 @@ use std::os::windows::ffi::OsStringExt; use std::path::Path; use windows::core::{Error, HRESULT, PCWSTR}; -use windows::Win32::Foundation::{CloseHandle, HANDLE, MAX_PATH}; +use windows::Win32::Foundation::MAX_PATH; use windows::Win32::Storage::FileSystem::{ - CreateFileW, FindFirstVolumeW, FindNextVolumeW, FindVolumeClose, GetDiskFreeSpaceExW, - GetDriveTypeW, GetVolumeInformationW, GetVolumePathNamesForVolumeNameW, FILE_ACCESS_RIGHTS, - FILE_SHARE_READ, FILE_SHARE_WRITE, OPEN_EXISTING, + FindFirstVolumeW, FindNextVolumeW, FindVolumeClose, GetDiskFreeSpaceExW, GetDriveTypeW, + GetVolumeInformationW, GetVolumePathNamesForVolumeNameW, }; use windows::Win32::System::Ioctl::{ PropertyStandardQuery, StorageDeviceSeekPenaltyProperty, DEVICE_SEEK_PENALTY_DESCRIPTOR, @@ -205,31 +205,6 @@ impl DisksInner { } } -struct HandleWrapper(HANDLE); - -impl HandleWrapper { - unsafe fn new(drive_name: &[u16], open_rights: FILE_ACCESS_RIGHTS) -> Option { - let lpfilename = PCWSTR::from_raw(drive_name.as_ptr()); - let handle = CreateFileW( - lpfilename, - open_rights.0, - FILE_SHARE_READ | FILE_SHARE_WRITE, - None, - OPEN_EXISTING, - Default::default(), - HANDLE::default(), - ) - .ok()?; - Some(Self(handle)) - } -} - -impl Drop for HandleWrapper { - fn drop(&mut self) { - let _err = unsafe { CloseHandle(self.0) }; - } -} - unsafe fn get_drive_size(mount_point: &[u16]) -> Option<(u64, u64)> { let mut total_size = 0; let mut available_space = 0; @@ -292,7 +267,8 @@ pub(crate) unsafe fn get_list() -> Vec { .copied() .chain([0]) .collect::>(); - let Some(handle) = HandleWrapper::new(&device_path[..], Default::default()) else { + let Some(handle) = HandleWrapper::new_from_file(&device_path[..], Default::default()) + else { return Vec::new(); }; let Some((total_space, available_space)) = get_drive_size(&mount_paths[0][..]) else { diff --git a/src/windows/process.rs b/src/windows/process.rs index 2896012c1..68ee6f471 100644 --- a/src/windows/process.rs +++ b/src/windows/process.rs @@ -1,6 +1,7 @@ // Take a look at the license at the top of the repository in the LICENSE file. use crate::sys::system::is_proc_running; +use crate::sys::utils::HandleWrapper; use crate::windows::Sid; use crate::{DiskUsage, Gid, Pid, ProcessRefreshKind, ProcessStatus, Signal, Uid}; @@ -9,7 +10,6 @@ use std::fmt; #[cfg(feature = "debug")] use std::io; use std::mem::{size_of, zeroed, MaybeUninit}; -use std::ops::Deref; use std::os::windows::ffi::OsStringExt; use std::os::windows::process::CommandExt; use std::path::{Path, PathBuf}; @@ -30,9 +30,8 @@ use windows::Wdk::System::Threading::{ ProcessWow64Information, PROCESSINFOCLASS, }; use windows::Win32::Foundation::{ - CloseHandle, LocalFree, ERROR_INSUFFICIENT_BUFFER, FILETIME, HANDLE, HINSTANCE, HLOCAL, - MAX_PATH, STATUS_BUFFER_OVERFLOW, STATUS_BUFFER_TOO_SMALL, STATUS_INFO_LENGTH_MISMATCH, - UNICODE_STRING, + LocalFree, ERROR_INSUFFICIENT_BUFFER, FILETIME, HANDLE, HINSTANCE, HLOCAL, MAX_PATH, + STATUS_BUFFER_OVERFLOW, STATUS_BUFFER_TOO_SMALL, STATUS_INFO_LENGTH_MISMATCH, UNICODE_STRING, }; use windows::Win32::Security::{GetTokenInformation, TokenUser, TOKEN_QUERY, TOKEN_USER}; use windows::Win32::System::Diagnostics::Debug::ReadProcessMemory; @@ -167,32 +166,6 @@ unsafe fn get_process_user_id(process: &mut ProcessInner, refresh_kind: ProcessR } } -struct HandleWrapper(HANDLE); - -impl HandleWrapper { - fn new(handle: HANDLE) -> Option { - if handle.is_invalid() { - None - } else { - Some(Self(handle)) - } - } -} - -impl Deref for HandleWrapper { - type Target = HANDLE; - - fn deref(&self) -> &Self::Target { - &self.0 - } -} - -impl Drop for HandleWrapper { - fn drop(&mut self) { - let _err = unsafe { CloseHandle(self.0) }; - } -} - #[allow(clippy::non_send_fields_in_send_ty)] unsafe impl Send for HandleWrapper {} unsafe impl Sync for HandleWrapper {} @@ -354,48 +327,20 @@ unsafe fn get_exe(process_handler: &HandleWrapper) -> Option { } impl ProcessInner { - pub(crate) fn new_from_pid( - pid: Pid, - now: u64, - refresh_kind: ProcessRefreshKind, - ) -> Option { + pub(crate) fn new_from_pid(pid: Pid, now: u64) -> Option { unsafe { let process_handler = get_process_handler(pid)?; - let mut info: MaybeUninit = MaybeUninit::uninit(); - if NtQueryInformationProcess( - process_handler.0, - ProcessBasicInformation, - info.as_mut_ptr().cast(), - size_of::() as _, - null_mut(), - ) - .is_err() - { - return None; - } - let info = info.assume_init(); - let name = get_process_name(pid).unwrap_or_default(); - let exe = if refresh_kind.exe().needs_update(|| true) { - get_exe(&process_handler) - } else { - None - }; let (start_time, run_time) = get_start_and_run_time(*process_handler, now); - let parent = if info.InheritedFromUniqueProcessId != 0 { - Some(Pid(info.InheritedFromUniqueProcessId as _)) - } else { - None - }; - let mut p = Self { + Some(Self { handle: Some(Arc::new(process_handler)), name, pid, - parent, + parent: None, user_id: None, cmd: Vec::new(), environ: Vec::new(), - exe, + exe: None, cwd: None, root: None, status: ProcessStatus::Run, @@ -410,10 +355,7 @@ impl ProcessInner { old_written_bytes: 0, read_bytes: 0, written_bytes: 0, - }; - get_process_user_id(&mut p, refresh_kind); - get_process_params(&mut p, refresh_kind); - Some(p) + }) } } @@ -424,75 +366,36 @@ impl ProcessInner { virtual_memory: u64, name: OsString, now: u64, - refresh_kind: ProcessRefreshKind, ) -> Self { - if let Some(handle) = get_process_handler(pid) { - unsafe { - let exe = if refresh_kind.exe().needs_update(|| true) { - get_exe(&handle) - } else { - None - }; - let (start_time, run_time) = get_start_and_run_time(*handle, now); - let mut p = Self { - handle: Some(Arc::new(handle)), - name, - pid, - user_id: None, - parent, - cmd: Vec::new(), - environ: Vec::new(), - exe, - cwd: None, - root: None, - status: ProcessStatus::Run, - memory, - virtual_memory, - cpu_usage: 0., - cpu_calc_values: CPUsageCalculationValues::new(), - start_time, - run_time, - updated: true, - old_read_bytes: 0, - old_written_bytes: 0, - read_bytes: 0, - written_bytes: 0, - }; - - get_process_user_id(&mut p, refresh_kind); - get_process_params(&mut p, refresh_kind); - p - } + let (handle, start_time, run_time) = if let Some(handle) = get_process_handler(pid) { + let (start_time, run_time) = get_start_and_run_time(*handle, now); + (Some(Arc::new(handle)), start_time, run_time) } else { - let exe = if refresh_kind.exe().needs_update(|| true) { - get_executable_path(pid) - } else { - None - }; - Self { - handle: None, - name, - pid, - user_id: None, - parent, - cmd: Vec::new(), - environ: Vec::new(), - exe, - cwd: None, - root: None, - status: ProcessStatus::Run, - memory, - virtual_memory, - cpu_usage: 0., - cpu_calc_values: CPUsageCalculationValues::new(), - start_time: 0, - run_time: 0, - updated: true, - old_read_bytes: 0, - old_written_bytes: 0, - read_bytes: 0, - written_bytes: 0, - } + (None, 0, 0) + }; + Self { + handle, + name, + pid, + user_id: None, + parent, + cmd: Vec::new(), + environ: Vec::new(), + exe: None, + cwd: None, + root: None, + status: ProcessStatus::Run, + memory, + virtual_memory, + cpu_usage: 0., + cpu_calc_values: CPUsageCalculationValues::new(), + start_time, + run_time, + updated: true, + old_read_bytes: 0, + old_written_bytes: 0, + read_bytes: 0, + written_bytes: 0, } } @@ -501,6 +404,7 @@ impl ProcessInner { refresh_kind: crate::ProcessRefreshKind, nb_cpus: u64, now: u64, + refresh_parent: bool, ) { if refresh_kind.cpu() { compute_cpu_usage(self, nb_cpus); @@ -513,7 +417,7 @@ impl ProcessInner { } unsafe { get_process_user_id(self, refresh_kind); - get_process_params(self, refresh_kind); + get_process_params(self, refresh_kind, refresh_parent); } if refresh_kind.exe().needs_update(|| self.exe.is_none()) { unsafe { @@ -858,16 +762,25 @@ macro_rules! impl_RtlUserProcessParameters { impl_RtlUserProcessParameters!(RTL_USER_PROCESS_PARAMETERS32); impl_RtlUserProcessParameters!(RTL_USER_PROCESS_PARAMETERS); -unsafe fn get_process_params(process: &mut ProcessInner, refresh_kind: ProcessRefreshKind) { - if !(refresh_kind.cmd().needs_update(|| process.cmd.is_empty()) +fn has_anything_to_update(process: &ProcessInner, refresh_kind: ProcessRefreshKind) -> bool { + refresh_kind.cmd().needs_update(|| process.cmd.is_empty()) || refresh_kind .environ() .needs_update(|| process.environ.is_empty()) || refresh_kind.cwd().needs_update(|| process.cwd.is_none()) - || refresh_kind.root().needs_update(|| process.root.is_none())) - { + || refresh_kind.root().needs_update(|| process.root.is_none()) +} + +unsafe fn get_process_params( + process: &mut ProcessInner, + refresh_kind: ProcessRefreshKind, + refresh_parent: bool, +) { + let has_anything_to_update = has_anything_to_update(process, refresh_kind); + if !refresh_parent && !has_anything_to_update { return; } + let handle = match process.handle.as_ref().map(|handle| handle.0) { Some(h) => h, None => return, @@ -907,6 +820,18 @@ unsafe fn get_process_params(process: &mut ProcessInner, refresh_kind: ProcessRe } let pinfo = pbasicinfo.assume_init(); + let ppid: usize = pinfo.InheritedFromUniqueProcessId as _; + let parent = if ppid != 0 { + Some(Pid(pinfo.InheritedFromUniqueProcessId as _)) + } else { + None + }; + process.parent = parent; + + if !has_anything_to_update { + return; + } + let mut peb = MaybeUninit::::uninit(); if ReadProcessMemory( handle, @@ -950,6 +875,10 @@ unsafe fn get_process_params(process: &mut ProcessInner, refresh_kind: ProcessRe } // target is a 32 bit process in wow64 mode + if !has_anything_to_update { + return; + } + let mut peb32 = MaybeUninit::::uninit(); if ReadProcessMemory( handle, diff --git a/src/windows/system.rs b/src/windows/system.rs index 37730725d..0a9c6149f 100644 --- a/src/windows/system.rs +++ b/src/windows/system.rs @@ -184,8 +184,8 @@ impl SystemInner { } // We need to re-make the process because the PID owner changed. } - if let Some(mut p) = ProcessInner::new_from_pid(pid, now, refresh_kind) { - p.update(refresh_kind, nb_cpus, now); + if let Some(mut p) = ProcessInner::new_from_pid(pid, now) { + p.update(refresh_kind, nb_cpus, now, true); p.updated = false; self.process_list.insert(pid, Process { inner: p }); true @@ -309,7 +309,8 @@ impl SystemInner { // as above, read_unaligned is necessary let pi = ptr::read_unaligned(pi.0); let pid = Pid(pi.UniqueProcessId as _); - let parent = if pi.InheritedFromUniqueProcessId as usize != 0 { + let ppid: usize = pi.InheritedFromUniqueProcessId as _; + let parent = if ppid != 0 { Some(Pid(pi.InheritedFromUniqueProcessId as _)) } else { None @@ -325,7 +326,7 @@ impl SystemInner { proc_.memory = pi.WorkingSetSize as _; proc_.virtual_memory = pi.VirtualSize as _; } - proc_.update(refresh_kind, nb_cpus, now); + proc_.update(refresh_kind, nb_cpus, now, false); // Update the parent in case it changed. proc_.parent = parent; return None; @@ -339,16 +340,9 @@ impl SystemInner { } else { (0, 0) }; - let mut p = ProcessInner::new_full( - pid, - parent, - memory, - virtual_memory, - name, - now, - refresh_kind, - ); - p.update(refresh_kind.without_memory(), nb_cpus, now); + let mut p = + ProcessInner::new_full(pid, parent, memory, virtual_memory, name, now); + p.update(refresh_kind.without_memory(), nb_cpus, now, false); Some(Process { inner: p }) }) .collect::>(); @@ -533,7 +527,7 @@ fn refresh_existing_process( } else { return Some(false); } - proc_.update(refresh_kind, nb_cpus, now); + proc_.update(refresh_kind, nb_cpus, now, false); proc_.updated = false; Some(true) } diff --git a/src/windows/utils.rs b/src/windows/utils.rs index 1510f08a1..06fc69c0b 100644 --- a/src/windows/utils.rs +++ b/src/windows/utils.rs @@ -1,12 +1,16 @@ // Take a look at the license at the top of the repository in the LICENSE file. use windows::core::{PCWSTR, PWSTR}; -use windows::Win32::Foundation::{self, FILETIME}; +use windows::Win32::Foundation::{self, CloseHandle, FILETIME, HANDLE}; +use windows::Win32::Storage::FileSystem::{ + CreateFileW, FILE_ACCESS_RIGHTS, FILE_SHARE_READ, FILE_SHARE_WRITE, OPEN_EXISTING, +}; use windows::Win32::System::Registry::{ RegCloseKey, RegOpenKeyExW, RegQueryValueExW, HKEY, KEY_READ, REG_NONE, }; use std::ffi::OsStr; +use std::ops::Deref; use std::os::windows::ffi::OsStrExt; use std::time::SystemTime; @@ -132,3 +136,47 @@ pub(crate) fn get_reg_value_u32(hkey: HKEY, path: &str, field_name: &str) -> Opt .ok() } } + +pub(crate) struct HandleWrapper(pub(crate) HANDLE); + +impl HandleWrapper { + pub(crate) fn new(handle: HANDLE) -> Option { + if handle.is_invalid() { + None + } else { + Some(Self(handle)) + } + } + + pub(crate) unsafe fn new_from_file( + drive_name: &[u16], + open_rights: FILE_ACCESS_RIGHTS, + ) -> Option { + let lpfilename = PCWSTR::from_raw(drive_name.as_ptr()); + let handle = CreateFileW( + lpfilename, + open_rights.0, + FILE_SHARE_READ | FILE_SHARE_WRITE, + None, + OPEN_EXISTING, + Default::default(), + HANDLE::default(), + ) + .ok()?; + Some(Self(handle)) + } +} + +impl Deref for HandleWrapper { + type Target = HANDLE; + + fn deref(&self) -> &Self::Target { + &self.0 + } +} + +impl Drop for HandleWrapper { + fn drop(&mut self) { + let _err = unsafe { CloseHandle(self.0) }; + } +} diff --git a/tests/process.rs b/tests/process.rs index a97f98c8b..f4dfe5798 100644 --- a/tests/process.rs +++ b/tests/process.rs @@ -807,7 +807,8 @@ fn test_process_run_time() { // updated as well. #[test] fn test_parent_change() { - if !sysinfo::IS_SUPPORTED_SYSTEM || cfg!(feature = "apple-sandbox") { + if !sysinfo::IS_SUPPORTED_SYSTEM || cfg!(feature = "apple-sandbox") || cfg!(windows) { + // Windows never updates its parent PID so no need to check anything. return; }