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

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

Can't Edit
Can't Publish+Mail
Start Review
6 years, 10 months ago by gdb
3 years, 7 months ago
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

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


Total messages: 1
6 years, 9 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.

File Lib/concurrent/futures/process.py (right):

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.

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

File Lib/test/test_concurrent_futures.py (right):

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