Skip to content

Commit 4e8c9c6

Browse files
daniel-shieldsblurb-it[bot]gpshead
committed
gh-101267: ProcessPoolExecutor no longer shares 1 BrokenProcessPool exception among all failed futures (GH-101268)
(cherry picked from commit 3c00ebc) Co-authored-by: Daniel Shields <daniel.shields@twosigma.com> Co-authored-by: blurb-it[bot] <43283697+blurb-it[bot]@users.noreply.github.com> Co-authored-by: Gregory P. Smith <greg@krypto.org>
1 parent 04973e7 commit 4e8c9c6

4 files changed

Lines changed: 44 additions & 7 deletions

File tree

Lib/concurrent/futures/process.py

Lines changed: 9 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -469,11 +469,9 @@ def _terminate_broken(self, cause):
469469
executor._shutdown_thread = True
470470
executor = None
471471

472-
# All pending tasks are to be marked failed with the following
473-
# BrokenProcessPool error
474-
bpe = BrokenProcessPool("A process in the process pool was "
475-
"terminated abruptly while the future was "
476-
"running or pending.")
472+
# All pending tasks are to be marked failed with a
473+
# BrokenProcessPool error, as separate instances to avoid sharing
474+
# a traceback (gh-101267).
477475
cause_str = None
478476
if cause is not None:
479477
cause_str = ''.join(cause)
@@ -489,11 +487,15 @@ def _terminate_broken(self, cause):
489487
f"with exit code {p.exitcode}")
490488
if errors:
491489
cause_str = "\n".join(errors)
492-
if cause_str:
493-
bpe.__cause__ = _RemoteTraceback(f"\n'''\n{cause_str}'''")
490+
cause_tb = f"\n'''\n{cause_str}'''" if cause_str else None
494491

495492
# Mark pending tasks as failed.
496493
for work_id, work_item in self.pending_work_items.items():
494+
bpe = BrokenProcessPool("A process in the process pool was "
495+
"terminated abruptly while the future was "
496+
"running or pending.")
497+
if cause_tb is not None:
498+
bpe.__cause__ = _RemoteTraceback(cause_tb)
497499
try:
498500
work_item.future.set_exception(bpe)
499501
except _base.InvalidStateError:

Lib/test/test_concurrent_futures/test_process_pool.py

Lines changed: 27 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -3,6 +3,7 @@
33
import sys
44
import threading
55
import time
6+
import traceback
67
import unittest
78
import unittest.mock
89
from concurrent import futures
@@ -62,6 +63,32 @@ def test_killed_child(self):
6263
# Submitting other jobs fails as well.
6364
self.assertRaises(BrokenProcessPool, self.executor.submit, pow, 2, 8)
6465

66+
@warnings_helper.ignore_fork_in_thread_deprecation_warnings()
67+
def test_broken_process_pool_traceback(self):
68+
# When a child process is abruptly terminated, the whole pool gets
69+
# "broken", and a BrokenProcessPool exception should be created
70+
# for each future instead of sharing one exception among all futures.
71+
event = self.create_event()
72+
futures = [self.executor.submit(event.wait) for _ in range(3)]
73+
p = next(iter(self.executor._processes.values()))
74+
p.terminate()
75+
for fut in futures:
76+
# Don't use assertRaises(): it clears the traceback off exc.
77+
try:
78+
fut.result()
79+
except BrokenProcessPool as exc:
80+
tb = exc.__traceback__
81+
else:
82+
self.fail("BrokenProcessPool not raised")
83+
count = sum(
84+
1
85+
for frame_summary in traceback.extract_tb(tb)
86+
if frame_summary.filename == __file__
87+
)
88+
# This code file should appear exactly once in the traceback.
89+
# A shared exception would accumulate a frame per result() call.
90+
self.assertEqual(count, 1)
91+
6592
@warnings_helper.ignore_fork_in_thread_deprecation_warnings()
6693
def test_map_chunksize(self):
6794
def bad_map():

Misc/ACKS

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1762,6 +1762,7 @@ Charlie Shepherd
17621762
Bruce Sherwood
17631763
Gregory Shevchenko
17641764
Hai Shi
1765+
Daniel Shields
17651766
Alexander Shigin
17661767
Pete Shinners
17671768
Michael Shiplett
Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,7 @@
1+
When a worker process terminates unexpectedly,
2+
:class:`concurrent.futures.ProcessPoolExecutor` now sets a separate
3+
:exc:`~concurrent.futures.process.BrokenProcessPool` exception on each pending
4+
future instead of sharing a single instance among them all. Sharing one
5+
exception produced malformed tracebacks: each
6+
:meth:`Future.result() <concurrent.futures.Future.result>` call re-raised the
7+
same object, appending another copy of the traceback to it.

0 commit comments

Comments
 (0)