Skip to content

Commit 4869640

Browse files
ebonnalgraingertambv
authored
gh-74028: concurrent.futures.Executor.map: avoid reference cycles when an exception is raised (#131701)
Co-authored-by: Muse Code (Meta Muse Spark) Co-authored-by: Thomas Grainger <tagrain@gmail.com> Co-authored-by: Łukasz Langa <lukasz@langa.pl>
1 parent 7c39e8f commit 4869640

3 files changed

Lines changed: 51 additions & 4 deletions

File tree

‎Lib/concurrent/futures/_base.py‎

Lines changed: 5 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -723,7 +723,11 @@ 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+
# avoid a ref cycle where exc is captured in its traceback
730+
exc = None
727731
return value
728732

729733
def close(self):

‎Lib/concurrent/futures/interpreter.py‎

Lines changed: 5 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -88,7 +88,11 @@ def run(self, task):
8888
if exc is None:
8989
# The exception must have been not shareable.
9090
raise # re-raise
91-
raise exc from wrapper
91+
try:
92+
raise exc from wrapper
93+
finally:
94+
# avoid a ref cycle where exc is captured in its traceback
95+
exc = None
9296

9397

9498
class BrokenInterpreterPool(_thread.BrokenThreadPool):

‎Lib/test/test_concurrent_futures/executor.py‎

Lines changed: 41 additions & 2 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 traceback
46
import weakref
57
from concurrent import futures
68
from operator import add
@@ -80,7 +82,44 @@ def test_map_exception(self):
8082
i = self.executor.map(divmod, [5, 5, 5, 5], [2, 3, 0, 5])
8183
self.assertEqual(next(i), (2, 1))
8284
self.assertEqual(next(i), (1, 2))
83-
self.assertRaises(ZeroDivisionError, next, i)
85+
86+
exception = None
87+
try:
88+
i.__next__()
89+
except Exception as e:
90+
exception = e
91+
self.assertTrue(
92+
isinstance(exception, ZeroDivisionError),
93+
msg="should raise a ZeroDivisionError",
94+
)
95+
96+
# Wait for the completing thread to release its transient reference
97+
# to the exception; do not keep the referrers list alive in the loop.
98+
for _ in support.sleeping_retry(support.SHORT_TIMEOUT, error=False):
99+
if not gc.get_referrers(exception):
100+
break
101+
102+
self.assertFalse(
103+
gc.get_referrers(exception),
104+
msg="the exception should not have any referrers",
105+
)
106+
107+
self.assertFalse(
108+
[
109+
(var, val)
110+
# go through the frames of the exception's traceback
111+
for frame, _ in traceback.walk_tb(exception.__traceback__)
112+
# skipping the current frame
113+
if frame is not exception.__traceback__.tb_frame
114+
# go through the locals captured in that frame
115+
for var, val in frame.f_locals.items()
116+
# check if one of them is an exception
117+
if isinstance(val, Exception)
118+
# check if it is captured in its own traceback
119+
and frame is val.__traceback__.tb_frame
120+
],
121+
msg=f"the exception's traceback should not contain an exception captured in its own traceback",
122+
)
84123
self.assertEqual(next(i), (1, 0))
85124
self.assertRaises(StopIteration, next, i)
86125
self.assertRaises(StopIteration, next, i)
@@ -212,7 +251,7 @@ def test_map_buffersize_when_buffer_is_full(self):
212251
self.assertEqual(
213252
next(ints),
214253
buffersize,
215-
msg="should have fetched only `buffersize` elements from `ints`.",
254+
msg="should have fetched only `buffersize` elements from `ints`",
216255
)
217256

218257
@warnings_helper.ignore_fork_in_thread_deprecation_warnings()

0 commit comments

Comments
 (0)