[Patches] [ python-Patches-1220212 ] os.kill on windows

[SourceForge.net]

Patches item #1220212, was opened at 2005-06-14 05:11
Message generated for change (Comment added) made by tim_one
You can respond by visiting: 
https://sourceforge.net/tracker/?func=detail&atid=305470&aid=1220212&group_id=5470

Please note that this message will contain a full copy of the comment thread,
including the initial issue submission, for this request,
not just the latest update.
Category: Core (C code)
Group: Python 2.5
Status: Open
Resolution: None
Priority: 5
Submitted By: Miki Tebeka (tebeka)
Assigned to: Nobody/Anonymous (nobody)
Summary: os.kill on windows

Initial Comment:
This patch enables os.kill on windows. This way there
will be an "out of the box" way to kill process on windows.

Basically kill calls TerminateProcess and the "signal"
paramer will be the process return code.

Don't have CVS access today so I'm sending the whole
files in a zip (sorry).

Also some config wizard need to change the winsows
configuration and set HAVE_KILL.

----------------------------------------------------------------------

>Comment By: Tim Peters (tim_one)
Date: 2006-07-09 18:02

Message:
Logged In: YES 
user_id=31435

Well, os.waitpid() on Windows also takes a process handle. 
This was (of course) deliberate, so that os.waitpid() could
use the thingie returned by os.spawn() on Windows.  If
os.kill() were ever added to Windows, I think it would be
quite natural for it to take a process handle too, and for
the same reason (Python code mixing os.{spawn,waitpid,kill}
could be the same across platforms).  Zope's ZODB relies on
mixing spawn and waitpid this way, and Zope's ZRS implements
its own kill function on Windows so that the rest of the
Python code can just do kill(id) on all platforms (where
`id` is always something obtained from os.spawn()).

----------------------------------------------------------------------

Comment By: Martin v. Löwis (loewis)
Date: 2006-07-09 17:30

Message:
Logged In: YES 
user_id=21627

Ah, ok. Given that _subprocess already offers that
functionality, I'm still -1 on adding it to "nt". Adding
some kind of OpenProcess might be more useful, but then,
people can get to all these functions through ctypes, as well.

----------------------------------------------------------------------

Comment By: Tim Peters (tim_one)
Date: 2006-07-09 15:21

Message:
Logged In: YES 
user_id=31435

Martin, FYI, on Windows the os.spawn() functions return the
process handle, so that's the natural use case:  killing a
process spawned by Python's os.spawn*().  This is a little
confusing because the spawn() functions are documented as
returning "the process ID", and verbiage inside the spawn
docs explains that it doesn't really mean "process ID" on
Windows.

----------------------------------------------------------------------

Comment By: Martin v. Löwis (loewis)
Date: 2006-07-09 13:45

Message:
Logged In: YES 
user_id=21627

"Python uses the process handle as process id" makes no real
sense - Win32 works differently. You can't terminate a
process without a handle, and you can't just "use" the
process id as the handle. Perhaps some use cases for the API
would need to be defined first.

----------------------------------------------------------------------

Comment By: Miki Tebeka (tebeka)
Date: 2006-07-09 10:07

Message:
Logged In: YES 
user_id=358087

IIRC (the patch was done a long time ago), Python uses the
process handle as the process id, I've tested the code and
it worked.

I agree that the patch need more work, mainly the error
return value.

At least on Python 2.4.3, TerminateProcess is not exposed by
_subprocss (or subprocess).

I still think that having a way to kill a process OOTB on
win32 is very important, pretty swamped right now but I'll
try to improve the patch.

----------------------------------------------------------------------

Comment By: Martin v. Löwis (loewis)
Date: 2006-07-06 01:05

Message:
Logged In: YES 
user_id=21627

As is, the patch has two major problems:

- TerminateProcess expects a process handle, not a process
ID. This makes the patch pretty useless: To kill a process
other than yourself, you need to call OpenProcess first (or
obtain a process handle in some other way, e.g. by having
created it).

- if the call fails, it uses posix_error to report the
problem. However, posix_error expects that errno is set,
which isn't the case here.

Furthermore, this patch would duplicate
_subprocess.TerminateProcess, which already exposes
TerminateProcess (plus allowing to specify the exit code).

So in its current form, I think the patch should be rejected.

----------------------------------------------------------------------

Comment By: Neal Norwitz (nnorwitz)
Date: 2006-07-06 00:54

Message:
Logged In: YES 
user_id=33168

Karl, can you test this patch and verify it works?  It would
be especially helpful to test on Win9x and WinNT variants. 
Are there tests included?  Is there a doc patch?  These are
some of the potential reasons this isn't included.  As for
me, I can't test this patch (no windows), so it's a
non-starter.  If you'd like to help out, we could use the help.

----------------------------------------------------------------------

Comment By: Karl Pietrzak (kap4020)
Date: 2006-07-05 21:44

Message:
Logged In: YES 
user_id=1537118

So is there a reason this isn't in the latest Python?  The 
patch is tiny. 

----------------------------------------------------------------------

Comment By: Martin v. Löwis (loewis)
Date: 2005-07-11 12:30

Message:
Logged In: YES 
user_id=21627

Converted to unified diff.

----------------------------------------------------------------------

You can respond by visiting: 
https://sourceforge.net/tracker/?func=detail&atid=305470&aid=1220212&group_id=5470