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

Add close() to multiprocessing.Process #74781

Closed
pitrou opened this issue Jun 8, 2017 · 10 comments
Closed

Add close() to multiprocessing.Process #74781

pitrou opened this issue Jun 8, 2017 · 10 comments
Labels
3.7 stdlib type-feature

Comments

@pitrou
Copy link
Member

@pitrou pitrou commented Jun 8, 2017

BPO 30596
Nosy @pitrou, @vstinner, @JimJJewett, @applio
PRs
  • #2010
  • 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 = None
    closed_at = <Date 2017-06-24.17:22:45.256>
    created_at = <Date 2017-06-08.10:25:03.845>
    labels = ['3.7', 'type-feature', 'library']
    title = 'Add close() to multiprocessing.Process'
    updated_at = <Date 2017-06-26.11:48:35.111>
    user = 'https://github.com/pitrou'

    bugs.python.org fields:

    activity = <Date 2017-06-26.11:48:35.111>
    actor = 'pitrou'
    assignee = 'none'
    closed = True
    closed_date = <Date 2017-06-24.17:22:45.256>
    closer = 'pitrou'
    components = ['Library (Lib)']
    creation = <Date 2017-06-08.10:25:03.845>
    creator = 'pitrou'
    dependencies = []
    files = []
    hgrepos = []
    issue_num = 30596
    keywords = []
    message_count = 10.0
    messages = ['295396', '295461', '295741', '295750', '295783', '295784', '295803', '296779', '296877', '296878']
    nosy_count = 5.0
    nosy_names = ['pitrou', 'vstinner', 'asksol', 'Jim.Jewett', 'davin']
    pr_nums = ['2010']
    priority = 'normal'
    resolution = 'fixed'
    stage = 'resolved'
    status = 'closed'
    superseder = None
    type = 'enhancement'
    url = 'https://bugs.python.org/issue30596'
    versions = ['Python 3.7']

    @pitrou
    Copy link
    Member Author

    @pitrou pitrou commented Jun 8, 2017

    multiprocessing.Process (actually, the _popen object attached to it) has a GC-based finalizer to release system resources (such as fds). However, it would be nice to be able to release those resources in a timely manner. Adding a close() method would let users do that. What do you think?

    @pitrou pitrou added 3.7 stdlib type-feature labels Jun 8, 2017
    @pitrou
    Copy link
    Member Author

    @pitrou pitrou commented Jun 8, 2017

    close() wouldn't terminate the underlying process, so the process would still exist (and wouldn't easily be stoppable from Python anymore) if you were to call close() before terminate() or join().

    Perhaps we should instead mandate people call join() before close()?

    @jimjjewett
    Copy link
    Mannequin

    @jimjjewett jimjjewett mannequin commented Jun 12, 2017

    Then why not just call join as part of the default close method?

    @pitrou
    Copy link
    Member Author

    @pitrou pitrou commented Jun 12, 2017

    That's a good question. close() methods on other objects tend to avoid taking an infinite amount of time :-) But then, Process objects are different enough that they don't need to follow that rule.

    @jimjjewett
    Copy link
    Mannequin

    @jimjjewett jimjjewett mannequin commented Jun 12, 2017

    Could join be called in a background thread, or even asynchronously? That
    seems like mixing paradigms, but ...

    On Jun 12, 2017 3:15 AM, "Antoine Pitrou" <report@bugs.python.org> wrote:

    Antoine Pitrou added the comment:

    That's a good question. close() methods on other objects tend to avoid
    taking an infinite amount of time :-) But then, Process objects are
    different enough that they don't need to follow that rule.

    ----------


    Python tracker <report@bugs.python.org>
    <http://bugs.python.org/issue30596\>


    @pitrou
    Copy link
    Member Author

    @pitrou pitrou commented Jun 12, 2017

    I want close() to be deterministic. So I guess we have two simple possibilities:

    1. close() raises if the process is still alive
    2. close() calls join() implicitly if the process is still alive

    @pitrou
    Copy link
    Member Author

    @pitrou pitrou commented Jun 12, 2017

    I've decided it's better to raise a ValueError if the child process hasn't stopped yet. After all, join() alone may have no effect: often you want to send the process a signal (a literal signal in UNIX terms, or a figurative signal such as write something on a socket, etc.) to ask it to terminate gently.

    I'm gonna update the PR soon.

    @pitrou
    Copy link
    Member Author

    @pitrou pitrou commented Jun 24, 2017

    New changeset 13e96cc by Antoine Pitrou in branch 'master':
    Fix bpo-30596: Add close() method to multiprocessing.Process (bpo-2010)
    13e96cc

    @pitrou pitrou closed this as completed Jun 24, 2017
    @vstinner
    Copy link
    Member

    @vstinner vstinner commented Jun 26, 2017

    Since the object now has a close() method, would it make sense to emit a ResourceWarning if it's not closed explicitly? As I did recently in subprocess.Popen destructor.

    @pitrou
    Copy link
    Member Author

    @pitrou pitrou commented Jun 26, 2017

    I don't think so. It is ok to let the GC delete the resources by itself. close() is just there for people who want to ensure whatever small amount of resources (a file descriptor, mostly) are released timely.

    @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 stdlib type-feature
    Projects
    None yet
    Development

    No branches or pull requests

    2 participants