[issue14484] missing return in win32_kill?

[Irit Katriel]

Change by Irit Katriel :


--
versions: +Python 3.10, Python 3.11, Python 3.9 -Python 3.7

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue14484] missing return in win32_kill?

[Steve Dower]

Steve Dower  added the comment:

> Windows should have its own specific function.

Either that or mimic it properly. Having a single function that requires 
different parameters based on OS is a very user-hostile design.

If someone wants to shepherd it through the process, I'd support either 
making the typical POSIX values do sensible equivalents on Windows (as 
best possible), or adding a new function entirely and deprecating all 
use of the current one on Windows. Those are the only ways to lead 
people into writing code that works well on all platforms.

(My *real* preference is that people stop trying to kill applications ;) 
and try to set up a safe communication channel instead, but I know it's 
often unavoidable. Having common parameters that do the Right Things on 
all platforms is next best.)

--

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue14484] missing return in win32_kill?

[STINNER Victor]

STINNER Victor  added the comment:

IMO trying to mimic POSIX behavior on Windows in the single function os.kill() 
was a bad design idea. Windows should have its own specific function.

--

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue14484] missing return in win32_kill?

[Eryk Sun]

Eryk Sun  added the comment:

The details of os.kill() on Windows have been discussed extensively for years 
in various issues such as bpo-26350, bpo-23948, and bpo-42962. But the problem 
of the missing return statement is always overwhelmed by discussion of the 
egregiously bad design of this function and its misuse, which depends on bugs 
in WinAPI GenerateConsoleCtrlEvent() when passed a PID that is not a process 
group ID and/or not attached to the console.

--
versions: +Python 3.7 -Python 3.10, Python 3.11, Python 3.9

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue14484] missing return in win32_kill?

[Eryk Sun]

Change by Eryk Sun :


--
Removed message: https://bugs.python.org/msg408336

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue14484] missing return in win32_kill?

[Eryk Sun]

Eryk Sun  added the comment:

The details of os.kill() on Windows have been discussed extensively for years 
in various issues such as bpo-26350, bp-23948, and bp42962. But the problem of 
the missing return statement is always overwhelmed by discussion of the 
egregiously bad design of this function and its misuse, which depends on bugs 
in WinAPI GenerateConsoleCtrlEvent() when passed a PID that is not a process 
group ID and/or not attached to the console.

--
nosy: +eryksun

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue14484] missing return in win32_kill?

[Irit Katriel]

Irit Katriel  added the comment:

That piece of code is still there, the function is now called os_kill_impl.

--
nosy: +iritkatriel
versions: +Python 3.10, Python 3.11, Python 3.9 -Python 2.7, Python 3.4, Python 
3.5

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue14484] missing return in win32_kill?

[Mark Lawrence]

Change by Mark Lawrence :


--
nosy:  -BreamoreBoy

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue14484] missing return in win32_kill?

[John Ehresman]

John Ehresman added the comment:

I've created issue #23948 for the idea of deprecating os.kill().

Is a patch needed for adding a return in the error case?  It's that way in 2.7 
and I'm struggling to come up with a reason why it shouldn't be added other 
than strict backward compatibility.

--

___
Python tracker rep...@bugs.python.org
http://bugs.python.org/issue14484
___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue14484] missing return in win32_kill?

[John Ehresman]

John Ehresman added the comment:

I think at a minimum, a return should be added in the cases that call 
GenerateConsoleCtrlEvent and it fails.

Here's a more radical proposal, though: deprecate kill() on Windows and add a 
function that calls GenerateConsoleCtrlEvent and another that calls 
TerminateProcess.  The rationale is that the two do act quite a bit differently 
than kill does on non-Windows systems do and it's a bad idea to try to provide 
cross-platform functionality when it can't be done.  kill() on non-Windows 
systems would be left alone.

--
nosy: +jpe

___
Python tracker rep...@bugs.python.org
http://bugs.python.org/issue14484
___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue14484] missing return in win32_kill?

[Zachary Ware]

Zachary Ware added the comment:

That's a fair point.  So to make my plan work, we'd really need a good
way to differentiate CTRL_C_EVENT and 0.  If anybody has any good
ideas on that, I still like my plan.  If not, I'm afraid I have to
agree that os.kill will need to go down as doomed by Windows'
incompatibility with the rest of the world.

--

___
Python tracker rep...@bugs.python.org
http://bugs.python.org/issue14484
___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue14484] missing return in win32_kill?

