pid_exists() and Process() disagree on whether a pid exists when ERRO…

…R_ACCESS_DENIED (#2394)

## Summary

* OS: Windows
* Bug fix: yes
* Type: core
* Fixes: 2359

## Description

On Windows, `pid_exists()` may return True but `psutil.Process()` raises `NoSuchProcess`. Internally, this happens because of:
https://github.com/giampaolo/psutil/blob/034a1a6996d4ff5116fc45a9c5ed8477d0d73d37/psutil/arch/windows/proc_utils.c#L176-L178.

Differently from UNIX, the assumption in the code that ERROR_ACCESS_DENIED means there's a process to deny access to (hence it exists) is wrong. We therefore remove this assumption and also write a test case which ensures that `pid_exists()`, `Process()` and `pids()` APIs are all consistent with each other. 

As a bonus, I also discovered there are "hidden" PIDs on Windows (oh well!).

.github/workflows/build.yml

@@ -108,7 +108,7 @@ jobs:
         python-version: 3.x
     - name: 'Run linters'
       run: |
-        python3 -m pip install ruff black rstcheck toml-sort sphinx
+        python3 -m pip install ruff==0.3.4 black rstcheck toml-sort sphinx
         make lint-all
 
   # Check sanity of .tar.gz + wheel files

HISTORY.rst

@@ -12,6 +12,8 @@
 
 - 2254_, [Linux]: offline cpus raise NotImplementedError in cpu_freq() (patch by Shade Gladden)
 - 2272_: Add pickle support to psutil Exceptions.
+- 2359_, [Windows], [CRITICAL]: `pid_exists()`_ disagrees with `Process`_ on
+  whether a pid exists when ERROR_ACCESS_DENIED.
 - 2360_, [macOS]: can't compile on macOS < 10.13.  (patch by Ryan Schmidt)
 - 2362_, [macOS]: can't compile on macOS 10.11.  (patch by Ryan Schmidt)
 - 2365_, [macOS]: can't compile on macOS < 10.9.  (patch by Ryan Schmidt)

Makefile

@@ -20,7 +20,7 @@ PY3_DEPS = \
 	pypinfo \
 	requests \
 	rstcheck \
-	ruff \
+	ruff==0.3.4 \
 	setuptools \
 	sphinx_rtd_theme \
 	teyit \

psutil/arch/windows/proc_utils.c

@@ -173,17 +173,15 @@ psutil_pid_is_running(DWORD pid) {
 
     hProcess = OpenProcess(PROCESS_QUERY_LIMITED_INFORMATION, FALSE, pid);
 
-    // Access denied means there's a process to deny access to.
-    if ((hProcess == NULL) && (GetLastError() == ERROR_ACCESS_DENIED))
-        return 1;
-
-    hProcess = psutil_check_phandle(hProcess, pid, 1);
     if (hProcess != NULL) {
+        hProcess = psutil_check_phandle(hProcess, pid, 1);
+        if (hProcess != NULL) {
+            CloseHandle(hProcess);
+            return 1;
+        }
         CloseHandle(hProcess);
-        return 1;
     }
 
-    CloseHandle(hProcess);
     PyErr_Clear();
     return psutil_pid_in_pids(pid);
 }

psutil/tests/__init__.py

@@ -1551,7 +1551,10 @@ class system_namespace:
         ('virtual_memory', (), {}),
     ]
     if HAS_CPU_FREQ:
-        getters += [('cpu_freq', (), {'percpu': True})]
+        if MACOS and platform.machine() == 'arm64':  # skipped due to #1892
+            pass
+        else:
+            getters += [('cpu_freq', (), {'percpu': True})]
     if HAS_GETLOADAVG:
         getters += [('getloadavg', (), {})]
     if HAS_SENSORS_TEMPERATURES:

psutil/tests/test_osx.py

@@ -120,7 +120,9 @@ def test_cpu_count_cores(self):
         self.assertEqual(num, psutil.cpu_count(logical=False))
 
     # TODO: remove this once 1892 is fixed
-    @unittest.skipIf(platform.machine() == 'arm64', "skipped due to #1892")
+    @unittest.skipIf(
+        MACOS and platform.machine() == 'arm64', "skipped due to #1892"
+    )
     def test_cpu_freq(self):
         freq = psutil.cpu_freq()
         self.assertEqual(

psutil/tests/test_process_all.py

@@ -104,12 +104,14 @@ class TestFetchAllProcesses(PsutilTestCase):
     """
 
     def setUp(self):
+        psutil._set_debug(False)
         # Using a pool in a CI env may result in deadlock, see:
         # https://github.com/giampaolo/psutil/issues/2104
         if USE_PROC_POOL:
             self.pool = multiprocessing.Pool()
 
     def tearDown(self):
+        psutil._set_debug(True)
         if USE_PROC_POOL:
             self.pool.terminate()
             self.pool.join()
@@ -459,6 +461,79 @@ def environ(self, ret, info):
             self.assertIsInstance(v, str)
 
 
+class TestPidsRange(PsutilTestCase):
+    """Given pid_exists() return value for a range of PIDs which may or
+    may not exist, make sure that psutil.Process() and psutil.pids()
+    agree with pid_exists(). This guarantees that the 3 APIs are all
+    consistent with each other. See:
+    https://github.com/giampaolo/psutil/issues/2359
+
+    XXX - Note about Windows: it turns out there are some "hidden" PIDs
+    which are not returned by psutil.pids() and are also not revealed
+    by taskmgr.exe and ProcessHacker, still they can be instantiated by
+    psutil.Process() and queried. One of such PIDs is "conhost.exe".
+    Running as_dict() for it reveals that some Process() APIs
+    erroneously raise NoSuchProcess, so we know we have problem there.
+    Let's ignore this for now, since it's quite a corner case (who even
+    imagined hidden PIDs existed on Windows?).
+    """
+
+    def setUp(self):
+        psutil._set_debug(False)
+
+    def tearDown(self):
+        psutil._set_debug(True)
+
+    def test_it(self):
+        def is_linux_tid(pid):
+            try:
+                f = open("/proc/%s/status" % pid, "rb")
+            except FileNotFoundError:
+                return False
+            else:
+                with f:
+                    for line in f:
+                        if line.startswith(b"Tgid:"):
+                            tgid = int(line.split()[1])
+                            # If tgid and pid are different then we're
+                            # dealing with a process TID.
+                            return tgid != pid
+                    raise ValueError("'Tgid' line not found")
+
+        def check(pid):
+            # In case of failure retry up to 3 times in order to avoid
+            # race conditions, especially when running in a CI
+            # environment where PIDs may appear and disappear at any
+            # time.
+            x = 3
+            while True:
+                exists = psutil.pid_exists(pid)
+                try:
+                    if exists:
+                        psutil.Process(pid)
+                        if not WINDOWS:  # see docstring
+                            self.assertIn(pid, psutil.pids())
+                    else:
+                        with self.assertRaises(psutil.NoSuchProcess):
+                            psutil.Process(pid)
+                        if not WINDOWS:  # see docstring
+                            self.assertNotIn(pid, psutil.pids())
+                except (psutil.Error, AssertionError) as err:
+                    x -= 1
+                    if x == 0:
+                        raise
+                else:
+                    return
+
+        for pid in range(1, 3000):
+            if LINUX and is_linux_tid(pid):
+                # On Linux a TID (thread ID) can be passed to the
+                # Process class and is querable like a PID (process
+                # ID). Skip it.
+                continue
+            check(pid)
+
+
 if __name__ == '__main__':
     from psutil.tests.runner import run_from_name