Skip to content

Commit

Permalink
fix #875, win, cwd/environ/cmdline(): retry with incremental timeout …
Browse files Browse the repository at this point in the history
…in case of ERROR_PARTIAL_COPY
  • Loading branch information
giampaolo committed Nov 4, 2019
1 parent 2597253 commit 8b91eeb
Show file tree
Hide file tree
Showing 3 changed files with 60 additions and 17 deletions.
30 changes: 30 additions & 0 deletions psutil/_pswindows.py
Original file line number Diff line number Diff line change
Expand Up @@ -80,6 +80,7 @@
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:
Expand Down Expand Up @@ -709,6 +710,32 @@ def wrapper(self, *args, **kwargs):
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."""

Expand Down Expand Up @@ -772,6 +799,7 @@ def exe(self):
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
Expand All @@ -791,6 +819,7 @@ def cmdline(self):
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:
Expand Down Expand Up @@ -963,6 +992,7 @@ def resume(self):
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)
Expand Down
37 changes: 20 additions & 17 deletions psutil/arch/windows/process_info.c
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -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) {
Expand Down Expand Up @@ -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
Expand All @@ -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) {
Expand Down Expand Up @@ -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);
Expand All @@ -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);
Expand Down
10 changes: 10 additions & 0 deletions psutil/tests/test_windows.py
Original file line number Diff line number Diff line change
Expand Up @@ -517,6 +517,16 @@ def test_num_handles(self):
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):
Expand Down

0 comments on commit 8b91eeb

Please sign in to comment.