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

Disallow fork in a subinterpreter. #78832

Closed
ericsnowcurrently opened this issue Sep 12, 2018 · 12 comments
Closed

Disallow fork in a subinterpreter. #78832

ericsnowcurrently opened this issue Sep 12, 2018 · 12 comments
Assignees
Labels
3.8 only security fixes interpreter-core (Objects, Python, Grammar, and Parser dirs)

Comments

@ericsnowcurrently
Copy link
Member

BPO 34651
Nosy @gpshead, @vstinner, @ericsnowcurrently, @hroncok, @miss-islington
PRs
  • bpo-34651: Only allow the main interpreter to fork. #9279
  • bpo-38778: Document that os.fork is not allowed in subinterpreters #17123
  • [3.8] bpo-38778: Document that os.fork is not allowed in subinterpreters (GH-17123) #17179
  • 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/ericsnowcurrently'
    closed_at = <Date 2019-08-29.16:38:15.524>
    created_at = <Date 2018-09-12.21:04:23.258>
    labels = ['interpreter-core', '3.8']
    title = 'Disallow fork in a subinterpreter.'
    updated_at = <Date 2019-11-15.21:37:30.285>
    user = 'https://github.com/ericsnowcurrently'

    bugs.python.org fields:

    activity = <Date 2019-11-15.21:37:30.285>
    actor = 'miss-islington'
    assignee = 'eric.snow'
    closed = True
    closed_date = <Date 2019-08-29.16:38:15.524>
    closer = 'vstinner'
    components = ['Interpreter Core']
    creation = <Date 2018-09-12.21:04:23.258>
    creator = 'eric.snow'
    dependencies = []
    files = []
    hgrepos = []
    issue_num = 34651
    keywords = ['patch']
    message_count = 12.0
    messages = ['325181', '325302', '325400', '343364', '343369', '343415', '343421', '345599', '345603', '350652', '356694', '356720']
    nosy_count = 5.0
    nosy_names = ['gregory.p.smith', 'vstinner', 'eric.snow', 'hroncok', 'miss-islington']
    pr_nums = ['9279', '17123', '17179']
    priority = 'normal'
    resolution = 'fixed'
    stage = 'resolved'
    status = 'closed'
    superseder = None
    type = None
    url = 'https://bugs.python.org/issue34651'
    versions = ['Python 3.8']

    @ericsnowcurrently
    Copy link
    Member Author

    os.fork() potentially has some problematic behavior when called from a subinterpreter. In additional to the normal fork+threads madness, there's the question of what to do with existing subinterpreters. The simplest solution is to simply disallow fork in a subinterpreter and then wipe out all subinterpreters in the child process post-fork (if os.fork() called in the main interpreter).

    @ericsnowcurrently ericsnowcurrently added the 3.8 only security fixes label Sep 12, 2018
    @ericsnowcurrently ericsnowcurrently self-assigned this Sep 12, 2018
    @ericsnowcurrently ericsnowcurrently added the interpreter-core (Objects, Python, Grammar, and Parser dirs) label Sep 12, 2018
    @gpshead
    Copy link
    Member

    gpshead commented Sep 13, 2018

    +1 agreed. this is the simplest approach to start with.

    Code to restrict: os.fork itself and disallowing the use of preexec_fn on subprocess within subinterpreters.

    feel free to ignore preexec_fn in subprocess for the time being if desired, we already promise people that it is a bad idea legacy API that we don't want, guaranteed to cause undiagnosable problems and deadlocks at times. :)

    @ericsnowcurrently
    Copy link
    Member Author

    New changeset 5903296 by Eric Snow in branch 'master':
    bpo-34651: Only allow the main interpreter to fork. (gh-9279)
    5903296

    @hroncok
    Copy link
    Mannequin

    hroncok mannequin commented May 24, 2019

    It appears that as a result of this, subprocess.Popen cannot be called from within a subinterpreter either. Is that an obvious and desired limitation?

    @vstinner
    Copy link
    Member

    I reopen the issue to let Eric answer ;-)

    If the behavior is deliberate, maybe it should just be documented somewhere?

    @vstinner vstinner reopened this May 24, 2019
    @gpshead
    Copy link
    Member

    gpshead commented May 24, 2019

    I'd start by documenting the limitation and keeping a future feature request of "Allow subprocess to work from subinterpreters". That is doable. Supporting os.fork() is not.

    @vstinner
    Copy link
    Member

    I'd start by documenting the limitation and keeping a future feature request of "Allow subprocess to work from subinterpreters". That is doable. Supporting os.fork() is not.

    subprocess is able to use os.posix_spawn() in Python 3.8. In that case, it's not limited by os.fork() check ;-) Windows isn't limited neither, CreateProcess() isn't limited in subinterpreters.

    @ericsnowcurrently
    Copy link
    Member Author

    FYI, I plan on looking into this either today or next Friday.

    @vstinner
    Copy link
    Member

    See also bpo-37266: "Daemon threads must be forbidden in subinterpreters".

    @hroncok
    Copy link
    Mannequin

    hroncok mannequin commented Aug 28, 2019

    The problem with subprocess.Popen has been fixed in https://bugs.python.org/issue37951

    @miss-islington
    Copy link
    Contributor

    New changeset b220300 by Miss Islington (bot) (Phil Connell) in branch 'master':
    bpo-38778: Document that os.fork is not allowed in subinterpreters (GH-17123)
    b220300

    @miss-islington
    Copy link
    Contributor

    New changeset a4be5aa by Miss Islington (bot) in branch '3.8':
    bpo-38778: Document that os.fork is not allowed in subinterpreters (GH-17123)
    a4be5aa

    Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
    Labels
    3.8 only security fixes interpreter-core (Objects, Python, Grammar, and Parser dirs)
    Projects
    None yet
    Development

    No branches or pull requests

    4 participants