Skip to content
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

Jump to bottom

Set the SA_ONSTACK in PyOS_setsig to play well with other VMs like Golang #87556

Closed
gpshead opened this issue Mar 3, 2021 · 3 comments
Closed

Set the SA_ONSTACK in PyOS_setsig to play well with other VMs like Golang #87556

gpshead opened this issue Mar 3, 2021 · 3 comments
Assignees
@gpshead
Labels
3.10 only security fixes interpreter-core (Objects, Python, Grammar, and Parser dirs) performance Performance or resource usage

Comments

@gpshead
Copy link
Member

gpshead commented Mar 3, 2021

BPO 43390
Nosy @gpshead
PRs
  • bpo-43390: Set SA_ONSTACK in PyOS_setsig #24730
    		•

    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 = 'https://github.com/gpshead'
closed_at = <Date 2021-03-05.05:52:11.771>
created_at = <Date 2021-03-03.17:45:04.733>
labels = ['interpreter-core', '3.10', 'performance']
title = 'Set the SA_ONSTACK in PyOS_setsig to play well with other VMs like Golang'
updated_at = <Date 2021-03-05.05:52:11.770>
user = 'https://github.com/gpshead'

    bugs.python.org fields:

    activity = <Date 2021-03-05.05:52:11.770>
actor = 'gregory.p.smith'
assignee = 'gregory.p.smith'
closed = True
closed_date = <Date 2021-03-05.05:52:11.771>
closer = 'gregory.p.smith'
components = ['Interpreter Core']
creation = <Date 2021-03-03.17:45:04.733>
creator = 'gregory.p.smith'
dependencies = []
files = []
hgrepos = []
issue_num = 43390
keywords = ['patch']
message_count = 3.0
messages = ['388036', '388145', '388146']
nosy_count = 1.0
nosy_names = ['gregory.p.smith']
pr_nums = ['24730']
priority = 'normal'
resolution = 'fixed'
stage = 'commit review'
status = 'closed'
superseder = None
type = 'resource usage'
url = 'https://bugs.python.org/issue43390'
versions = ['Python 3.10']
    @gpshead
    Copy link
    Member Author

    gpshead commented Mar 3, 2021

    PyOS_setsig currently sets the struct sigaction context.sa_flags = 0 before calling sigaction.

    Other virtual machines such as Golang depend on signals using SA_ONSTACK such that signal handlers use a specially allocated stack that runtime sets up for reliability reasons as they use tiny stacks on normal threads.

    SA_ONSTACK is a no-op flag in the typical case where no sigaltstack() call has been made to setup an alternate signal handling stack. (as in 99.99% of all CPython applications)

    When a C/C++ extension module is linked with cgo to call into a Golang library, doing this increases reliability.

    As much as I try to dissuade anyone from creating and relying on hidden complexity multi-VM-hybrids in a single process like this, some people do, and this one line change helps.

    Golang references:
    https://golang.org/pkg/os/signal/#hdr-Go_programs_that_use_cgo_or_SWIG
    and
    https://go-review.googlesource.com/c/go/+/298269/ (which clarifies that SA_RESTART is no longer a requirement. Good. Because Python won't get along well with that one.)

    @gpshead gpshead added the 3.10 only security fixes label Mar 3, 2021
    @gpshead gpshead self-assigned this Mar 3, 2021
    @gpshead gpshead added interpreter-core (Objects, Python, Grammar, and Parser dirs) 3.10 only security fixes labels Mar 3, 2021
    @gpshead gpshead self-assigned this Mar 3, 2021
    @gpshead gpshead added interpreter-core (Objects, Python, Grammar, and Parser dirs) performance Performance or resource usage labels Mar 3, 2021
    @gpshead
    Copy link
    Member Author

    gpshead commented Mar 5, 2021

    New changeset 02ac6f4 by Gregory P. Smith in branch 'master':
    bpo-43390: Set SA_ONSTACK in PyOS_setsig (GH-24730)
    02ac6f4

    @gpshead
    Copy link
    Member Author

    gpshead commented Mar 5, 2021

    I expect zero fallout from this given the semantics. SA_ONSTACK really appears to be something that should've been the POSIX default since it was introduced as a feature in ~BSD4.2 in the early 80s. But it never was.

    It'll be good to have in the beta releases to see if anyone pipes up.

    We'll be running our interpreter inside Google with this flag enabled.

    @gpshead gpshead closed this as completed Mar 5, 2021
    @gpshead gpshead closed this as completed Mar 5, 2021
    @ezio-melotti ezio-melotti transferred this issue from another repository Apr 10, 2022
    Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
    Assignees

    @gpshead gpshead

    Labels
    3.10 only security fixes interpreter-core (Objects, Python, Grammar, and Parser dirs) performance Performance or resource usage
    Projects
    None yet
    Milestone
    No milestone
    Development

    No branches or pull requests
    1 participant
    @gpshead