[Zachary Ware]

Zachary Ware added the comment:

I think what I'd rather do is deprecate the use of os.kill (on Windows) with a 
signal other than CTRL_C_EVENT and CTRL_BREAK_EVENT, document very clearly that 
os.kill is strictly for sending signals, and add os.terminate to implement 
TerminateProcess.

--

___
Python tracker rep...@bugs.python.org
http://bugs.python.org/issue14484
___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue14484] missing return in win32_kill?

[John Ehresman]

John Ehresman added the comment:

A problem with os.kill with CTRL_C_EVENT is that CTRL_C_EVENT == 0 and on unix 
kill w/ a signal number of 0 is how you test to see if a process is running.  
This means that code written on unix to see if a process exists will try to 
send a ctrl-c to the other process; it will fail because 
GenerateConsoleCtrlEvent is so limited but the error message is likely to be 
confusing.

Not using the kill() name also means that developers coming from unix won't 
expect other signal numbers to work.

--

___
Python tracker rep...@bugs.python.org
http://bugs.python.org/issue14484
___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue14484] missing return in win32_kill?

[Mark Lawrence]

Mark Lawrence added the comment:

#14480 os.kill on Windows should accept zero as signal references this.  It 
seems that we either go all the way and change the code as Victor has suggested 
or keep the status quo and change the docs as Zach has said.  Thoughts?

--
nosy: +steve.dower

___
Python tracker rep...@bugs.python.org
http://bugs.python.org/issue14484
___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue14484] missing return in win32_kill?

[STINNER Victor]

STINNER Victor added the comment:

I understand that os.kill(pid, sig) should call TerminateProcess() or 
GenerateConsoleCtrlEvent() depending on the value of sig.

The problem is that these two functions are very different. A process can set a 
control handler for CTRL_C_EVENT and CTRL_BREAK_EVENT, so can decide how to 
handle GenerateConsoleCtrlEvent() event.

TerminateProcess() kills the process with the specified exit code.

To me it looks wrong to call TerminateProcess() with a signal number or event 
for the exit code!? We need to expose TerminateProcess() as a new Python 
function, os.TerminateProcess(pid, exitcode) for example.

os.kill(pid, sig) should raise a ValueError if sig is not CTRL_C_EVENT nor 
CTRL_BREAK_EVENT.

--
nosy: +haypo

___
Python tracker rep...@bugs.python.org
http://bugs.python.org/issue14484
___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue14484] missing return in win32_kill?

[Zachary Ware]

Zachary Ware added the comment:

It looks like we have a bit of a mess here.  2.7 has a return there (and thus 
doesn't fall back to TerminateProcess if GenerateConsoleCtrlEvent fails), added 
40 commits after the initial implementation in b1c00c7d3c85, but 3.x was never 
changed so 2.7 and 3.x have behaved differently from the the time it was 
implemented.  Which version is right, or is it too late to change either one 
and 3.x should just remove the unused error setting?  An interesting 
possibility might be to convert the signal.CTRL_* values to an enum, and use 
that as a way to distinguish between ``0`` and ``signal.CTRL_C_EVENT``.  I 
suspect that might become rather hairy, though.

Either way, I don't think os.kill can promise much more than try to make the 
specified process die on Windows; signals are just so crippled on Windows that 
that's about all you can do with them anyway.  It might not hurt for the docs 
to try to make that clearer.

--

___
Python tracker rep...@bugs.python.org
http://bugs.python.org/issue14484
___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue14484] missing return in win32_kill?

[Mark Lawrence]

Mark Lawrence added the comment:

@Zach can you add anything to this?

--
nosy: +BreamoreBoy, zach.ware
versions: +Python 3.4, Python 3.5 -Python 3.2, Python 3.3

___
Python tracker rep...@bugs.python.org
http://bugs.python.org/issue14484
___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue14484] missing return in win32_kill?

[Brian Curtin]

Changes by Brian Curtin br...@python.org:


--
nosy:  -brian.curtin

___
Python tracker rep...@bugs.python.org
http://bugs.python.org/issue14484
___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue14484] missing return in win32_kill?

[Antoine Pitrou]

New submission from Antoine Pitrou pit...@free.fr:

Here is an excerpt from the os.kill implementation under Windows (in 
win32_kill(), posixmodule.c):

