[PATCH 1 of 3] killdaemons: fix error handling on Windows

[Matt Harbison]

# HG changeset patch
# User Matt Harbison <matt_harbison at yahoo.com>
# Date 1494779534 14400
#      Sun May 14 12:32:14 2017 -0400
# Node ID 024271e90987e5794dab6eb00844467065fae7e4
# Parent  db0b470547d5e21f042390a204f09ae7d7303757
killdaemons: fix error handling on Windows

After taking Adrian's suggestion[1] to use util.posixfile to avoid os.unlink()
errors, the error changed from:

  Errored test-hgweb-json.t: Traceback (most recent call last):
    File "./run-tests.py", line 724, in run
      self.tearDown()
    File "./run-tests.py", line 805, in tearDown
      killdaemons(entry)
    File "./run-tests.py", line 540, in killdaemons
      logfn=vlog)
    File "...\tests\killdaemons.py", line 94, in killdaemons
      os.unlink(pidfile)
  WindowsError: [Error 32] The process cannot access the file because it is
     being used by another process: '...\\hgtests.zmpqj3\\child80\\daemon.pids'

to:

  Errored test-largefiles.t: Traceback (most recent call last):
    File "./run-tests.py", line 724, in run
      self.tearDown()
    File "./run-tests.py", line 805, in tearDown
      killdaemons(entry)
    File "./run-tests.py", line 540, in killdaemons
      logfn=vlog)
    File "...\tests\killdaemons.py", line 91, in killdaemons
      kill(pid, logfn, tryhard)
    File "...\tests\killdaemons.py", line 55, in kill
      _check(ctypes.windll.kernel32.CloseHandle(handle))
    File "...\tests\killdaemons.py", line 18, in _check
      raise ctypes.WinError(winerrno)
  WindowsError: [Error 6] The handle is invalid.

The handle in question is for the process, not the file.  That made me wonder
why WaitForSingleObject() didn't raise an error, and it's because it isn't a
BOOL return.  (For this method, a return of 0 is WAIT_OBJECT_0, and an error is
signalled by returning (DWORD) 0xFFFFFFFF.)  When I printed out that value (as
returned by the function) with '0x%x', it printed '0x-1'.  There are some bugs
around DWORDs in this range[2], and since the code explictly tests for errors it
wants to handle, it seems safer to test for 'ret != TRUE', to avoid silent
breakage in the future if this bug is fixed.  The only case not handled is
WAIT_ABANDONED, which will now raise.  ctypes.windll.kernel32.GetLastError has a
'c_long' in its restype field, instead of 'c_ulong' for some reason.

With that fixed, WaitForSingleObject() started raising an error with the same
message, meaning OpenProcess() was returning an invalid handle.  Since HANDLE is
a pointer, the proper check is for None, not 0.  With this fixed, I confirmed
with a print statement that the err 87 case now properly bails out.

[1] https://www.mercurial-scm.org/pipermail/mercurial-devel/2017-May/097600.html
[2] https://bugs.python.org/issue28474

diff --git a/tests/killdaemons.py b/tests/killdaemons.py
--- a/tests/killdaemons.py
+++ b/tests/killdaemons.py
@@ -11,7 +11,10 @@
     import ctypes
 
     def _check(ret, expectederr=None):
-        if ret == 0:
+        # 0 is typically a failure, but NOT for WaitForSingleObject().  It is
+        # expected that the cases which can be handled for WaitForSingleObject()
+        # will be before calling this.
+        if ret != 1:
             winerrno = ctypes.GetLastError()
             if winerrno == expectederr:
                 return True
@@ -27,7 +30,7 @@
         handle = ctypes.windll.kernel32.OpenProcess(
                 PROCESS_TERMINATE|SYNCHRONIZE|PROCESS_QUERY_INFORMATION,
                 False, pid)
-        if handle == 0:
+        if handle is None:
             _check(0, 87) # err 87 when process not found
             return # process not found, already finished
         try: