Patches item #1220212, was opened at 2005-06-14 11:11 Message generated for change (Comment added) made by loewis 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: Martin v. Löwis (loewis) Date: 2006-07-09 23: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 21: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 19: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 16: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 07: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 06: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-06 03: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 18: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 _______________________________________________ Patches mailing list [email protected] http://mail.python.org/mailman/listinfo/patches
