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

[Mads Kiilerich]

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.

/Mads