Skip to content

os.kill on Windows should accept zero as signal #58685

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom
Closed
asvetlov opened this issue Apr 3, 2012 · 6 comments
Closed

os.kill on Windows should accept zero as signal #58685

asvetlov opened this issue Apr 3, 2012 · 6 comments
Labels
easy OS-windows stdlib Python modules in the Lib dir type-feature A feature request or enhancement

Comments

@asvetlov
Copy link
Contributor

asvetlov commented Apr 3, 2012

BPO 14480
Nosy @vstinner, @giampaolo, @bitdancer, @briancurtin, @asvetlov

Note: these values reflect the state of the issue at the time it was migrated and might not reflect the current state.

Show more details

GitHub fields:

assignee = None
closed_at = <Date 2015-03-20.12:26:30.145>
created_at = <Date 2012-04-03.07:23:13.829>
labels = ['easy', 'type-feature', 'library', 'OS-windows']
title = 'os.kill on Windows should accept zero as signal'
updated_at = <Date 2015-03-20.12:26:30.144>
user = 'https://github.com/asvetlov'

bugs.python.org fields:

activity = <Date 2015-03-20.12:26:30.144>
actor = 'vstinner'
assignee = 'none'
closed = True
closed_date = <Date 2015-03-20.12:26:30.145>
closer = 'vstinner'
components = ['Library (Lib)', 'Windows']
creation = <Date 2012-04-03.07:23:13.829>
creator = 'asvetlov'
dependencies = []
files = []
hgrepos = []
issue_num = 14480
keywords = ['easy']
message_count = 6.0
messages = ['157398', '157418', '157425', '157427', '157430', '238654']
nosy_count = 5.0
nosy_names = ['vstinner', 'giampaolo.rodola', 'r.david.murray', 'brian.curtin', 'asvetlov']
pr_nums = []
priority = 'normal'
resolution = 'rejected'
stage = 'needs patch'
status = 'closed'
superseder = None
type = 'enhancement'
url = 'https://bugs.python.org/issue14480'
versions = ['Python 3.3']
@asvetlov
Copy link
Contributor Author

asvetlov commented Apr 3, 2012

Starting from 3.2 Python supports os.kill for Windows.
It process signal.CTRL_C_EVENT and signal.CTRL_BREAK_EVENT, and kills pid for all other signals.

Posix allows to pass zero signal to check pid for existing.
It will be nice to keep that behavior for Windows also.
The patch should be trivial: just don't call TerminateProcess in Modules/posixmodule.c:win32_kill if sig is zero.

@asvetlov asvetlov added type-bug An unexpected behavior, bug, or error stdlib Python modules in the Lib dir OS-windows easy labels Apr 3, 2012
@bitdancer bitdancer added type-feature A feature request or enhancement and removed type-bug An unexpected behavior, bug, or error labels Apr 3, 2012
@briancurtin
Copy link
Member

-1

0 has no special meaning on Windows so I'd rather not add another special case for posix emulation. Additionally, 0 unfortunately already means two things as it is: signal.CTRL_C_EVENT and the int 0.

@bitdancer
Copy link
Member

Hmm. I would think it would be a good idea to have os.kill do posix emulation where that makes sense, it makes cross-platform usage easier. That's what 'kill' with no signal does, right (kills the process, just like the posix default)?

The posix spec says "If sig is 0 (the null signal), error checking is performed but no signal is actually sent. The null signal can be used to check the validity of pid." So *signal* zero doesn't have any special meaning on posix, either, but it does have a special meaning to the kill command.

It seems like it would be nice to add support for that to the windows version, but I'm a little confused. First you say that signal 0 has no special meaning, and then you say that it does (signal.CTRL_C_EVENT). Which is it?

@briancurtin
Copy link
Member

I meant that in the underlying, such as in the TerminateProcess API, 0 doesn't mean anything special. As is being debated over on bpo-14484 we currently take all integers to be passed to TerminateProcess (the int becomes the killed proc's return code), and two signals to pass to GenerateConsoleCtrlEvent (which is more like posix os.kill).

@asvetlov
Copy link
Contributor Author

asvetlov commented Apr 3, 2012

There are no kill function in Windows API.
From my perspective win32_kill was added to emulate posix sibling if possible. If not — better to give another Windows native name to that function.
Really don't see good solution. Maybe better what we can do — declare what os.kill cannot be called with 0 signum on Windows for proc presence checking and note that fact in os.rst.
Terminating process looks like overcomplication. POSIX kill only sends signal to process and nothing more.

@vstinner
Copy link
Member

0 has no special meaning on Windows so I'd rather not add another special case for posix emulation. Additionally, 0 unfortunately already means two things as it is: signal.CTRL_C_EVENT and the int 0.

I agree, it's not a good idea to support os.kill(pid, 0) on Windows. I reject the issue.

See also the issue bpo-14484.

@ezio-melotti ezio-melotti transferred this issue from another repository Apr 10, 2022
This was referenced Nov 21, 2022
Closed
Merged
@davidLif davidLif mentioned this issue Dec 4, 2022
Merged
@mattip mattip mentioned this issue Dec 9, 2022
Closed
7 tasks
rkooo567 pushed a commit to ray-project/ray that referenced this issue Jan 18, 2023
@mattip
Windows and LogMonitor (#30643)
93288fe 
The LogMonitor did not work correctly on windows.

It had two problems that this PR solves:

the code to add nice worker_pid names depended on posix path separators. This also caused the tests to fail on windows so they were skipped. Fixing this problem allowed enabling the tests
the way to check "liveness" of a PID used os.kill which would kill the process on windows, see os.kill on Windows should accept zero as signal python/cpython#58685 and missing return in win32_kill? python/cpython#58689. Since psutil is already installed, use it instead
In addition this PR has cleanups:

Avoid a warning when compiling regular expressions
Check for exceptions when using gputil, which raises on windows if drivers are installed but no graphics card is found (for instance, if the user disabled/removed the GPU).
andreapiso pushed a commit to andreapiso/ray that referenced this issue Jan 22, 2023
@mattip @andreapiso
Windows and LogMonitor (ray-project#30643)
b97912f 
The LogMonitor did not work correctly on windows.

It had two problems that this PR solves:

the code to add nice worker_pid names depended on posix path separators. This also caused the tests to fail on windows so they were skipped. Fixing this problem allowed enabling the tests
the way to check "liveness" of a PID used os.kill which would kill the process on windows, see os.kill on Windows should accept zero as signal python/cpython#58685 and missing return in win32_kill? python/cpython#58689. Since psutil is already installed, use it instead
In addition this PR has cleanups:

Avoid a warning when compiling regular expressions
Check for exceptions when using gputil, which raises on windows if drivers are installed but no graphics card is found (for instance, if the user disabled/removed the GPU).

Signed-off-by: Andrea Pisoni <andreapiso@gmail.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
easy OS-windows stdlib Python modules in the Lib dir type-feature A feature request or enhancement
Projects
None yet
Milestone
No milestone
Development

No branches or pull requests
4 participants
@vstinner @asvetlov @briancurtin @bitdancer