Skip to content

bpo-29884: faulthandler: Restore the old sigaltstack during teardown #777

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.

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

Merged
merged 1 commit into from
Mar 23, 2017

Conversation

tich
Copy link
Contributor

@tich tich commented Mar 23, 2017

No description provided.

@mention-bot
Copy link

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

Copy link
Member

@vstinner vstinner left a comment

Choose a reason for hiding this comment

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

The fix itself LGTM, but please also:

  • Add yourself to Misc/ACKS
  • Mention the fix in Misc/NEWS, in the Library section

@serhiy-storchaka
Copy link
Member

And tests.

@vstinner
Copy link
Member

And tests.

IMHO it's overkill to test this bugfix. It's a corner case which requires to write a program which embeds Python and uses sigaltstack().

@tich
Copy link
Contributor Author

tich commented Mar 23, 2017

Added myself to ACKS and listed the fix in NEWS. Thanks!

Misc/NEWS Outdated
@@ -775,6 +775,9 @@ Library
- Issue #29581: ABCMeta.__new__ now accepts **kwargs, allowing abstract base
classes to use keyword parameters in __init_subclass__. Patch by Nate Soares.

- Issue #29884: faulthandler: Restore the old sigaltstack during teardown.
Copy link
Member

Choose a reason for hiding this comment

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

Move it to the start of the Library section.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Done

@serhiy-storchaka
Copy link
Member

IMHO it's overkill to test this bugfix. It's a corner case which requires to write a program which embeds Python and uses sigaltstack().

Okay, then tests are not required.

@vstinner vstinner merged commit 20fbf8a into python:master Mar 23, 2017
@vstinner
Copy link
Member

Thanks! I merged your fix. It would be nice to backport the fix Python 3.5 & 3.6 (using git cherry-pick).

tich added a commit to tich/cpython that referenced this pull request Mar 24, 2017
tich added a commit to tich/cpython that referenced this pull request Mar 24, 2017
Mariatta pushed a commit that referenced this pull request Mar 24, 2017
Mariatta pushed a commit that referenced this pull request Mar 24, 2017
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants