classification
Title: Expose the C raise() function in the signal module, for use on Windows
Type: enhancement Stage: resolved
Components: Library (Lib) Versions: Python 3.8
process
Status: closed Resolution: fixed
Dependencies: Superseder:
Assigned To: Nosy List: miss-islington, njs, paul.moore, steve.dower, tim.golden, v2m, zach.ware
Priority: normal Keywords: easy (C), patch, patch, patch

Created on 2018-12-23 03:55 by njs, last changed 2019-01-08 09:59 by asvetlov. This issue is now closed.

Pull Requests
URL Status Linked Edit
PR 11335 merged v2m, 2018-12-28 03:38
PR 11335 merged v2m, 2018-12-28 03:38
PR 11335 merged v2m, 2018-12-28 03:38
Messages (7)
msg332382 - (view) Author: Nathaniel Smith (njs) * (Python committer) Date: 2018-12-23 03:55
Suppose we want to test how a program responds to control-C. We'll want to write a test that delivers a SIGINT to our process at a time of our choosing, and then checks what happens. For example, asyncio and Trio both have tests like this, and Trio even does a similar thing at run-time to avoid dropping signals in an edge case [1].

So, how can we deliver a signal to our process?

On POSIX platforms, you can use `os.kill(os.getpid(), signal.SIGINT)`, and that works fine. But on Windows, things are much more complicated: https://github.com/python/cpython/pull/11274#issuecomment-449543725

The simplest solution is to use the raise() function. On POSIX, raise(signum) is just a shorthand for kill(getpid(), signum). But, that's only on POSIX. On Windows, kill() doesn't even exist... but raise() does. In fact raise() is specified in C89, so *every* C runtime has to provide raise(), no matter what OS it runs on.

So, you might think, that's ok, if we need to generate synthetic signals on Windows then we'll just use ctypes/cffi to access raise(). *But*, Windows has multiple C runtime libraries (for example: regular and debug), and you have to load raise() from the same library that Python is linked against. And I don't know of any way for a Python script to figure out which runtime it's linked against. (I know how to detect whether the interpreter is configured in debug mode, but that's not necessarily the same as being linked against the debug CRT.) So on the one platform where you really need to use raise(), there's AFAICT no reliable way to get access to it.

This would all be much simpler if the signal module wrapped the raise() function, so that we could just do 'signal.raise_(signal.SIGINT)'. We should do that.

-------

[1] Specifically, consider the following case (I'll use asyncio terminology for simplicity): (1) the user calls loop.add_signal_handler(...) to register a custom signal handler. (2) a signal arrives, and is written to the wakeup pipe. (3) but, before the loop reads the wakeup pipe, the user calls loop.remove_signal_handler(...) to remove the custom handler and restore the original signal settings. (4) now the loop reads the wakeup pipe, and discovers that a signal has arrived, that it no longer knows how to handle. Now what? In this case trio uses raise() to redeliver the signal, so that the new signal handler has a chance to run.
msg332383 - (view) Author: Steve Dower (steve.dower) * (Python committer) Date: 2018-12-23 05:19
Sounds fine to me.

Any particular reason to put it in signal rather than os/posixmodule? If it's just to avoid treating os/posixmodule like a dumping ground for C89/POSIX APIs... well... too late :) I say keep dumping them there. But I don't have a strong opinion about this, and would approve a PR to either location.
msg332384 - (view) Author: Nathaniel Smith (njs) * (Python committer) Date: 2018-12-23 05:47
It probably doesn't matter too much either way, but almost all the signal-related wrappers are in signal. os.kill is the only exception AFAIK.
msg332385 - (view) Author: Nathaniel Smith (njs) * (Python committer) Date: 2018-12-23 07:12
Vladimir Matveev pointed out that there's already a wrapper in _testcapimodule.c: https://github.com/python/cpython/blob/f06fba5965b4265c42291d10454f387b76111f26/Modules/_testcapimodule.c#L3862-L3879

So if we do add this to signalmodule.c, we can drop that.
msg332468 - (view) Author: Steve Dower (steve.dower) * (Python committer) Date: 2018-12-24 15:38
That implementation will require some more of our usual macros around the call itself (for GIL and invalid values), and maybe clinicisation, as well as more through tests, which are probably going to be the hardest part.

Nathaniel - were you planning to do this? Or should we put the "easy (C)" keyword on it and wait for some sprints?
msg332577 - (view) Author: Nathaniel Smith (njs) * (Python committer) Date: 2018-12-27 07:41
I'd like to see it in 3.8, but don't know if I'll get to it, and it'd be a good onramp issue for someone who wants to get into cpython development. So, let's put the keyword on it for now, and see what happens...
msg333216 - (view) Author: miss-islington (miss-islington) Date: 2019-01-08 09:58
New changeset c24c6c2c9357da99961bf257078240529181daf3 by Miss Islington (bot) (Vladimir Matveev) in branch 'master':
bpo-35568: add 'raise_signal' function (GH-11335)
https://github.com/python/cpython/commit/c24c6c2c9357da99961bf257078240529181daf3
History
Date User Action Args
2019-01-08 09:59:37asvetlovsetkeywords: patch, patch, patch, easy (C)
status: open -> closed
resolution: fixed
components: + Library (Lib)
stage: patch review -> resolved
2019-01-08 09:58:30miss-islingtonsetnosy: + miss-islington
messages: + msg333216
2018-12-28 03:38:39v2msetkeywords: + patch
stage: patch review
pull_requests: + pull_request10608
2018-12-28 03:38:25v2msetkeywords: + patch
stage: (no value)
pull_requests: + pull_request10607
2018-12-28 03:38:09v2msetkeywords: + patch
stage: (no value)
pull_requests: + pull_request10606
2018-12-27 07:41:14njssetkeywords: + easy (C)

messages: + msg332577
2018-12-24 15:38:04steve.dowersetmessages: + msg332468
2018-12-24 06:38:47v2msetnosy: + v2m
2018-12-23 07:12:30njssetmessages: + msg332385
2018-12-23 05:47:47njssetmessages: + msg332384
2018-12-23 05:19:06steve.dowersetmessages: + msg332383
2018-12-23 03:55:56njscreate