Fix PID owner change on Windows

.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:

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<HANDLE> {
         self.handle.as_ref().map(|h| ***h)
     }
+
+    pub(crate) fn get_start_time(&self) -> Option<u64> {
+        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)]

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<bool> {
+    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)]