Patchwork [STABLE] win32: spawndetached returns pid of detached process and not of cmd.exe

login
register
mail settings
Submitter Simon Heimberg
Date Feb. 15, 2014, 2:20 p.m.
Message ID <3a6bd40c957d95699b49.1392474041@lapsi.heimberg.home>
Download mbox | patch
Permalink /patch/3673/
State Accepted
Headers show

Comments

Simon Heimberg - Feb. 15, 2014, 2:20 p.m.
# HG changeset patch
# User Simon Heimberg <simohe@besonet.ch>
# Date 1391866507 -3600
# Branch stable
# Node ID 3a6bd40c957d95699b493f99b184e4cebd58377d
# Parent  4e41b2fe46ccfb9722e51e23902f7019d286b15d
win32: spawndetached returns pid of detached process and not of cmd.exe

win32.spawndetached starts the detached process by `cmd.exe` (or COMSPEC). The
pid it returned was the one of cmd.exe and not the one of the detached process.
When this pid is used to kill the process, the detached process is not killed,
but only cmd.exe.

With this patch the pid of the detached process is written to the pid file.
Killing the process works as expected.

The pid is only evaluated on writing the pid file. It is unnecessary to search
the pid when it is not needed. And more important, it probably does not yet
exist right after the cmd.exe process was started. When the pid is written to
the file, waiting for the start of the detached process has already happened.
Use this functionality instead of writing a 2nd wait function.

Many tests on windows will not fail anymore, all those with the first failing
line "abort: child process failed to start". (The processes still hanging
around from previous test runs have to be killed first. They still block a
tcp port.)
A good test for the functionality of this patch is test-treediscovery.t,
because it starts and kills `hg serve -d` several times.

(grafted from ca6aa8362f33dddfe74efb03107b99f5947e21f6, for also tests from
"stable" running nice on windows.)

Patch

diff -r 4e41b2fe46cc -r 3a6bd40c957d mercurial/win32.py
--- a/mercurial/win32.py	Don Feb 13 13:05:09 2014 +0100
+++ b/mercurial/win32.py	Sam Feb 08 14:35:07 2014 +0100
@@ -119,6 +119,27 @@ 
 
 _STD_ERROR_HANDLE = _DWORD(-12).value
 
+# CreateToolhelp32Snapshot, Process32First, Process32Next
+_TH32CS_SNAPPROCESS = 0x00000002
+_MAX_PATH = 260
+
+class _tagPROCESSENTRY32(ctypes.Structure):
+    _fields_ = [('dwsize', _DWORD),
+                ('cntUsage', _DWORD),
+                ('th32ProcessID', _DWORD),
+                ('th32DefaultHeapID', ctypes.c_void_p),
+                ('th32ModuleID', _DWORD),
+                ('cntThreads', _DWORD),
+                ('th32ParentProcessID', _DWORD),
+                ('pcPriClassBase', _LONG),
+                ('dwFlags', _DWORD),
+                ('szExeFile', ctypes.c_char * _MAX_PATH)]
+
+    def __init__(self):
+        super(_tagPROCESSENTRY32, self).__init__()
+        self.dwsize = ctypes.sizeof(self)
+
+
 # types of parameters of C functions used (required by pypy)
 
 _kernel32.CreateFileA.argtypes = [_LPCSTR, _DWORD, _DWORD, ctypes.c_void_p,
@@ -186,6 +207,15 @@ 
 _user32.EnumWindows.argtypes = [_WNDENUMPROC, _LPARAM]
 _user32.EnumWindows.restype = _BOOL
 
+_kernel32.CreateToolhelp32Snapshot.argtypes = [_DWORD, _DWORD]
+_kernel32.CreateToolhelp32Snapshot.restype = _BOOL
+
+_kernel32.Process32First.argtypes = [_HANDLE, ctypes.c_void_p]
+_kernel32.Process32First.restype = _BOOL
+
+_kernel32.Process32Next.argtypes = [_HANDLE, ctypes.c_void_p]
+_kernel32.Process32Next.restype = _BOOL
+
 def _raiseoserror(name):
     err = ctypes.WinError()
     raise OSError(err.errno, '%s: %s' % (name, err.strerror))
@@ -309,6 +339,51 @@ 
     width = csbi.srWindow.Right - csbi.srWindow.Left
     return width
 
+def _1stchild(pid):
+    '''return the 1st found child of the given pid
+
+    None is returned when no child is found'''
+    pe = _tagPROCESSENTRY32()
+
+    # create handle to list all processes
+    ph = _kernel32.CreateToolhelp32Snapshot(_TH32CS_SNAPPROCESS, 0)
+    if ph == _INVALID_HANDLE_VALUE:
+        raise ctypes.WinError
+    try:
+        r = _kernel32.Process32First(ph, ctypes.byref(pe))
+        # loop over all processes
+        while r:
+            if pe.th32ParentProcessID == pid:
+                # return first child found
+                return pe.th32ProcessID
+            r = _kernel32.Process32Next(ph, ctypes.byref(pe))
+    finally:
+        _kernel32.CloseHandle(ph)
+    if _kernel32.GetLastError() != _ERROR_NO_MORE_FILES:
+        raise ctypes.WinError
+    return None # no child found
+
+class _tochildpid(int): # pid is _DWORD, which always matches in an int
+    '''helper for spawndetached, returns the child pid on conversion to string
+
+    Does not resolve the child pid immediately because the child may not yet be
+    started.
+    '''
+    def childpid(self):
+        '''returns the child pid of the first found child of the process
+        with this pid'''
+        return _1stchild(self)
+    def __str__(self):
+        # run when the pid is written to the file
+        ppid = self.childpid()
+        if ppid is None:
+            # race, child has exited since check
+            # fall back to this pid. Its process will also have disappeared,
+            # raising the same error type later as when the child pid would
+            # be returned.
+            return " %d" % self
+        return str(ppid)
+
 def spawndetached(args):
     # No standard library function really spawns a fully detached
     # process under win32 because they allocate pipes or other objects
@@ -339,7 +414,8 @@ 
     if not res:
         raise ctypes.WinError
 
-    return pi.dwProcessId
+    # _tochildpid because the process is the child of COMSPEC
+    return _tochildpid(pi.dwProcessId)
 
 def unlink(f):
     '''try to implement POSIX' unlink semantics on Windows'''