1224 fix windows Process.wait() (#1253)

* define C TimeoutExpired exception

* fix git commit hook cmd on win

* raise TimeoutAbandoned on WAIT_ABANDONED and rely on a pure-python internal polling

* update HISTORY

* update HISTORY

* update comments

* update comments

5 files changed, 81 insertions, 17 deletions

diff --git a/HISTORY.rst b/HISTORY.rst
index 5279f203..d0bb4b0a 100644
--- a/HISTORY.rst
+++ b/HISTORY.rst
@@ -29,6 +29,7 @@ XXXX-XX-XX
 - 1216_: fix compatibility with python 2.6 on Windows (patch by Dan Vinakovsky)
 - 1222_: [Linux] Process.memory_full_info() was erroneously summing "Swap:" and
   "SwapPss:". Same for "Pss:" and "SwapPss". Not anymore.
+- 1224_: [Windows] Process.wait() may erroneously raise TimeoutExpired.
 - 1238_: [Linux] sensors_battery() may return None in case battery is not
   listed as "BAT0" under /sys/class/power_supply.
 - 1240_: [Windows] cpu_times() float loses accuracy in a long running system.
diff --git a/Makefile b/Makefile
index d9f4a854..f7b4cd7f 100644
--- a/Makefile
+++ b/Makefile
@@ -264,7 +264,7 @@ bench-oneshot-2:  ## Same as above but using perf module (supposed to be more pr
 	$(TEST_PREFIX) $(PYTHON) scripts/internal/bench_oneshot_2.py
 
 check-broken-links:  ## Look for broken links in source files.
-		git ls-files | xargs $(PYTHON) -Wa scripts/internal/check_broken_links.py
+	git ls-files | xargs $(PYTHON) -Wa scripts/internal/check_broken_links.py
 
 help: ## Display callable targets.
 	@grep -E '^[a-zA-Z_-]+:.*?## .*$$' $(MAKEFILE_LIST) | sort | awk 'BEGIN {FS = ":.*?## "}; {printf "\033[36m%-20s\033[0m %s\n", $$1, $$2}'
diff --git a/psutil/_psutil_windows.c b/psutil/_psutil_windows.c
index f164168a..bb993dbd 100644
--- a/psutil/_psutil_windows.c
+++ b/psutil/_psutil_windows.c
@@ -201,6 +201,9 @@ psutil_get_nic_addresses() {
  * ============================================================================
  */
 
+// Raised by Process.wait().
+static PyObject *TimeoutExpired;
+static PyObject *TimeoutAbandoned;
 
 static ULONGLONG (*psutil_GetTickCount64)(void) = NULL;
 
@@ -400,20 +403,33 @@ psutil_proc_wait(PyObject *self, PyObject *args) {
     retVal = WaitForSingleObject(hProcess, timeout);
     Py_END_ALLOW_THREADS
 
+    // handle return code
     if (retVal == WAIT_FAILED) {
         CloseHandle(hProcess);
-        return PyErr_SetFromWindowsErr(0);
+        PyErr_SetFromWindowsErr(0);
+        return NULL;
     }
     if (retVal == WAIT_TIMEOUT) {
         CloseHandle(hProcess);
-        return Py_BuildValue("l", WAIT_TIMEOUT);
+        PyErr_SetString(TimeoutExpired,
+                        "WaitForSingleObject() returned WAIT_TIMEOUT");
+        return NULL;
+    }
+    if (retVal == WAIT_ABANDONED) {
+        psutil_debug("WaitForSingleObject() -> WAIT_ABANDONED");
+        CloseHandle(hProcess);
+        PyErr_SetString(TimeoutAbandoned,
+                        "WaitForSingleObject() returned WAIT_ABANDONED");
+        return NULL;
     }
 
+    // WaitForSingleObject() returned WAIT_OBJECT_0. It means the
+    // process is gone so we can get its process exit code. The PID
+    // may still stick around though but we'll handle that from Python.
     if (GetExitCodeProcess(hProcess, &ExitCode) == 0) {
         CloseHandle(hProcess);
         return PyErr_SetFromWindowsErr(GetLastError());
     }
-
     CloseHandle(hProcess);
 
 #if PY_MAJOR_VERSION >= 3
@@ -2847,7 +2863,7 @@ psutil_proc_info(PyObject *self, PyObject *args) {
                 (double)process->UserTime.LowPart * LO_T;
     kernel_time = (double)process->KernelTime.HighPart * HI_T + \
                     (double)process->KernelTime.LowPart * LO_T;
-    
+
     // Convert the LARGE_INTEGER union to a Unix time.
     // It's the best I could find by googling and borrowing code here
     // and there. The time returned has a precision of 1 second.
@@ -3768,6 +3784,18 @@ void init_psutil_windows(void)
         INITERROR;
     }
 
+    // Exceptions.
+    TimeoutExpired = PyErr_NewException(
+        "_psutil_windows.TimeoutExpired", NULL, NULL);
+    Py_INCREF(TimeoutExpired);
+    PyModule_AddObject(module, "TimeoutExpired", TimeoutExpired);
+
+    TimeoutAbandoned = PyErr_NewException(
+        "_psutil_windows.TimeoutAbandoned", NULL, NULL);
+    Py_INCREF(TimeoutAbandoned);
+    PyModule_AddObject(module, "TimeoutAbandoned", TimeoutAbandoned);
+
+    // version constant
     PyModule_AddIntConstant(module, "version", PSUTIL_VERSION);
 
     // process status constants
diff --git a/psutil/_pswindows.py b/psutil/_pswindows.py
index 0eb4b14f..bb95d2a0 100644
--- a/psutil/_pswindows.py
+++ b/psutil/_pswindows.py
@@ -9,6 +9,7 @@ import errno
 import functools
 import os
 import sys
+import time
 from collections import namedtuple
 
 from . import _common
@@ -78,7 +79,6 @@ __extra__all__ = [
 # =====================================================================
 
 CONN_DELETE_TCB = "DELETE_TCB"
-WAIT_TIMEOUT = 0x00000102  # 258 in decimal
 ACCESS_DENIED_ERRSET = frozenset([errno.EPERM, errno.EACCES,
                                   cext.ERROR_ACCESS_DENIED])
 NO_SUCH_SERVICE_ERRSET = frozenset([cext.ERROR_INVALID_NAME,
@@ -792,18 +792,43 @@ class Process(object):
         if timeout is None:
             cext_timeout = cext.INFINITE
         else:
-            # WaitForSingleObject() expects time in milliseconds
+            # WaitForSingleObject() expects time in milliseconds.
             cext_timeout = int(timeout * 1000)
+
+        timer = getattr(time, 'monotonic', time.time)
+        stop_at = timer() + timeout if timeout is not None else None
+
+        try:
+            # Exit code is supposed to come from GetExitCodeProcess().
+            # May also be None if OpenProcess() failed with
+            # ERROR_INVALID_PARAMETER, meaning PID is already gone.
+            exit_code = cext.proc_wait(self.pid, cext_timeout)
+        except cext.TimeoutExpired:
+            # WaitForSingleObject() returned WAIT_TIMEOUT. Just raise.
+            raise TimeoutExpired(timeout, self.pid, self._name)
+        except cext.TimeoutAbandoned:
+            # WaitForSingleObject() returned WAIT_ABANDONED, see:
+            # https://github.com/giampaolo/psutil/issues/1224
+            # We'll just rely on the internal polling and return None
+            # when the PID disappears. Subprocess module does the same
+            # (return None):
+            # https://github.com/python/cpython/blob/
+            #     be50a7b627d0aa37e08fa8e2d5568891f19903ce/
+            #     Lib/subprocess.py#L1193-L1194
+            exit_code = None
+
+        # At this point WaitForSingleObject() returned WAIT_OBJECT_0,
+        # meaning the process is gone. Stupidly there are cases where
+        # its PID may still stick around so we do a further internal
+        # polling.
+        delay = 0.0001
         while True:
-            ret = cext.proc_wait(self.pid, cext_timeout)
-            if ret == WAIT_TIMEOUT:
-                raise TimeoutExpired(timeout, self.pid, self._name)
-            if pid_exists(self.pid):
-                if timeout is None:
-                    continue
-                else:
-                    raise TimeoutExpired(timeout, self.pid, self._name)
-            return ret
+            if not pid_exists(self.pid):
+                return exit_code
+            if stop_at and timer() >= stop_at:
+                raise TimeoutExpired(timeout, pid=self.pid, name=self._name)
+            time.sleep(delay)
+            delay = min(delay * 2, 0.04)  # incremental delay
 
     @wrap_exceptions
     def username(self):
diff --git a/scripts/internal/winmake.py b/scripts/internal/winmake.py
index 548f7a8e..19c27ed1 100755
--- a/scripts/internal/winmake.py
+++ b/scripts/internal/winmake.py
@@ -28,6 +28,8 @@ PYTHON = os.getenv('PYTHON', sys.executable)
 TSCRIPT = os.getenv('TSCRIPT', 'psutil\\tests\\__main__.py')
 GET_PIP_URL = "https://bootstrap.pypa.io/get-pip.py"
 PY3 = sys.version_info[0] == 3
+HERE = os.path.abspath(os.path.dirname(__file__))
+ROOT_DIR = os.path.realpath(os.path.join(HERE, "..", ".."))
 DEPS = [
     "coverage",
     "flake8",
@@ -442,18 +444,26 @@ def test_memleaks():
 
 @cmd
 def install_git_hooks():
+    """Install GIT pre-commit hook."""
     if os.path.isdir('.git'):
-        shutil.copy(".git-pre-commit", ".git\\hooks\\pre-commit")
+        src = os.path.join(ROOT_DIR, ".git-pre-commit")
+        dst = os.path.realpath(
+            os.path.join(ROOT_DIR, ".git", "hooks", "pre-commit"))
+        with open(src, "rt") as s:
+            with open(dst, "wt") as d:
+                d.write(s.read())
 
 
 @cmd
 def bench_oneshot():
+    """Benchmarks for oneshot() ctx manager (see #799)."""
     install()
     sh("%s -Wa scripts\\internal\\bench_oneshot.py" % PYTHON)
 
 
 @cmd
 def bench_oneshot_2():
+    """Same as above but using perf module (supposed to be more precise)."""
     install()
     sh("%s -Wa scripts\\internal\\bench_oneshot_2.py" % PYTHON)