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

subprocess: child_exec() uses _Py_set_inheritable() which is not async-signal-safe #76958

Closed
izbyshev mannequin opened this issue Feb 6, 2018 · 6 comments
Closed

subprocess: child_exec() uses _Py_set_inheritable() which is not async-signal-safe #76958

izbyshev mannequin opened this issue Feb 6, 2018 · 6 comments
Assignees
Labels
3.7 (EOL) end of life extension-modules C modules in the Modules dir stdlib Python modules in the Lib dir type-bug An unexpected behavior, bug, or error

Comments

@izbyshev
Copy link
Mannequin

izbyshev mannequin commented Feb 6, 2018

BPO 32777
Nosy @gpshead, @vstinner, @izbyshev
PRs
  • bpo-32777: subprocess: Fix usage of _Py_set_inheritable() in child_exec() #5560
  • [3.7] bpo-32777: Fix _Py_set_inheritable async-safety in subprocess (GH-5560) #5562
  • [3.6] bpo-32777: Fix _Py_set_inheritable async-safety in subprocess (GH-5560) #5563
  • 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 2018-02-06.06:52:35.951>
    created_at = <Date 2018-02-06.01:34:02.804>
    labels = ['extension-modules', 'type-bug', 'library', '3.7']
    title = 'subprocess: child_exec() uses _Py_set_inheritable() which is not async-signal-safe'
    updated_at = <Date 2018-02-06.07:25:34.347>
    user = 'https://github.com/izbyshev'

    bugs.python.org fields:

    activity = <Date 2018-02-06.07:25:34.347>
    actor = 'izbyshev'
    assignee = 'gregory.p.smith'
    closed = True
    closed_date = <Date 2018-02-06.06:52:35.951>
    closer = 'gregory.p.smith'
    components = ['Extension Modules', 'Library (Lib)']
    creation = <Date 2018-02-06.01:34:02.804>
    creator = 'izbyshev'
    dependencies = []
    files = []
    hgrepos = []
    issue_num = 32777
    keywords = ['patch']
    message_count = 6.0
    messages = ['311699', '311706', '311707', '311708', '311709', '311712']
    nosy_count = 3.0
    nosy_names = ['gregory.p.smith', 'vstinner', 'izbyshev']
    pr_nums = ['5560', '5562', '5563']
    priority = 'normal'
    resolution = 'fixed'
    stage = 'commit review'
    status = 'closed'
    superseder = None
    type = 'behavior'
    url = 'https://bugs.python.org/issue32777'
    versions = ['Python 3.6', 'Python 3.7']

    @izbyshev
    Copy link
    Mannequin Author

    izbyshev mannequin commented Feb 6, 2018

    _Py_set_inheritable() raises a Python-level exception on error and thus is not async-signal-safe, but child_exec() must use only async-signal-safe functions because it's executed between fork() and exec().

    Since a non-raising version is already implemented in Python/fileutils.c for internal use (set_inheritable), I suggest to simply expose it via another public function (similar to _Py_open_noraise(), etc.).

    @izbyshev izbyshev mannequin added 3.7 (EOL) end of life stdlib Python modules in the Lib dir type-bug An unexpected behavior, bug, or error labels Feb 6, 2018
    @gpshead gpshead self-assigned this Feb 6, 2018
    @gpshead
    Copy link
    Member

    gpshead commented Feb 6, 2018

    out of curiosity, did you actually diagnose a process deadlocked due to this or was it noted via code inspection?

    this issue has been around since 3.4 and the file descriptor inheritance changes from the looks of things.

    @gpshead
    Copy link
    Member

    gpshead commented Feb 6, 2018

    New changeset c1e46e9 by Gregory P. Smith (Alexey Izbyshev) in branch 'master':
    bpo-32777: Fix _Py_set_inheritable async-safety in subprocess (GH-5560)
    c1e46e9

    @gpshead
    Copy link
    Member

    gpshead commented Feb 6, 2018

    New changeset 2bb0bfa by Gregory P. Smith (Miss Islington (bot)) in branch '3.7':
    bpo-32777: Fix _Py_set_inheritable async-safety in subprocess (GH-5560) (GH-5562)
    2bb0bfa

    @gpshead
    Copy link
    Member

    gpshead commented Feb 6, 2018

    New changeset b90c685 by Gregory P. Smith (Miss Islington (bot)) in branch '3.6':
    bpo-32777: Fix _Py_set_inheritable async-safety in subprocess (GH-5560) (GH-5563)
    b90c685

    @gpshead gpshead added the extension-modules C modules in the Modules dir label Feb 6, 2018
    @gpshead gpshead closed this as completed Feb 6, 2018
    @izbyshev
    Copy link
    Mannequin Author

    izbyshev mannequin commented Feb 6, 2018

    out of curiosity, did you actually diagnose a process deadlocked due to this or was it noted via code inspection?

    The latter. I noted it while working on another issue. While it was easy to trigger a malloc() in child_exec() by e.g. specifying an invalid fd in pass_fds for Popen, I haven't tried to arrange a deadlock.

    @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
    Labels
    3.7 (EOL) end of life extension-modules C modules in the Modules dir stdlib Python modules in the Lib dir type-bug An unexpected behavior, bug, or error
    Projects
    None yet
    Development

    No branches or pull requests

    1 participant