From 67b68be3ed94cc3953b3c65e90f7aa42609f702b Mon Sep 17 00:00:00 2001 From: Guillaume Gomez Date: Tue, 9 Aug 2022 17:04:38 +0200 Subject: [PATCH 1/2] Fix PID owner change on Windows --- src/windows/process.rs | 40 ++++++++++++++++++++------ src/windows/system.rs | 65 ++++++++++++++++++++++++++++-------------- 2 files changed, 75 insertions(+), 30 deletions(-) diff --git a/src/windows/process.rs b/src/windows/process.rs index ae8a39c47..e26c36a5d 100644 --- a/src/windows/process.rs +++ b/src/windows/process.rs @@ -328,7 +328,7 @@ impl Process { (Vec::new(), Vec::new(), PathBuf::new()) } }; - let (start_time, run_time) = get_start_and_run_time(&process_handler, now); + let (start_time, run_time) = get_start_and_run_time(*process_handler, now); let parent = if info.InheritedFromUniqueProcessId as usize != 0 { Some(Pid(info.InheritedFromUniqueProcessId as _)) } else { @@ -383,7 +383,7 @@ impl Process { (Vec::new(), Vec::new(), PathBuf::new()) } }; - let (start_time, run_time) = get_start_and_run_time(&handle, now); + let (start_time, run_time) = get_start_and_run_time(*handle, now); let user_id = get_process_user_id(&handle, refresh_kind); Process { handle: Some(Arc::new(handle)), @@ -457,6 +457,10 @@ impl Process { pub(crate) fn get_handle(&self) -> Option { self.handle.as_ref().map(|h| ***h) } + + pub(crate) fn get_start_time(&self) -> Option { + self.handle.as_ref().map(|handle| get_start_time(***handle)) + } } impl ProcessExt for Process { @@ -545,23 +549,43 @@ impl ProcessExt for Process { } } -unsafe fn get_start_and_run_time(handle: &HandleWrapper, now: u64) -> (u64, u64) { +#[inline] +unsafe fn get_process_times(handle: HANDLE) -> u64 { let mut fstart: FILETIME = zeroed(); let mut x = zeroed(); GetProcessTimes( - **handle, + handle, &mut fstart as *mut FILETIME, &mut x as *mut FILETIME, &mut x as *mut FILETIME, &mut x as *mut FILETIME, ); - let tmp = super::utils::filetime_to_u64(fstart); + super::utils::filetime_to_u64(fstart) +} + +#[inline] +fn compute_start(process_times: u64) -> u64 { // 11_644_473_600 is the number of seconds between the Windows epoch (1601-01-01) and // the linux epoch (1970-01-01). - let start = tmp / 10_000_000 - 11_644_473_600; - let run_time = check_sub(now, start); - (start, run_time) + process_times / 10_000_000 - 11_644_473_600 +} + +fn get_start_and_run_time(handle: HANDLE, now: u64) -> (u64, u64) { + unsafe { + let process_times = get_process_times(handle); + let start = compute_start(process_times); + let run_time = check_sub(now, start); + (start, run_time) + } +} + +#[inline] +pub(crate) fn get_start_time(handle: HANDLE) -> u64 { + unsafe { + let process_times = get_process_times(handle); + compute_start(process_times) + } } #[allow(clippy::uninit_vec)] diff --git a/src/windows/system.rs b/src/windows/system.rs index ffab40b26..d037c8c24 100644 --- a/src/windows/system.rs +++ b/src/windows/system.rs @@ -9,7 +9,7 @@ use winapi::um::winreg::HKEY_LOCAL_MACHINE; use crate::sys::component::{self, Component}; use crate::sys::cpu::*; use crate::sys::disk::{get_disks, Disk}; -use crate::sys::process::{update_memory, Process}; +use crate::sys::process::{get_start_time, update_memory, Process}; use crate::sys::tools::*; use crate::sys::users::get_users; use crate::sys::utils::get_now; @@ -190,12 +190,17 @@ impl SystemExt for System { #[allow(clippy::map_entry)] fn refresh_process_specifics(&mut self, pid: Pid, refresh_kind: ProcessRefreshKind) -> bool { - if self.process_list.contains_key(&pid) { - return refresh_existing_process(self, pid, refresh_kind); - } let now = get_now(); + let nb_cpus = self.cpus.len() as u64; + + if let Some(proc_) = self.process_list.get_mut(&pid) { + if let Some(ret) = refresh_existing_process(proc_, nb_cpus, now, refresh_kind) { + return ret; + } + // We need to re-make the process because the PID owner changed. + } if let Some(mut p) = Process::new_from_pid(pid, now, refresh_kind) { - p.update(refresh_kind, self.cpus.len() as u64, now); + p.update(refresh_kind, nb_cpus, now); p.updated = false; self.process_list.insert(pid, p); true @@ -266,10 +271,18 @@ impl SystemExt for System { let pi = *pi.0; let pid = Pid(pi.UniqueProcessId as _); if let Some(proc_) = (*process_list.0.get()).get_mut(&pid) { - proc_.memory = (pi.WorkingSetSize as u64) / 1_000; - proc_.virtual_memory = (pi.VirtualSize as u64) / 1_000; - proc_.update(refresh_kind, nb_cpus, now); - return None; + if proc_ + .get_start_time() + .map(|start| start == proc_.start_time()) + .unwrap_or(true) + { + proc_.memory = (pi.WorkingSetSize as u64) / 1_000; + proc_.virtual_memory = (pi.VirtualSize as u64) / 1_000; + proc_.update(refresh_kind, nb_cpus, now); + return None; + } + // If the PID owner changed, we need to recompute the whole process. + sysinfo_debug!("owner changed for PID {}", proc_.pid()); } let name = get_process_name(&pi, pid); let mut p = Process::new_full( @@ -476,22 +489,30 @@ fn is_proc_running(handle: HANDLE) -> bool { } } -fn refresh_existing_process(s: &mut System, pid: Pid, refresh_kind: ProcessRefreshKind) -> bool { - if let Some(ref mut entry) = s.process_list.get_mut(&pid) { - if let Some(handle) = entry.get_handle() { - if !is_proc_running(handle) { - return false; - } - } else { - return false; +/// If it returns `None`, it means that the PID owner changed and that the `Process` must be +/// completely recomputed. +fn refresh_existing_process( + proc_: &mut Process, + nb_cpus: u64, + now: u64, + refresh_kind: ProcessRefreshKind, +) -> Option { + if let Some(handle) = proc_.get_handle() { + if get_start_time(handle) != proc_.start_time() { + sysinfo_debug!("owner changed for PID {}", proc_.pid()); + // PID owner changed! + return None; + } + if !is_proc_running(handle) { + return Some(false); } - update_memory(entry); - entry.update(refresh_kind, s.cpus.len() as u64, get_now()); - entry.updated = false; - true } else { - false + return Some(false); } + update_memory(proc_); + proc_.update(refresh_kind, nb_cpus, now); + proc_.updated = false; + Some(true) } #[allow(clippy::size_of_in_element_count)] From abd247ed3ea1d15cfae6f32447cea45be2eee4fc Mon Sep 17 00:00:00 2001 From: Guillaume Gomez Date: Tue, 9 Aug 2022 17:16:07 +0200 Subject: [PATCH 2/2] Add check for debug feature --- .github/workflows/CI.yml | 8 ++++++++ 1 file changed, 8 insertions(+) diff --git a/.github/workflows/CI.yml b/.github/workflows/CI.yml index b51a39de7..1417b3754 100644 --- a/.github/workflows/CI.yml +++ b/.github/workflows/CI.yml @@ -87,6 +87,14 @@ jobs: command: rustc args: --target=${{ matrix.triple.target }} --manifest-path=Cargo.toml -- -D warnings use-cross: ${{ matrix.triple.cross }} + + - name: Check debug feature + uses: actions-rs/cargo@v1 + with: + command: rustc + args: --target=${{ matrix.triple.target }} --manifest-path=Cargo.toml --features=debug -- -D warnings + use-cross: ${{ matrix.triple.cross }} + - name: Check without multithreading uses: actions-rs/cargo@v1 with: