fix #875, win, cwd/environ/cmdline(): retry with incremental timeout in case of ERROR_PARTIAL_COPY

3 files changed, 60 insertions, 17 deletions

diff --git a/psutil/_pswindows.py b/psutil/_pswindows.py
index cf938a6f..636b0af9 100644
--- a/psutil/_pswindows.py
+++ b/psutil/_pswindows.py
@@ -80,6 +80,7 @@ __extra__all__ = [
 CONN_DELETE_TCB = "DELETE_TCB"
 HAS_PROC_IO_PRIORITY = hasattr(cext, "proc_io_priority_get")
 HAS_GETLOADAVG = hasattr(cext, "getloadavg")
+ERROR_PARTIAL_COPY = 299
 
 
 if enum is None:
@@ -709,6 +710,32 @@ def wrap_exceptions(fun):
     return wrapper
 
 
+def retry_error_partial_copy(fun):
+    """Workaround for https://github.com/giampaolo/psutil/issues/875.
+    See: https://stackoverflow.com/questions/4457745#4457745
+    """
+    @functools.wraps(fun)
+    def wrapper(self, *args, **kwargs):
+        delay = 0.0001
+        times = 33
+        for x in range(times):  # retries for roughly 1 second
+            try:
+                return fun(self, *args, **kwargs)
+            except WindowsError as _:
+                err = _
+                if err.winerror == ERROR_PARTIAL_COPY:
+                    time.sleep(delay)
+                    delay = min(delay * 2, 0.04)
+                    continue
+                else:
+                    raise
+        else:
+            msg = "%s retried %s times, converted to AccessDenied as it's " \
+                "still returning %r" % (fun, times, err)
+            raise AccessDenied(pid=self.pid, name=self._name, msg=msg)
+    return wrapper
+
+
 class Process(object):
     """Wrapper class around underlying C implementation."""
 
@@ -772,6 +799,7 @@ class Process(object):
         return py2_strencode(exe)
 
     @wrap_exceptions
+    @retry_error_partial_copy
     def cmdline(self):
         if cext.WINVER >= cext.WINDOWS_8_1:
             # PEB method detects cmdline changes but requires more
@@ -791,6 +819,7 @@ class Process(object):
             return [py2_strencode(s) for s in ret]
 
     @wrap_exceptions
+    @retry_error_partial_copy
     def environ(self):
         ustr = cext.proc_environ(self.pid)
         if ustr and not PY3:
@@ -963,6 +992,7 @@ class Process(object):
         cext.proc_suspend_or_resume(self.pid, False)
 
     @wrap_exceptions
+    @retry_error_partial_copy
     def cwd(self):
         if self.pid in (0, 4):
             raise AccessDenied(self.pid, self._name)
diff --git a/psutil/arch/windows/process_info.c b/psutil/arch/windows/process_info.c
index bd568d56..62735728 100644
--- a/psutil/arch/windows/process_info.c
+++ b/psutil/arch/windows/process_info.c
@@ -512,7 +512,10 @@ psutil_get_process_data(long pid,
 
         // read PEB
         if (!ReadProcessMemory(hProcess, ppeb32, &peb32, sizeof(peb32), NULL)) {
-            goto read_process_memory_error;
+            // May fail with ERROR_PARTIAL_COPY, see:
+            // https://github.com/giampaolo/psutil/issues/875
+            PyErr_SetFromWindowsErr(0);
+            goto error;
         }
 
         // read process parameters
@@ -522,7 +525,10 @@ psutil_get_process_data(long pid,
                                sizeof(procParameters32),
                                NULL))
         {
-            goto read_process_memory_error;
+            // May fail with ERROR_PARTIAL_COPY, see:
+            // https://github.com/giampaolo/psutil/issues/875
+            PyErr_SetFromWindowsErr(0);
+            goto error;
         }
 
         switch (kind) {
@@ -657,7 +663,10 @@ psutil_get_process_data(long pid,
                                sizeof(peb),
                                NULL))
         {
-            goto read_process_memory_error;
+            // May fail with ERROR_PARTIAL_COPY, see:
+            // https://github.com/giampaolo/psutil/issues/875
+            PyErr_SetFromWindowsErr(0);
+            goto error;
         }
 
         // read process parameters
@@ -667,7 +676,10 @@ psutil_get_process_data(long pid,
                                sizeof(procParameters),
                                NULL))
         {
-            goto read_process_memory_error;
+            // May fail with ERROR_PARTIAL_COPY, see:
+            // https://github.com/giampaolo/psutil/issues/875
+            PyErr_SetFromWindowsErr(0);
+            goto error;
         }
 
         switch (kind) {
@@ -718,7 +730,10 @@ psutil_get_process_data(long pid,
     } else
 #endif
     if (!ReadProcessMemory(hProcess, src, buffer, size, NULL)) {
-        goto read_process_memory_error;
+        // May fail with ERROR_PARTIAL_COPY, see:
+        // https://github.com/giampaolo/psutil/issues/875
+        PyErr_SetFromWindowsErr(0);
+        goto error;
     }
 
     CloseHandle(hProcess);
@@ -728,18 +743,6 @@ psutil_get_process_data(long pid,
 
     return 0;
 
-read_process_memory_error:
-    // see: https://github.com/giampaolo/psutil/issues/875
-    if (GetLastError() == ERROR_PARTIAL_COPY) {
-        psutil_debug("ReadProcessMemory() failed with ERROR_PARTIAL_COPY; "
-                     "converting to EACCES");
-        AccessDenied("ReadProcessMemory() failed with ERROR_PARTIAL_COPY");
-    }
-    else {
-        PyErr_SetFromWindowsErr(0);
-    }
-    goto error;
-
 error:
     if (hProcess != NULL)
         CloseHandle(hProcess);
diff --git a/psutil/tests/test_windows.py b/psutil/tests/test_windows.py
index 51b53f0e..1075efdd 100755
--- a/psutil/tests/test_windows.py
+++ b/psutil/tests/test_windows.py
@@ -517,6 +517,16 @@ class TestProcess(unittest.TestCase):
         psutil_value = psutil.Process(self.pid).num_handles()
         self.assertEqual(psutil_value, sys_value)
 
+    def test_error_partial_copy(self):
+        # https://github.com/giampaolo/psutil/issues/875
+        exc = WindowsError()
+        exc.winerror = 299
+        with mock.patch("psutil._psplatform.cext.proc_cwd", side_effect=exc):
+            with mock.patch("time.sleep") as m:
+                p = psutil.Process()
+                self.assertRaises(psutil.AccessDenied, p.cwd)
+        self.assertGreaterEqual(m.call_count, 5)
+
 
 @unittest.skipIf(not WINDOWS, "WINDOWS only")
 class TestProcessWMI(unittest.TestCase):