Skip to content

Commit d21f0f4

Browse files
committed
gh-158497: Fix traceback loss and a reference cycle in Executor.map()
_process_chunk() returned a failed call's bare exception, so ProcessPoolExecutor.map() lost the _RemoteTraceback cause that _process_worker() attaches. Wrap it in _ExceptionWithTraceback. _MapResultIterator.__next__() kept the exception it raised in a local, so the exception referenced its own raising frame through its traceback. Clear the local after raising, as Future.__get_result() does.
1 parent 77c0675 commit d21f0f4

4 files changed

Lines changed: 34 additions & 3 deletions

File tree

‎Lib/concurrent/futures/_base.py‎

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -723,7 +723,10 @@ def __iter__(self):
723723
def __next__(self):
724724
value, exc = next(self.gen)
725725
if exc is not None:
726-
raise exc
726+
try:
727+
raise exc
728+
finally:
729+
exc = None
727730
return value
728731

729732
def close(self):

‎Lib/concurrent/futures/process.py‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -206,7 +206,7 @@ def _process_chunk(fn, chunk):
206206
try:
207207
result = (fn(*args), None)
208208
except BaseException as exc:
209-
result = (None, exc)
209+
result = (None, _ExceptionWithTraceback(exc, exc.__traceback__))
210210
results.append(result)
211211
return results
212212

‎Lib/test/test_concurrent_futures/executor.py‎

Lines changed: 18 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,8 @@
1+
import gc
12
import itertools
23
import threading
34
import time
5+
import types
46
import weakref
57
from concurrent import futures
68
from operator import add
@@ -93,6 +95,22 @@ def test_map_exception(self):
9395
self.assertRaises(StopIteration, next, i)
9496
self.assertRaises(StopIteration, next, i)
9597

98+
@warnings_helper.ignore_fork_in_thread_deprecation_warnings()
99+
@support.cpython_only
100+
def test_map_exception_refcycle(self):
101+
# The iterator's frame that re-raises the exception must not keep
102+
# a reference to it, or the exception and its traceback stay alive
103+
# until the next garbage collection.
104+
i = self.executor.map(raiser, [ValueError])
105+
try:
106+
next(i)
107+
except ValueError as e:
108+
exc = e
109+
code = futures._base._MapResultIterator.__next__.__code__
110+
frames = [r for r in gc.get_referrers(exc)
111+
if isinstance(r, types.FrameType) and r.f_code is code]
112+
self.assertEqual(frames, [])
113+
96114
@warnings_helper.ignore_fork_in_thread_deprecation_warnings()
97115
def test_map_timeout_from_callable(self):
98116
# A TimeoutError from the callable is not the map() timeout, whether

‎Lib/test/test_concurrent_futures/test_process_pool.py‎

Lines changed: 11 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -14,7 +14,7 @@
1414
from test.support import hashlib_helper, threading_helper, warnings_helper
1515
from test.test_importlib.metadata.fixtures import parameterize
1616

17-
from .executor import ExecutorTest, mul
17+
from .executor import ExecutorTest, mul, raiser
1818
from .util import (
1919
ProcessPoolForkMixin, ProcessPoolForkserverMixin, ProcessPoolSpawnMixin,
2020
create_executor_tests, setup_module)
@@ -141,6 +141,16 @@ def test_traceback(self):
141141
self.assertIn('raise RuntimeError(123) # some comment',
142142
f1.getvalue())
143143

144+
@warnings_helper.ignore_fork_in_thread_deprecation_warnings()
145+
def test_map_traceback(self):
146+
# The traceback from the child process is also kept for map().
147+
i = self.executor.map(raiser, [RuntimeError])
148+
with self.assertRaises(RuntimeError) as cm:
149+
next(i)
150+
cause = cm.exception.__cause__
151+
self.assertIs(type(cause), futures.process._RemoteTraceback)
152+
self.assertIn('raise exception(msg)', cause.tb)
153+
144154
def test_traceback_when_child_process_terminates_abruptly(self):
145155
# gh-139462 enhancement - BrokenProcessPool exceptions
146156
# should describe which process terminated.

0 commit comments

Comments
 (0)