[PATCH 1 of 2] run-tests: implement proper process killing for Windows

[Adrian Buehlmann]

On 2012-06-18 14:48, Mads Kiilerich wrote:
> On 18/06/12 02:39, Adrian Buehlmann wrote:
>> On 2012-06-18 02:20, Mads Kiilerich wrote:
>>> Adrian Buehlmann wrote, On 06/18/2012 12:35 AM:
>>>> # HG changeset patch
>>>> # User Adrian Buehlmann <adrian at cadifra.com>
>>>> # Date 1339972395 -7200
>>>> # Node ID 954a063d12edfe1d2a3d32df1a540c0ee5268156
>>>> # Parent  191f0c6cc47c1cf85372f5e5943ca94aa14fcb65
>>>> run-tests: implement proper process killing for Windows
>>> This is allegedly a hard problem. It is great to have it solved!
>> Not at all. I just tried to avoid looking into it so far. It's not
>> really that much fun to have to jump through all these hoops here.
>>
>>>> diff --git a/tests/run-tests.py b/tests/run-tests.py
>>>> --- a/tests/run-tests.py
>>>> +++ b/tests/run-tests.py
>>>> @@ -347,6 +347,25 @@
>>>>        except OSError:
>>>>            pass
>>>>    
>>>> +if os.name == 'nt':
>>>> +    import ctypes
>>>> +    def killprocess(pid):
>>>> +        vlog('# Killing daemon process %d' % pid)
>>>> +        PROCESS_TERMINATE = 1
>>>> +        k = ctypes.windll.kernel32
>>>> +        handle = k.OpenProcess(PROCESS_TERMINATE, False, pid)
>>>> +        k.TerminateProcess(handle, -1)
>>>> +        k.CloseHandle(handle)
>>> How is this different from os.kill ... or msys kill? Why can't os.kill
>>> be used? I assume you have done some research and found something that
>>> would be helpful to have in the commit message for the next guy looking
>>> at this.
>> The difference is that os.kill is claimed to be implemented in Python
>> 2.7, but I found it to be defunct. I haven't investigated why and in
>> what ways exactly.
>>
>> I then copied the snippet above from a stackoverflow discussion and
>> pasted it into run-tests like this. They said, it also works for Python
>> 2.5, where os.kill isn't implemented (which makes sense). Note that
>> ctypes first appeared in the Python standard lib with Python 2.5. And
>> mercurial already depends on it (see win32.py).
>>
>> And I found it works pretty well, reliably kills the respective python
>> process (verified on Windows 7). I also haven't investigated why.
> 
> Right; it is a killer argument that we need the ctypes implementation to 
> support Python 2.6. But it is strange that 2.7 os.kill doesn't work.
> 
> Some tests use the shell 'kill' command, but IIRC it didn't work. (It 
> would be nice to know why.) The tests cold perhaps use killdaemons.py 
> instead, which suggests that the common kill implementation should live 
> there instead.
> 
> (BTW: I get a vague recollection that the problems I have seen might 
> have been related to killing whole process trees.)
> 
> In the bigger perspective we have 'hg serve --daemon' which works on 
> windows. On unix the daemon can easily be killed with the ubiquitous 
> kill command, but there is no convenient way to stop a serve daemon on 
> windows. That could be considered a bug that perhaps could be solved by 
> introducing something like 'hg serve --kill 117 118 119' or 'hg serve 
> --kill --pid-file daemon.pids'. It could thus make sense to let the code 
> live in 'oskill' in win32.py / util.py from the beginning.

In other words: you don't want my patch?