Rietveld Code Review Tool
Help | Bug tracker | Discussion group | Source code | Sign in
(49)

#9205: Parent process hanging in multiprocessing if children terminate unexpectedly

Can't Edit
Can't Publish+Mail
Start Review
Created:
5 years, 12 months ago by gdb
Modified:
2 years, 8 months ago
Reviewers:
brian
CC:
jcea, bquinlan, AntoinePitrou, haypo, jnoller_gmail.com, hongqn_gmail.com, asksol, andrey.vlasovskikh_gmail.com, Charles-Fran├žois Natali, gdb_mit.edu, fullung_gmail.com, stillfire_gmail.com, devnull_psf.upfronthosting.co.za, sbt, gokcen.eraslan_gmail.com, dan.oreilly
Visibility:
Public.

Patch Set 1 #

Total comments: 3

Patch Set 2 #

Unified diffs Side-by-side diffs Delta from patch set Stats Patch
Doc/library/multiprocessing.rst View 1 2 chunks +14 lines, -0 lines 0 comments Download
Lib/multiprocessing/pool.py View 1 14 chunks +93 lines, -14 lines 0 comments Download
Lib/test/_test_multiprocessing.py View 1 2 chunks +15 lines, -0 lines 0 comments Download

Messages

Total messages: 1
bquinlan
5 years, 10 months ago #1
I only looked at the concurrent.future part but that looks good!

I would say that the implementation is non-obvious so maybe some strategic
comments would be in order.

http://bugs.python.org/review/9205/diff/2788/6752
File Lib/concurrent/futures/process.py (right):

http://bugs.python.org/review/9205/diff/2788/6752#newcode201
Lib/concurrent/futures/process.py:201: nb_children_alive = sum(p.is_alive() for
p in processes.values())
nb => num 
just to be consistent with the style of the rest of the package.

http://bugs.python.org/review/9205/diff/2788/6752#newcode298
Lib/concurrent/futures/process.py:298: """
Could you start the doc comment with a single line synopsis to be consistent
with the rest of the package?

Also, I think that this should go into the _base module and be renamed
"InternalExecutorError" or something for two reasons:
1. we don't currently require users to reach into the process or thread modules
to access exceptions
2. this exception might be useful for other executor types and it would be nice
if users could handle this kind of failure without knowing the executor type

http://bugs.python.org/review/9205/diff/2788/6756
File Lib/test/test_concurrent_futures.py (right):

http://bugs.python.org/review/9205/diff/2788/6756#newcode391
Lib/test/test_concurrent_futures.py:391: for fut in futures:
fut => f (for consistency)
Sign in to reply to this message.

RSS Feeds Recent Issues | This issue
This is Rietveld 894c83f36cb7