Skip to content

gh-87063: Add closed attribute to multiprocessing.Process#125838

Open
rruuaanng wants to merge 20 commits intopython:mainfrom
rruuaanng:gh87063
Open

gh-87063: Add closed attribute to multiprocessing.Process#125838
rruuaanng wants to merge 20 commits intopython:mainfrom
rruuaanng:gh87063

Conversation

@rruuaanng
Copy link
Copy Markdown
Contributor

@rruuaanng rruuaanng commented Oct 22, 2024 •
edited
Loading

for example:

import multiprocessing


def f():
    print('hello')
    print('world')

if __name__ == "__main__":
    p = multiprocessing.Process(target=f)
    p.start()
    p.join()

    # In a more context, it's closed
    p.close()
    
    # Currnet context
    # It's thrown a ValueError
    if p.exitcode == 0:
        pass

    # Changed
    if not p.is_closed():
        print(p.exitcode)

@rruuaanng rruuaanng requested a review from gpshead as a code owner October 22, 2024 12:07
@bedevere-app bedevere-app Bot mentioned this pull request Oct 22, 2024
Open
@rruuaanng
Copy link
Copy Markdown
Contributor Author

rruuaanng commented Oct 22, 2024

It seems that this NEWS error can be ignored.

picnixz
picnixz reviewed Oct 22, 2024
View reviewed changes
Copy link
Copy Markdown
Member

@picnixz picnixz left a comment

Choose a reason for hiding this comment

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

More generally, required CI checks must never be ignored except for NEWS missing / issue missing since normal users cannot apply the skip news/issue labels themselves.

Comment thread Misc/NEWS.d/next/Library/2024-10-22-19-52-15.gh-issue-87063.fR-wJa.rst Outdated
Comment thread Lib/test/_test_multiprocessing.py Outdated
Comment thread Lib/multiprocessing/process.py
picnixz
picnixz reviewed Oct 24, 2024
View reviewed changes
Comment thread Doc/library/multiprocessing.rst Outdated
Comment thread Doc/library/multiprocessing.rst Outdated
Comment thread Lib/test/_test_multiprocessing.py Outdated
Comment thread Misc/NEWS.d/next/Library/2024-10-22-19-52-15.gh-issue-87063.fR-wJa.rst Outdated
@erlend-aasland erlend-aasland changed the title gh-87063: Add is_closed method to multiprocessing.Process gh-87063: Add is_closed attribute to multiprocessing.Process Oct 25, 2024
@erlend-aasland erlend-aasland changed the title gh-87063: Add is_closed attribute to multiprocessing.Process gh-87063: Add closed attribute to multiprocessing.Process Oct 25, 2024
erlend-aasland
erlend-aasland reviewed Oct 25, 2024
View reviewed changes
Comment thread Misc/NEWS.d/next/Library/2024-10-22-19-52-15.gh-issue-87063.fR-wJa.rst Outdated
erlend-aasland
erlend-aasland requested changes Oct 26, 2024
View reviewed changes
Copy link
Copy Markdown
Contributor

@erlend-aasland erlend-aasland left a comment

Choose a reason for hiding this comment

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

PR author did not listen to feedback. Requesting changes to make this point clearer.

@bedevere-app
Copy link
Copy Markdown

bedevere-app Bot commented Oct 26, 2024

A Python core developer has requested some changes be made to your pull request before we can consider merging it. If you could please address their requests along with any other requests in other reviews from core developers that would be appreciated.

Once you have made the requested changes, please leave a comment on this pull request containing the phrase I have made the requested changes; please review again. I will then notify any core developers who have left a review that you're ready for them to take another look at this pull request.

@rruuaanng
Copy link
Copy Markdown
Contributor Author

rruuaanng commented Oct 26, 2024

Sorry for my misunderstanding. I thought it was to change NEWS to what's NEWS, but it was actually to add to it. I'm sorry for that. I've made change :)

picnixz
picnixz reviewed Oct 26, 2024
View reviewed changes
Comment thread Misc/NEWS.d/next/Library/2024-10-22-19-52-15.gh-issue-87063.fR-wJa.rst Outdated
Comment thread Doc/whatsnew/3.14.rst Outdated
Comment thread Doc/whatsnew/3.14.rst Outdated
picnixz
picnixz reviewed Oct 26, 2024
View reviewed changes
Comment thread Lib/test/_test_multiprocessing.py Outdated
Comment thread Lib/test/_test_multiprocessing.py
@rruuaanng
Copy link
Copy Markdown
Contributor Author

rruuaanng commented Oct 26, 2024

Thanks to Bénédikt for taking responsibility for this PR. And, thanks to erlend for pointing out my mistake.👍

picnixz
picnixz reviewed Oct 26, 2024
View reviewed changes
Copy link
Copy Markdown
Member

@picnixz picnixz left a comment

Choose a reason for hiding this comment

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

A final nitpick. Concerning the naming of the attribute, I suggested a property + closed but if @gpshead feels that it should be a method + is_closed(), then I'm sorry in advance :) (just recalling that my rationale was that multiprocessing.connection.Connection uses the closed property but I don't know if properties are preferred over true methods when we are just returning a private attribute).

Comment thread Doc/whatsnew/3.14.rst
erlend-aasland
erlend-aasland reviewed Oct 26, 2024
View reviewed changes
Comment thread Misc/NEWS.d/next/Library/2024-10-22-19-52-15.gh-issue-87063.fR-wJa.rst Outdated
@rruuaanng

This comment was marked as outdated.

Sign in to view
@rruuaanng
Copy link
Copy Markdown
Contributor Author

rruuaanng commented Nov 8, 2024

@erlend-aasland Please review them again! :)

@erlend-aasland
Copy link
Copy Markdown
Contributor

erlend-aasland commented Nov 8, 2024

@erlend-aasland Please review them again! :)

I asked some questions on the issue; IMO, they should be clarified before eventually landing this PR. In other words: I believe more discussion and perhaps a larger audience is needed.

@rruuaanng
Copy link
Copy Markdown
Contributor Author

rruuaanng commented Jan 10, 2025

Happy New Year, everyone. It's been a long time since the last review. Do we still need to keep this open?

@github-actions
Copy link
Copy Markdown

github-actions Bot commented Apr 19, 2026

This PR is stale because it has been open for 30 days with no activity.

@github-actions github-actions Bot added the stale Stale PR or inactive for long period of time. label Apr 19, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@picnixz picnixz picnixz left review comments

@gpshead gpshead Awaiting requested review from gpshead gpshead is a code owner

@erlend-aasland erlend-aasland Awaiting requested review from erlend-aasland

Assignees

No one assigned

Labels

awaiting changes stale Stale PR or inactive for long period of time.

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

3 participants

@rruuaanng @erlend-aasland @picnixz