if (sig == CTRL_C_EVENT || sig == CTRL_BREAK_EVENT) {
if (GenerateConsoleCtrlEvent(sig, pid) == 0) {
err = GetLastError();
PyErr_SetFromWindowsErr(err);
}
else
Py_RETURN_NONE;
}

It seems there is a missing return in the first branch, when 
GenerateConsoleCtrlEvent() fails.

--
components: Windows
messages: 157419
nosy: asvetlov, brian.curtin, pitrou, tim.golden
priority: normal
severity: normal
status: open
title: missing return in win32_kill?
type: behavior
versions: Python 2.7, Python 3.2, Python 3.3

___
Python tracker rep...@bugs.python.org
http://bugs.python.org/issue14484
___
___
Python-bugs-list mailing list
Unsubscribe: 
http://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue14484] missing return in win32_kill?

[Andrew Svetlov]

Andrew Svetlov andrew.svet...@gmail.com added the comment:

Antonie, you right.

--

___
Python tracker rep...@bugs.python.org
http://bugs.python.org/issue14484
___
___
Python-bugs-list mailing list
Unsubscribe: 
http://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue14484] missing return in win32_kill?

[Brian Curtin]

Brian Curtin br...@python.org added the comment:

I can't find where we talked about this, maybe just IRC, but that's there 
(perhaps poorly) as a special case.

0 is 0, but signal.CTRL_C_EVENT is also 0. We try the signal version first then 
fall back to TerminateProcess.

I was just looking at this myself and it's not great. I actually wish we could 
change what signal.CTRL_C_EVENT means, or maybe add a flag to determine if the 
second parameter is supposed to be a return code or a signal? I'm open to 
anything.

--

___
Python tracker rep...@bugs.python.org
http://bugs.python.org/issue14484
___
___
Python-bugs-list mailing list
Unsubscribe: 
http://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue14484] missing return in win32_kill?

[Antoine Pitrou]

Antoine Pitrou pit...@free.fr added the comment:

 I can't find where we talked about this, maybe just IRC, but that's
 there (perhaps poorly) as a special case.
 
 0 is 0, but signal.CTRL_C_EVENT is also 0. We try the signal version
 first then fall back to TerminateProcess.

Then why set the error?
That said, abruptly killing another process and making it return 0
sounds a bit perverted.

--

___
Python tracker rep...@bugs.python.org
http://bugs.python.org/issue14484
___
___
Python-bugs-list mailing list
Unsubscribe: 
http://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue14484] missing return in win32_kill?

[Brian Curtin]

Brian Curtin br...@python.org added the comment:

I don't remember exactly why, but it can be removed. It may have just been left 
in while I was debugging it.

As for the second point, why else are you calling os.kill if you don't want to 
kill the given process? I don't disagree that it's on the perverse side, but 
that's the functionality available. Perhaps we should *not* have the fallback 
and only operate on the signals?

--

___
Python tracker rep...@bugs.python.org
http://bugs.python.org/issue14484
___
___
Python-bugs-list mailing list
Unsubscribe: 
http://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue14484] missing return in win32_kill?

[R. David Murray]

R. David Murray rdmur...@bitdance.com added the comment:

I would think that if Windows doesn't support a specific signal, os.kill should 
raise a ValueError.  But I'm an outsider here, I know nothing about how Windows 
works for this except what I'm reading here.  

To answer your question: there are many reasons to call kill on unix, and only 
a few of them kill the process.  Kill is just an historical name, it really 
means 'send a signal'.

In a broader picture, I think that os.kill calls should have the same 
meaning, insofar as possible, on both linux and windows.  Having a single API 
with the same syntax but different semantics on different platforms sounds bad 
to me.

--
nosy: +r.david.murray

___
Python tracker rep...@bugs.python.org
http://bugs.python.org/issue14484
___
___
Python-bugs-list mailing list
Unsubscribe: 
http://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue14484] missing return in win32_kill?

[Antoine Pitrou]

Antoine Pitrou pit...@free.fr added the comment:

 As for the second point, why else are you calling os.kill if you don't
 want to kill the given process? I don't disagree that it's on the
 perverse side, but that's the functionality available. Perhaps we
 should *not* have the fallback and only operate on the signals?

I just meant that exiting with 0 isn't exactly expected when a process
is forcibly killed (0 meaning success, as far as I know).

--

___
Python tracker rep...@bugs.python.org
http://bugs.python.org/issue14484
___
___
Python-bugs-list mailing list
Unsubscribe: 
http://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com