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.

Sign up for GitHub

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

bpo-28326: Fixes multiprocessing.Process depends on sys.stdout being open #1410

Closed
wants to merge 1 commit into from
Closed

bpo-28326: Fixes multiprocessing.Process depends on sys.stdout being open #1410

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 4 additions & 2 deletions Lib/multiprocessing/popen_fork.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -14,8 +14,10 @@ class Popen(object):
method = 'fork'

def __init__(self, process_obj):
sys.stdout.flush()
sys.stderr.flush()
if not sys.stdout.closed:
Copy link
Member

Choose a reason for hiding this comment

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

Either change this to
if sys.stdout is not None and not sys.stdout.closed:

or change the entire statement to

        try:
            sys.stdout.flush()
        except (AttributeError, ValueError):  # stdout None or closed.
            pass

and the equivalent for stderr. I slightly prefer the latter (these exceptions should be rare), but if Antoine expresses a preference for the former, use that.

sys.stdout.flush()
if not sys.stderr.closed:
sys.stderr.flush()
self.returncode = None
self._launch(process_obj)

Expand Down
9 changes: 8 additions & 1 deletion Lib/test/test_multiprocessing_main_handling.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -38,7 +38,7 @@

import sys
import time
from multiprocessing import Pool, set_start_method
from multiprocessing import Pool, Process, set_start_method

# We use this __main__ defined function in the map call below in order to
# check that multiprocessing in correctly running the unguarded
Expand All @@ -65,6 +65,13 @@ def f(x):
raise RuntimeError("Timed out waiting for results")
results.sort()
print(start_method, "->", results)
sys.stdout.close()
Copy link
Member

Choose a reason for hiding this comment

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

While I understand it's practical to do so, it's a bit weird to add your test there, as it doesn't really have anything to do with __main__ handling. Instead, I would expect it to be somewhere in _TestProcess in _test_multiprocessing.py.

Copy link
Member

Choose a reason for hiding this comment

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

I also wondered about the placement, so I think you should follow Antoine's suggestion.

sys.stderr.close()
process = Process(target=f, args=(1,))
process.start()
process.join(10)
if process.is_alive():
raise RuntimeError("Timed out waiting for single process")
"""

test_source_main_skipped_in_children = """\
Expand Down