◐ Shell
reader mode source ↗
Skip to content

bpo-30703: Improve signal delivery#2415

Merged
pitrou merged 10 commits into
python:masterfrom
pitrou:signal_delivery
Jun 28, 2017
Merged

bpo-30703: Improve signal delivery#2415
pitrou merged 10 commits into
python:masterfrom
pitrou:signal_delivery

Conversation

@pitrou

@pitrou pitrou commented Jun 26, 2017

Copy link
Copy Markdown
Member

Avoid using Py_AddPendingCall from signal handler, to avoid calling signal-unsafe functions.

@pitrou
Improve signal delivery …
0a68ec4 
Avoid using Py_AddPendingCall from signal handler, to avoid calling signal-unsafe functions.
@mention-bot

mention-bot commented Jun 26, 2017

Copy link
Copy Markdown

@pitrou, thanks for your PR! By analyzing the history of the files in this pull request, we identified @benjaminp, @serhiy-storchaka and @1st1 to be potential reviewers.

@pitrou pitrou mentioned this pull request Jun 26, 2017
Closed
@pitrou pitrou changed the title Improve signal delivery Jun 26, 2017
@pitrou pitrou mentioned this pull request Jun 26, 2017
Merged
@pitrou

pitrou commented Jun 26, 2017

Copy link
Copy Markdown
Member Author

The test I'm adding here fails without the rest of the patch (on Linux and OS X). This means our signal delivery logic really had holes in it.

@pitrou

pitrou commented Jun 27, 2017

Copy link
Copy Markdown
Member Author

@tim-one, would you like to review this one?

vstinner
vstinner reviewed Jun 28, 2017
View reviewed changes

@vstinner vstinner left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hide comment

Since I wrote the first version of this change, I like the design, haha. More seriously, the overall change LGTM.

But I added a few comments with some questions.

@vstinner

vstinner commented Jun 28, 2017

Copy link
Copy Markdown
Member

About the test, I would even prefer to not have such test. Or maybe only run it a few times (ex: 100 signals, not 10 000).

vstinner
vstinner approved these changes Jun 28, 2017
View reviewed changes

@vstinner vstinner left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hide comment

LGTM. I'm not confortable to backport this change on stable versions right now. If you want to backport it, maybe wait at least one week to see if the test works well on all buildbots.

@pitrou

pitrou commented Jun 28, 2017

Copy link
Copy Markdown
Member Author

Agreed. If it goes well then I may consider a backport to 3.6. I think 2.7 and 3.5 are out of question.

@pitrou pitrou merged commit c08177a into python:master Jun 28, 2017
@pitrou pitrou deleted the signal_delivery branch June 28, 2017 21:29
@pitrou

pitrou commented Jun 28, 2017

Copy link
Copy Markdown
Member Author

It seems some systems may have crappy resolution for setitimer. For example, the FreeBSD man page says:

Time values smaller than the resolution of the system clock are rounded up to this resolution (typically 10 milliseconds).

(https://www.freebsd.org/cgi/man.cgi?query=setitimer&apropos=0&sektion=2&manpath=FreeBSD+10.0-RELEASE&arch=default&format=html)

This might explain the failure here: http://buildbot.python.org/all/builders/AMD64%20FreeBSD%209.x%203.x/builds/275/steps/test/logs/stdio
(note signal 14 is SIGALRM)

@pitrou

pitrou commented Jun 28, 2017

Copy link
Copy Markdown
Member Author

Same for AIX:

If a nonprivileged user attempts to submit a fine granularity timer (that is, a timer request of less than 10 milliseconds), the timer request interval is automatically raised to 10 milliseconds.

(https://www.ibm.com/support/knowledgecenter/en/ssw_aix_72/com.ibm.aix.basetrf1/getinterval.htm)

@vstinner

vstinner commented Jun 28, 2017 via email

Copy link
Copy Markdown
Member

@pitrou

pitrou commented Jun 28, 2017

Copy link
Copy Markdown
Member Author

Please stop being offensive with my test :-)

@vstinner

vstinner commented Jun 28, 2017 via email

Copy link
Copy Markdown
Member

pitrou added a commit that referenced this pull request Jul 1, 2017
@pitrou
[3.6] bpo-30703: Improve signal delivery (GH-2415) (#2527) …
3024c05 
* [3.6] bpo-30703: Improve signal delivery (GH-2415)

* Improve signal delivery

Avoid using Py_AddPendingCall from signal handler, to avoid calling signal-unsafe functions.

* Remove unused function

* Improve comments

* Add stress test

* Adapt for --without-threads

* Add second stress test

* Add NEWS blurb

* Address comments @Haypo.
(cherry picked from commit c08177a)

* bpo-30796: Fix failures in signal delivery stress test (#2488)

* bpo-30796: Fix failures in signal delivery stress test

setitimer() can have a poor minimum resolution on some machines,
this would make the test reach its deadline (and a stray signal
could then kill a subsequent test).

* Make sure to clear the itimer after the test
@ma8ma ma8ma mentioned this pull request Jul 5, 2017
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@vstinner vstinner vstinner approved these changes

Assignees

No one assigned

Labels

type-bug An unexpected behavior, bug, or error

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

4 participants

@pitrou @mention-bot @vstinner @the-knights-who-say-ni