From 7dc850b64060de12f191b5df4a78d1c525a6f31c Mon Sep 17 00:00:00 2001 From: ebonnal Date: Mon, 24 Mar 2025 23:49:31 +0000 Subject: [PATCH 01/41] =?UTF-8?q?`test=5Fconcurrent=5Ffutures.test=5Fmap?= =?UTF-8?q?=5Fexception`:=20assert=20no=20reference=20cycle=20from=20faile?= =?UTF-8?q?d=20future=20captured=20in=20its=20exception=E2=80=99s=20traceb?= =?UTF-8?q?ack?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- Lib/test/test_concurrent_futures/executor.py | 17 +++++++++++++++-- 1 file changed, 15 insertions(+), 2 deletions(-) diff --git a/Lib/test/test_concurrent_futures/executor.py b/Lib/test/test_concurrent_futures/executor.py index d88c34d1c8c8e4..375bae8be300f9 100644 --- a/Lib/test/test_concurrent_futures/executor.py +++ b/Lib/test/test_concurrent_futures/executor.py @@ -1,3 +1,4 @@ +import gc import itertools import threading import time @@ -55,8 +56,20 @@ def test_map_exception(self): i = self.executor.map(divmod, [1, 1, 1, 1], [2, 3, 0, 5]) self.assertEqual(i.__next__(), (0, 1)) self.assertEqual(i.__next__(), (0, 1)) - with self.assertRaises(ZeroDivisionError): - i.__next__() + + error = None + try: + next(i) + except ZeroDivisionError as zero_div_error: + error = zero_div_error + self.assertIsNotNone( + error, + msg="next one should raise a ZeroDivisionError", + ) + + # a failed future must not be captured in its + # future._exception.__traceback__ to avoid a reference cycle + self.assertFalse(gc.get_referrers(error)) @support.requires_resource('walltime') def test_map_timeout(self): From 19edcedcf76f0ffbd13e19ffc00cf5e9f3b6bcbe Mon Sep 17 00:00:00 2001 From: ebonnal Date: Tue, 25 Mar 2025 00:33:59 +0000 Subject: [PATCH 02/41] test there is no future in referrers --- Lib/test/test_concurrent_futures/executor.py | 7 ++++++- 1 file changed, 6 insertions(+), 1 deletion(-) diff --git a/Lib/test/test_concurrent_futures/executor.py b/Lib/test/test_concurrent_futures/executor.py index 375bae8be300f9..5496ba88475562 100644 --- a/Lib/test/test_concurrent_futures/executor.py +++ b/Lib/test/test_concurrent_futures/executor.py @@ -69,7 +69,12 @@ def test_map_exception(self): # a failed future must not be captured in its # future._exception.__traceback__ to avoid a reference cycle - self.assertFalse(gc.get_referrers(error)) + self.assertFalse( + any( + isinstance(referrer, futures.Future) + for referrer in gc.get_referrers(error) + ) + ) @support.requires_resource('walltime') def test_map_timeout(self): From 1c3d01ac1b696044b16e60e5c18f83a842d8f370 Mon Sep 17 00:00:00 2001 From: ebonnal Date: Tue, 25 Mar 2025 01:42:12 +0000 Subject: [PATCH 03/41] switch to assertTrue(all(...)) --- Lib/test/test_concurrent_futures/executor.py | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/Lib/test/test_concurrent_futures/executor.py b/Lib/test/test_concurrent_futures/executor.py index 5496ba88475562..9d4e687574eb4a 100644 --- a/Lib/test/test_concurrent_futures/executor.py +++ b/Lib/test/test_concurrent_futures/executor.py @@ -69,9 +69,9 @@ def test_map_exception(self): # a failed future must not be captured in its # future._exception.__traceback__ to avoid a reference cycle - self.assertFalse( - any( - isinstance(referrer, futures.Future) + self.assertTrue( + all( + not isinstance(referrer, futures.Future) for referrer in gc.get_referrers(error) ) ) From fa790f1329dc7359b8acfd35074339eb9ae40836 Mon Sep 17 00:00:00 2001 From: ebonnal Date: Tue, 25 Mar 2025 11:12:36 +0000 Subject: [PATCH 04/41] add test msg --- Lib/test/test_concurrent_futures/executor.py | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/Lib/test/test_concurrent_futures/executor.py b/Lib/test/test_concurrent_futures/executor.py index 9d4e687574eb4a..d7c14aff9e3fbb 100644 --- a/Lib/test/test_concurrent_futures/executor.py +++ b/Lib/test/test_concurrent_futures/executor.py @@ -73,7 +73,8 @@ def test_map_exception(self): all( not isinstance(referrer, futures.Future) for referrer in gc.get_referrers(error) - ) + ), + msg="none of the referrers should be a Future.", ) @support.requires_resource('walltime') From e48d0e01a24a91e1730093fb54af1270893b0466 Mon Sep 17 00:00:00 2001 From: ebonnal Date: Tue, 25 Mar 2025 11:14:03 +0000 Subject: [PATCH 05/41] format msgs --- Lib/test/test_concurrent_futures/executor.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/Lib/test/test_concurrent_futures/executor.py b/Lib/test/test_concurrent_futures/executor.py index d7c14aff9e3fbb..dde6f43b7acc59 100644 --- a/Lib/test/test_concurrent_futures/executor.py +++ b/Lib/test/test_concurrent_futures/executor.py @@ -74,7 +74,7 @@ def test_map_exception(self): not isinstance(referrer, futures.Future) for referrer in gc.get_referrers(error) ), - msg="none of the referrers should be a Future.", + msg="none of the referrers should be a Future", ) @support.requires_resource('walltime') @@ -159,7 +159,7 @@ def test_map_buffersize_when_buffer_is_full(self): self.assertEqual( next(ints), buffersize, - msg="should have fetched only `buffersize` elements from `ints`.", + msg="should have fetched only `buffersize` elements from `ints`", ) def test_shutdown_race_issue12456(self): From 99850b8c9531cbedc47263a7a05c41d349633ac8 Mon Sep 17 00:00:00 2001 From: ebonnal Date: Tue, 25 Mar 2025 11:23:58 +0000 Subject: [PATCH 06/41] switch to assert list empty for a better assertion error message --- Lib/test/test_concurrent_futures/executor.py | 9 +++++---- 1 file changed, 5 insertions(+), 4 deletions(-) diff --git a/Lib/test/test_concurrent_futures/executor.py b/Lib/test/test_concurrent_futures/executor.py index dde6f43b7acc59..b939ef0379c375 100644 --- a/Lib/test/test_concurrent_futures/executor.py +++ b/Lib/test/test_concurrent_futures/executor.py @@ -69,11 +69,12 @@ def test_map_exception(self): # a failed future must not be captured in its # future._exception.__traceback__ to avoid a reference cycle - self.assertTrue( - all( - not isinstance(referrer, futures.Future) + self.assertFalse( + [ + referrer for referrer in gc.get_referrers(error) - ), + if isinstance(referrer, futures.Future) + ], msg="none of the referrers should be a Future", ) From 6e780140026081a4dff5bc4588b26502b1613742 Mon Sep 17 00:00:00 2001 From: ebonnal Date: Wed, 26 Mar 2025 22:49:32 +0000 Subject: [PATCH 07/41] edit comment --- Lib/test/test_concurrent_futures/executor.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/Lib/test/test_concurrent_futures/executor.py b/Lib/test/test_concurrent_futures/executor.py index b939ef0379c375..068c7aa494de95 100644 --- a/Lib/test/test_concurrent_futures/executor.py +++ b/Lib/test/test_concurrent_futures/executor.py @@ -67,7 +67,7 @@ def test_map_exception(self): msg="next one should raise a ZeroDivisionError", ) - # a failed future must not be captured in its + # a failed future should not be captured in its # future._exception.__traceback__ to avoid a reference cycle self.assertFalse( [ From 3dc24610854ace9c177bfb9747f5c23ec80f0046 Mon Sep 17 00:00:00 2001 From: Thomas Grainger Date: Sun, 30 Mar 2025 13:48:04 +0100 Subject: [PATCH 08/41] assert there are no reference cycles at all --- Lib/test/test_concurrent_futures/executor.py | 9 +-------- 1 file changed, 1 insertion(+), 8 deletions(-) diff --git a/Lib/test/test_concurrent_futures/executor.py b/Lib/test/test_concurrent_futures/executor.py index 068c7aa494de95..a52da877da21ef 100644 --- a/Lib/test/test_concurrent_futures/executor.py +++ b/Lib/test/test_concurrent_futures/executor.py @@ -69,14 +69,7 @@ def test_map_exception(self): # a failed future should not be captured in its # future._exception.__traceback__ to avoid a reference cycle - self.assertFalse( - [ - referrer - for referrer in gc.get_referrers(error) - if isinstance(referrer, futures.Future) - ], - msg="none of the referrers should be a Future", - ) + self.assertListEqual(gc.get_referrers(error), []) @support.requires_resource('walltime') def test_map_timeout(self): From 4dce280760eb7d8d6b3d6cd3e69437774129ca54 Mon Sep 17 00:00:00 2001 From: Thomas Grainger Date: Sun, 30 Mar 2025 13:48:38 +0100 Subject: [PATCH 09/41] avoid a reference cycle in subinterpreter pools --- Lib/concurrent/futures/interpreter.py | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/Lib/concurrent/futures/interpreter.py b/Lib/concurrent/futures/interpreter.py index d17688dc9d7346..744ccafa012aa3 100644 --- a/Lib/concurrent/futures/interpreter.py +++ b/Lib/concurrent/futures/interpreter.py @@ -205,8 +205,7 @@ def run(self, task): assert res is None, res assert pickled assert exc_wrapper is not None - exc = pickle.loads(excdata) - raise exc from exc_wrapper + raise pickle.loads(excdata) from exc_wrapper return pickle.loads(res) if pickled else res From 9bc69b9e90aa95395ba159579c21857712acdf96 Mon Sep 17 00:00:00 2001 From: ebonnal Date: Sun, 30 Mar 2025 21:20:37 +0100 Subject: [PATCH 10/41] test that traceback frames should not contain any variables referring to an Exception or a failed Future --- Lib/test/test_concurrent_futures/executor.py | 19 ++++++++++++++++--- 1 file changed, 16 insertions(+), 3 deletions(-) diff --git a/Lib/test/test_concurrent_futures/executor.py b/Lib/test/test_concurrent_futures/executor.py index a52da877da21ef..7c6992a5ce356d 100644 --- a/Lib/test/test_concurrent_futures/executor.py +++ b/Lib/test/test_concurrent_futures/executor.py @@ -67,9 +67,22 @@ def test_map_exception(self): msg="next one should raise a ZeroDivisionError", ) - # a failed future should not be captured in its - # future._exception.__traceback__ to avoid a reference cycle - self.assertListEqual(gc.get_referrers(error), []) + self.assertFalse( + gc.get_referrers(error), + msg="the raised error should not have any referrer", + ) + + tb = error.__traceback__ + while (tb := tb.tb_next): + self.assertFalse( + { + var: val + for var, val in tb.tb_frame.f_locals.items() + if isinstance(val, Exception) + or (isinstance(val, futures.Future) and val.exception()) + }, + msg=f"traceback frames should not contain any variables referring to an Exception or a failed Future", + ) @support.requires_resource('walltime') def test_map_timeout(self): From 9c781c1a4ed3ced35b0fbd3c890a9e47812a76b0 Mon Sep 17 00:00:00 2001 From: ebonnal Date: Sun, 30 Mar 2025 21:29:05 +0100 Subject: [PATCH 11/41] interpreter.WorkerContext.run: finally unassign exc_wrapper --- Lib/concurrent/futures/interpreter.py | 55 ++++++++++++++------------- 1 file changed, 29 insertions(+), 26 deletions(-) diff --git a/Lib/concurrent/futures/interpreter.py b/Lib/concurrent/futures/interpreter.py index 744ccafa012aa3..758ce7a738bd15 100644 --- a/Lib/concurrent/futures/interpreter.py +++ b/Lib/concurrent/futures/interpreter.py @@ -179,34 +179,37 @@ def run(self, task): raise NotImplementedError(kind) try: - self._exec(script) - except ExecutionFailed as exc: - exc_wrapper = exc - else: - exc_wrapper = None - - # Return the result, or raise the exception. - while True: try: - obj = _interpqueues.get(self.resultsid) - except _interpqueues.QueueNotFoundError: - raise # re-raise - except _interpqueues.QueueError: - continue - except ModuleNotFoundError: - # interpreters.queues doesn't exist, which means - # QueueEmpty doesn't. Act as though it does. - continue + self._exec(script) + except ExecutionFailed as exc: + exc_wrapper = exc else: - break - (res, excdata), pickled, unboundop = obj - assert unboundop is None, unboundop - if excdata is not None: - assert res is None, res - assert pickled - assert exc_wrapper is not None - raise pickle.loads(excdata) from exc_wrapper - return pickle.loads(res) if pickled else res + exc_wrapper = None + + # Return the result, or raise the exception. + while True: + try: + obj = _interpqueues.get(self.resultsid) + except _interpqueues.QueueNotFoundError: + raise # re-raise + except _interpqueues.QueueError: + continue + except ModuleNotFoundError: + # interpreters.queues doesn't exist, which means + # QueueEmpty doesn't. Act as though it does. + continue + else: + break + (res, excdata), pickled, unboundop = obj + assert unboundop is None, unboundop + if excdata is not None: + assert res is None, res + assert pickled + assert exc_wrapper is not None + raise pickle.loads(excdata) from exc_wrapper + return pickle.loads(res) if pickled else res + finally: + exc_wrapper = None class BrokenInterpreterPool(_thread.BrokenThreadPool): From a438301404ac092e83c8a46fad7bbd5e364b5b84 Mon Sep 17 00:00:00 2001 From: ebonnal Date: Sun, 30 Mar 2025 21:31:30 +0100 Subject: [PATCH 12/41] fix msg grammar --- Lib/test/test_concurrent_futures/executor.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/Lib/test/test_concurrent_futures/executor.py b/Lib/test/test_concurrent_futures/executor.py index 7c6992a5ce356d..35ab185f87ec78 100644 --- a/Lib/test/test_concurrent_futures/executor.py +++ b/Lib/test/test_concurrent_futures/executor.py @@ -69,7 +69,7 @@ def test_map_exception(self): self.assertFalse( gc.get_referrers(error), - msg="the raised error should not have any referrer", + msg="the raised error should not have any referrers", ) tb = error.__traceback__ From ada614007556c13284dd2a81ac4cbbb0dacdb462 Mon Sep 17 00:00:00 2001 From: ebonnal Date: Sun, 30 Mar 2025 21:47:00 +0100 Subject: [PATCH 13/41] fix access to exception --- Lib/test/test_concurrent_futures/executor.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/Lib/test/test_concurrent_futures/executor.py b/Lib/test/test_concurrent_futures/executor.py index 35ab185f87ec78..9349ac376a4c26 100644 --- a/Lib/test/test_concurrent_futures/executor.py +++ b/Lib/test/test_concurrent_futures/executor.py @@ -79,7 +79,7 @@ def test_map_exception(self): var: val for var, val in tb.tb_frame.f_locals.items() if isinstance(val, Exception) - or (isinstance(val, futures.Future) and val.exception()) + or (isinstance(val, futures.Future) and val._exception) }, msg=f"traceback frames should not contain any variables referring to an Exception or a failed Future", ) From d5e8c7a40e0577aefd248471c4648320442fb2c5 Mon Sep 17 00:00:00 2001 From: ebonnal Date: Sun, 30 Mar 2025 22:34:57 +0100 Subject: [PATCH 14/41] narrow test: traceback frames should not contain any exception --- Lib/test/test_concurrent_futures/executor.py | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/Lib/test/test_concurrent_futures/executor.py b/Lib/test/test_concurrent_futures/executor.py index 9349ac376a4c26..ac4d5aaa4434c6 100644 --- a/Lib/test/test_concurrent_futures/executor.py +++ b/Lib/test/test_concurrent_futures/executor.py @@ -79,9 +79,8 @@ def test_map_exception(self): var: val for var, val in tb.tb_frame.f_locals.items() if isinstance(val, Exception) - or (isinstance(val, futures.Future) and val._exception) }, - msg=f"traceback frames should not contain any variables referring to an Exception or a failed Future", + msg=f"traceback frames should not contain any exception", ) @support.requires_resource('walltime') From c4b771a19bed925aa6e586073be7a5de04b5eb1f Mon Sep 17 00:00:00 2001 From: ebonnal Date: Mon, 31 Mar 2025 10:51:56 +0100 Subject: [PATCH 15/41] clean up exc_wrapper with minimal indentation --- Lib/concurrent/futures/interpreter.py | 56 +++++++++++++-------------- 1 file changed, 28 insertions(+), 28 deletions(-) diff --git a/Lib/concurrent/futures/interpreter.py b/Lib/concurrent/futures/interpreter.py index 758ce7a738bd15..1c1187272a26ed 100644 --- a/Lib/concurrent/futures/interpreter.py +++ b/Lib/concurrent/futures/interpreter.py @@ -179,37 +179,37 @@ def run(self, task): raise NotImplementedError(kind) try: + self._exec(script) + except ExecutionFailed as exc: + exc_wrapper = exc + else: + exc_wrapper = None + + # Return the result, or raise the exception. + while True: try: - self._exec(script) - except ExecutionFailed as exc: - exc_wrapper = exc + obj = _interpqueues.get(self.resultsid) + except _interpqueues.QueueNotFoundError: + raise # re-raise + except _interpqueues.QueueError: + continue + except ModuleNotFoundError: + # interpreters.queues doesn't exist, which means + # QueueEmpty doesn't. Act as though it does. + continue else: - exc_wrapper = None - - # Return the result, or raise the exception. - while True: - try: - obj = _interpqueues.get(self.resultsid) - except _interpqueues.QueueNotFoundError: - raise # re-raise - except _interpqueues.QueueError: - continue - except ModuleNotFoundError: - # interpreters.queues doesn't exist, which means - # QueueEmpty doesn't. Act as though it does. - continue - else: - break - (res, excdata), pickled, unboundop = obj - assert unboundop is None, unboundop - if excdata is not None: - assert res is None, res - assert pickled - assert exc_wrapper is not None + break + (res, excdata), pickled, unboundop = obj + assert unboundop is None, unboundop + if excdata is not None: + assert res is None, res + assert pickled + assert exc_wrapper is not None + try: raise pickle.loads(excdata) from exc_wrapper - return pickle.loads(res) if pickled else res - finally: - exc_wrapper = None + finally: + exc_wrapper = None + return pickle.loads(res) if pickled else res class BrokenInterpreterPool(_thread.BrokenThreadPool): From 03f8ab47a3726b6294c842b03f0a0bf1ab7ea3b3 Mon Sep 17 00:00:00 2001 From: ebonnal Date: Mon, 31 Mar 2025 10:53:39 +0100 Subject: [PATCH 16/41] test_map_exception: only search for exception that captures itself in its own traceback --- Lib/test/test_concurrent_futures/executor.py | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/Lib/test/test_concurrent_futures/executor.py b/Lib/test/test_concurrent_futures/executor.py index ac4d5aaa4434c6..47bfda9096b560 100644 --- a/Lib/test/test_concurrent_futures/executor.py +++ b/Lib/test/test_concurrent_futures/executor.py @@ -79,8 +79,9 @@ def test_map_exception(self): var: val for var, val in tb.tb_frame.f_locals.items() if isinstance(val, Exception) + and var in val.__traceback__.tb_frame.f_locals }, - msg=f"traceback frames should not contain any exception", + msg=f"the raised exception's traceback should not contain an exception that captures itself in its own traceback", ) @support.requires_resource('walltime') From ef301226c144bc92c64f3bbc5fb78a8c6177d298 Mon Sep 17 00:00:00 2001 From: ebonnal Date: Mon, 31 Mar 2025 11:42:27 +0100 Subject: [PATCH 17/41] concurrent.futures.process: avoid ref cycle in _sendback_result --- Lib/concurrent/futures/process.py | 10 +++++++--- 1 file changed, 7 insertions(+), 3 deletions(-) diff --git a/Lib/concurrent/futures/process.py b/Lib/concurrent/futures/process.py index 4847550908adab..e6e7f304f255ca 100644 --- a/Lib/concurrent/futures/process.py +++ b/Lib/concurrent/futures/process.py @@ -210,9 +210,13 @@ def _sendback_result(result_queue, work_id, result=None, exception=None, result_queue.put(_ResultItem(work_id, result=result, exception=exception, exit_pid=exit_pid)) except BaseException as e: - exc = _ExceptionWithTraceback(e, e.__traceback__) - result_queue.put(_ResultItem(work_id, exception=exc, - exit_pid=exit_pid)) + result_queue.put( + _ResultItem( + work_id, + exception=_ExceptionWithTraceback(e, e.__traceback__), + exit_pid=exit_pid + ) + ) def _process_worker(call_queue, result_queue, initializer, initargs, max_tasks=None): From 09b08197db97ad31e23289e7cbb920e41c8706af Mon Sep 17 00:00:00 2001 From: ebonnal Date: Mon, 31 Mar 2025 11:43:47 +0100 Subject: [PATCH 18/41] concurrent.futures.process: avoid ref cycle in _process_worker --- Lib/concurrent/futures/process.py | 9 ++++++--- 1 file changed, 6 insertions(+), 3 deletions(-) diff --git a/Lib/concurrent/futures/process.py b/Lib/concurrent/futures/process.py index e6e7f304f255ca..e8d701de17855e 100644 --- a/Lib/concurrent/futures/process.py +++ b/Lib/concurrent/futures/process.py @@ -257,9 +257,12 @@ def _process_worker(call_queue, result_queue, initializer, initargs, max_tasks=N try: r = call_item.fn(*call_item.args, **call_item.kwargs) except BaseException as e: - exc = _ExceptionWithTraceback(e, e.__traceback__) - _sendback_result(result_queue, call_item.work_id, exception=exc, - exit_pid=exit_pid) + _sendback_result( + result_queue, + call_item.work_id, + exception=_ExceptionWithTraceback(e, e.__traceback__), + exit_pid=exit_pid, + ) else: _sendback_result(result_queue, call_item.work_id, result=r, exit_pid=exit_pid) From 4fab860f21bad3615b665723f0a1f43d77ea45f9 Mon Sep 17 00:00:00 2001 From: ebonnal Date: Mon, 31 Mar 2025 12:27:17 +0100 Subject: [PATCH 19/41] format test msg --- Lib/test/test_concurrent_futures/executor.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/Lib/test/test_concurrent_futures/executor.py b/Lib/test/test_concurrent_futures/executor.py index 47bfda9096b560..807b2938fa21d9 100644 --- a/Lib/test/test_concurrent_futures/executor.py +++ b/Lib/test/test_concurrent_futures/executor.py @@ -69,7 +69,7 @@ def test_map_exception(self): self.assertFalse( gc.get_referrers(error), - msg="the raised error should not have any referrers", + msg="the exception should not have any referrers", ) tb = error.__traceback__ @@ -81,7 +81,7 @@ def test_map_exception(self): if isinstance(val, Exception) and var in val.__traceback__.tb_frame.f_locals }, - msg=f"the raised exception's traceback should not contain an exception that captures itself in its own traceback", + msg=f"the exception's traceback should not contain an exception that captures itself in its own traceback", ) @support.requires_resource('walltime') From f5135abb6145d511202cd866f5637e9e0182217e Mon Sep 17 00:00:00 2001 From: ebonnal Date: Mon, 31 Mar 2025 12:30:04 +0100 Subject: [PATCH 20/41] refactor ZeroDivisionError assert --- Lib/test/test_concurrent_futures/executor.py | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/Lib/test/test_concurrent_futures/executor.py b/Lib/test/test_concurrent_futures/executor.py index 807b2938fa21d9..32aae3f40939b2 100644 --- a/Lib/test/test_concurrent_futures/executor.py +++ b/Lib/test/test_concurrent_futures/executor.py @@ -60,11 +60,11 @@ def test_map_exception(self): error = None try: next(i) - except ZeroDivisionError as zero_div_error: - error = zero_div_error - self.assertIsNotNone( - error, - msg="next one should raise a ZeroDivisionError", + except Exception as e: + error = e + self.assertTrue( + isinstance(error, ZeroDivisionError), + msg="next should raise a ZeroDivisionError", ) self.assertFalse( From 855c427bb7cbdbdb13f8a53f524dd80a02b74ef0 Mon Sep 17 00:00:00 2001 From: ebonnal Date: Mon, 31 Mar 2025 12:49:22 +0100 Subject: [PATCH 21/41] Revert "concurrent.futures.process: avoid ref cycle in _process_worker" This reverts commit 09b08197db97ad31e23289e7cbb920e41c8706af. --- Lib/concurrent/futures/process.py | 9 +++------ 1 file changed, 3 insertions(+), 6 deletions(-) diff --git a/Lib/concurrent/futures/process.py b/Lib/concurrent/futures/process.py index e8d701de17855e..e6e7f304f255ca 100644 --- a/Lib/concurrent/futures/process.py +++ b/Lib/concurrent/futures/process.py @@ -257,12 +257,9 @@ def _process_worker(call_queue, result_queue, initializer, initargs, max_tasks=N try: r = call_item.fn(*call_item.args, **call_item.kwargs) except BaseException as e: - _sendback_result( - result_queue, - call_item.work_id, - exception=_ExceptionWithTraceback(e, e.__traceback__), - exit_pid=exit_pid, - ) + exc = _ExceptionWithTraceback(e, e.__traceback__) + _sendback_result(result_queue, call_item.work_id, exception=exc, + exit_pid=exit_pid) else: _sendback_result(result_queue, call_item.work_id, result=r, exit_pid=exit_pid) From 4504516459ecd5e1600a12db51b123ef9a789a4e Mon Sep 17 00:00:00 2001 From: ebonnal Date: Mon, 31 Mar 2025 12:49:37 +0100 Subject: [PATCH 22/41] Revert "concurrent.futures.process: avoid ref cycle in _sendback_result" This reverts commit ef301226c144bc92c64f3bbc5fb78a8c6177d298. --- Lib/concurrent/futures/process.py | 10 +++------- 1 file changed, 3 insertions(+), 7 deletions(-) diff --git a/Lib/concurrent/futures/process.py b/Lib/concurrent/futures/process.py index e6e7f304f255ca..4847550908adab 100644 --- a/Lib/concurrent/futures/process.py +++ b/Lib/concurrent/futures/process.py @@ -210,13 +210,9 @@ def _sendback_result(result_queue, work_id, result=None, exception=None, result_queue.put(_ResultItem(work_id, result=result, exception=exception, exit_pid=exit_pid)) except BaseException as e: - result_queue.put( - _ResultItem( - work_id, - exception=_ExceptionWithTraceback(e, e.__traceback__), - exit_pid=exit_pid - ) - ) + exc = _ExceptionWithTraceback(e, e.__traceback__) + result_queue.put(_ResultItem(work_id, exception=exc, + exit_pid=exit_pid)) def _process_worker(call_queue, result_queue, initializer, initargs, max_tasks=None): From fc7a569a71d53722f40b31a61018541847aa976b Mon Sep 17 00:00:00 2001 From: ebonnal Date: Tue, 1 Apr 2025 00:01:37 +0100 Subject: [PATCH 23/41] skip referrers test for free-threading build on win/linux --- Lib/test/test_concurrent_futures/executor.py | 13 +++++++++---- 1 file changed, 9 insertions(+), 4 deletions(-) diff --git a/Lib/test/test_concurrent_futures/executor.py b/Lib/test/test_concurrent_futures/executor.py index 32aae3f40939b2..699bdffcabd13f 100644 --- a/Lib/test/test_concurrent_futures/executor.py +++ b/Lib/test/test_concurrent_futures/executor.py @@ -1,10 +1,12 @@ import gc import itertools +import sys import threading import time import weakref from concurrent import futures from operator import add +from sysconfig import get_config_var from test import support from test.support import Py_GIL_DISABLED @@ -67,10 +69,13 @@ def test_map_exception(self): msg="next should raise a ZeroDivisionError", ) - self.assertFalse( - gc.get_referrers(error), - msg="the exception should not have any referrers", - ) + # some referrers may remain for free-threading build on Windows/Linux + is_free_threading = '--disable-gil' in get_config_var("CONFIG_ARGS") + if not is_free_threading or sys.platform not in ("linux", "win32"): + self.assertFalse( + gc.get_referrers(error), + msg="the exception should not have any referrers", + ) tb = error.__traceback__ while (tb := tb.tb_next): From cae64747fa2954c8cf868695beb094ebee40032c Mon Sep 17 00:00:00 2001 From: ebonnal Date: Tue, 1 Apr 2025 00:16:11 +0100 Subject: [PATCH 24/41] call gc.collect for free-threading on linux/windows --- Lib/test/test_concurrent_futures/executor.py | 18 +++++++++++------- 1 file changed, 11 insertions(+), 7 deletions(-) diff --git a/Lib/test/test_concurrent_futures/executor.py b/Lib/test/test_concurrent_futures/executor.py index 699bdffcabd13f..639461891787d6 100644 --- a/Lib/test/test_concurrent_futures/executor.py +++ b/Lib/test/test_concurrent_futures/executor.py @@ -69,13 +69,17 @@ def test_map_exception(self): msg="next should raise a ZeroDivisionError", ) - # some referrers may remain for free-threading build on Windows/Linux - is_free_threading = '--disable-gil' in get_config_var("CONFIG_ARGS") - if not is_free_threading or sys.platform not in ("linux", "win32"): - self.assertFalse( - gc.get_referrers(error), - msg="the exception should not have any referrers", - ) + # a GC collection may be necessary for free-threading build on Windows/Linux + if ( + '--disable-gil' in get_config_var("CONFIG_ARGS") + and sys.platform in ("linux", "win32") + ): + gc.collect() + + self.assertFalse( + gc.get_referrers(error), + msg="the exception should not have any referrers", + ) tb = error.__traceback__ while (tb := tb.tb_next): From 4abb239e5b1e9b8f75eb3b263a277ac0d98b05ce Mon Sep 17 00:00:00 2001 From: ebonnal Date: Tue, 1 Apr 2025 11:52:20 +0100 Subject: [PATCH 25/41] Revert "call gc.collect for free-threading on linux/windows" This reverts commit cae64747fa2954c8cf868695beb094ebee40032c. --- Lib/test/test_concurrent_futures/executor.py | 18 +++++++----------- 1 file changed, 7 insertions(+), 11 deletions(-) diff --git a/Lib/test/test_concurrent_futures/executor.py b/Lib/test/test_concurrent_futures/executor.py index 639461891787d6..699bdffcabd13f 100644 --- a/Lib/test/test_concurrent_futures/executor.py +++ b/Lib/test/test_concurrent_futures/executor.py @@ -69,17 +69,13 @@ def test_map_exception(self): msg="next should raise a ZeroDivisionError", ) - # a GC collection may be necessary for free-threading build on Windows/Linux - if ( - '--disable-gil' in get_config_var("CONFIG_ARGS") - and sys.platform in ("linux", "win32") - ): - gc.collect() - - self.assertFalse( - gc.get_referrers(error), - msg="the exception should not have any referrers", - ) + # some referrers may remain for free-threading build on Windows/Linux + is_free_threading = '--disable-gil' in get_config_var("CONFIG_ARGS") + if not is_free_threading or sys.platform not in ("linux", "win32"): + self.assertFalse( + gc.get_referrers(error), + msg="the exception should not have any referrers", + ) tb = error.__traceback__ while (tb := tb.tb_next): From 4bc07c082f92c34f475fc38fa08eb2cba811ed62 Mon Sep 17 00:00:00 2001 From: ebonnal Date: Tue, 1 Apr 2025 11:52:30 +0100 Subject: [PATCH 26/41] Revert "skip referrers test for free-threading build on win/linux" This reverts commit fc7a569a71d53722f40b31a61018541847aa976b. --- Lib/test/test_concurrent_futures/executor.py | 13 ++++--------- 1 file changed, 4 insertions(+), 9 deletions(-) diff --git a/Lib/test/test_concurrent_futures/executor.py b/Lib/test/test_concurrent_futures/executor.py index 699bdffcabd13f..32aae3f40939b2 100644 --- a/Lib/test/test_concurrent_futures/executor.py +++ b/Lib/test/test_concurrent_futures/executor.py @@ -1,12 +1,10 @@ import gc import itertools -import sys import threading import time import weakref from concurrent import futures from operator import add -from sysconfig import get_config_var from test import support from test.support import Py_GIL_DISABLED @@ -69,13 +67,10 @@ def test_map_exception(self): msg="next should raise a ZeroDivisionError", ) - # some referrers may remain for free-threading build on Windows/Linux - is_free_threading = '--disable-gil' in get_config_var("CONFIG_ARGS") - if not is_free_threading or sys.platform not in ("linux", "win32"): - self.assertFalse( - gc.get_referrers(error), - msg="the exception should not have any referrers", - ) + self.assertFalse( + gc.get_referrers(error), + msg="the exception should not have any referrers", + ) tb = error.__traceback__ while (tb := tb.tb_next): From 3b00c5ffc5640d65089349e8ad02cf152a748b52 Mon Sep 17 00:00:00 2001 From: Thomas Grainger Date: Thu, 3 Apr 2025 08:40:08 +0100 Subject: [PATCH 27/41] more gc referrers --- Lib/test/test_concurrent_futures/executor.py | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/Lib/test/test_concurrent_futures/executor.py b/Lib/test/test_concurrent_futures/executor.py index 32aae3f40939b2..611ba62a1edc22 100644 --- a/Lib/test/test_concurrent_futures/executor.py +++ b/Lib/test/test_concurrent_futures/executor.py @@ -6,7 +6,7 @@ from concurrent import futures from operator import add from test import support -from test.support import Py_GIL_DISABLED +from test.support import Py_GIL_DISABLED, disable_gc def mul(x, y): @@ -52,6 +52,7 @@ def test_map(self): list(self.executor.map(pow, range(10), range(10), chunksize=3)), list(map(pow, range(10), range(10)))) + @disable_gc() def test_map_exception(self): i = self.executor.map(divmod, [1, 1, 1, 1], [2, 3, 0, 5]) self.assertEqual(i.__next__(), (0, 1)) @@ -67,8 +68,9 @@ def test_map_exception(self): msg="next should raise a ZeroDivisionError", ) + # TODO: remove only for debugging self.assertFalse( - gc.get_referrers(error), + [gc.get_referrers(x) for x in gc.get_referrers(error)], msg="the exception should not have any referrers", ) From 1948eb3526e2a5bebd3acfcaa358c35dfb46f6b7 Mon Sep 17 00:00:00 2001 From: Thomas Grainger Date: Thu, 3 Apr 2025 09:00:02 +0100 Subject: [PATCH 28/41] even more gc referrers --- Lib/test/test_concurrent_futures/executor.py | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/Lib/test/test_concurrent_futures/executor.py b/Lib/test/test_concurrent_futures/executor.py index 611ba62a1edc22..251585b45ae97d 100644 --- a/Lib/test/test_concurrent_futures/executor.py +++ b/Lib/test/test_concurrent_futures/executor.py @@ -70,7 +70,11 @@ def test_map_exception(self): # TODO: remove only for debugging self.assertFalse( - [gc.get_referrers(x) for x in gc.get_referrers(error)], + [ + gc.get_referrers(y) + for x in gc.get_referrers(error) + for y in gc.get_referrers(x) + ], msg="the exception should not have any referrers", ) From 91849646fafc0c7fa42dba1ac80936f88a13cfd0 Mon Sep 17 00:00:00 2001 From: Thomas Grainger Date: Fri, 4 Apr 2025 07:56:24 +0100 Subject: [PATCH 29/41] delete a ref to work_item --- Lib/concurrent/futures/process.py | 21 ++++++++++++--------- 1 file changed, 12 insertions(+), 9 deletions(-) diff --git a/Lib/concurrent/futures/process.py b/Lib/concurrent/futures/process.py index 4847550908adab..6fef6abf7923e5 100644 --- a/Lib/concurrent/futures/process.py +++ b/Lib/concurrent/futures/process.py @@ -392,15 +392,18 @@ def add_call_item_to_queue(self): else: work_item = self.pending_work_items[work_id] - if work_item.future.set_running_or_notify_cancel(): - self.call_queue.put(_CallItem(work_id, - work_item.fn, - work_item.args, - work_item.kwargs), - block=True) - else: - del self.pending_work_items[work_id] - continue + try: + if work_item.future.set_running_or_notify_cancel(): + self.call_queue.put(_CallItem(work_id, + work_item.fn, + work_item.args, + work_item.kwargs), + block=True) + else: + del self.pending_work_items[work_id] + continue + finally: + del work_item def wait_result_broken_or_wakeup(self): # Wait for a result to be ready in the result_queue while checking From 7e5300cc618eaeab6e4f3f7d2f73b47fac109eef Mon Sep 17 00:00:00 2001 From: Thomas Grainger Date: Fri, 4 Apr 2025 08:24:08 +0100 Subject: [PATCH 30/41] delete reference to result_item before joining process --- Lib/concurrent/futures/process.py | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/Lib/concurrent/futures/process.py b/Lib/concurrent/futures/process.py index 6fef6abf7923e5..d6adf407c177f7 100644 --- a/Lib/concurrent/futures/process.py +++ b/Lib/concurrent/futures/process.py @@ -347,16 +347,16 @@ def run(self): return if result_item is not None: self.process_result_item(result_item) - - process_exited = result_item.exit_pid is not None - if process_exited: - p = self.processes.pop(result_item.exit_pid) - p.join() + exit_pid = result_item.exit_pid # Delete reference to result_item to avoid keeping references # while waiting on new results. del result_item + process_exited = exit_pid is not None + if process_exited: + self.processes.pop(exit_pid).join() + if executor := self.executor_reference(): if process_exited: with self.shutdown_lock: From c5e39ee5c73a48299d251ec73bc3b535803030f2 Mon Sep 17 00:00:00 2001 From: Thomas Grainger Date: Fri, 4 Apr 2025 08:45:02 +0100 Subject: [PATCH 31/41] unpack ResultItem and WorkItem --- Lib/concurrent/futures/process.py | 36 +++++++++++++++++-------------- 1 file changed, 20 insertions(+), 16 deletions(-) diff --git a/Lib/concurrent/futures/process.py b/Lib/concurrent/futures/process.py index d6adf407c177f7..968abefacd762d 100644 --- a/Lib/concurrent/futures/process.py +++ b/Lib/concurrent/futures/process.py @@ -346,12 +346,29 @@ def run(self): self.terminate_broken(cause) return if result_item is not None: - self.process_result_item(result_item) exit_pid = result_item.exit_pid + work_id = result_item.work_id + exception = result_item.exeption + result = result_item.result + del result_item + # Process the received a result_item. This can be either the PID of a + # worker that exited gracefully or a _ResultItem + + # Received a _ResultItem so mark the future as completed. + work_item = self.pending_work_items.pop(work_id, None) + # work_item can be None if another process terminated (see above) + if work_item is not None: + f = work_item.future + del work_item + if exception: + f.set_exception(exception) + else: + f.set_result(result) + del f - # Delete reference to result_item to avoid keeping references + # Delete reference to exception/result to avoid keeping references # while waiting on new results. - del result_item + del exception, result process_exited = exit_pid is not None if process_exited: @@ -435,19 +452,6 @@ def wait_result_broken_or_wakeup(self): return result_item, is_broken, cause - def process_result_item(self, result_item): - # Process the received a result_item. This can be either the PID of a - # worker that exited gracefully or a _ResultItem - - # Received a _ResultItem so mark the future as completed. - work_item = self.pending_work_items.pop(result_item.work_id, None) - # work_item can be None if another process terminated (see above) - if work_item is not None: - if result_item.exception: - work_item.future.set_exception(result_item.exception) - else: - work_item.future.set_result(result_item.result) - def is_shutting_down(self): # Check whether we should start shutting down the executor. executor = self.executor_reference() From 548d517339dbebc077bcfac5c6c637253e2bcc2b Mon Sep 17 00:00:00 2001 From: Thomas Grainger Date: Sat, 5 Apr 2025 05:48:10 +0100 Subject: [PATCH 32/41] fix typo --- Lib/concurrent/futures/process.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/Lib/concurrent/futures/process.py b/Lib/concurrent/futures/process.py index 968abefacd762d..268aa221d85b67 100644 --- a/Lib/concurrent/futures/process.py +++ b/Lib/concurrent/futures/process.py @@ -348,7 +348,7 @@ def run(self): if result_item is not None: exit_pid = result_item.exit_pid work_id = result_item.work_id - exception = result_item.exeption + exception = result_item.exception result = result_item.result del result_item # Process the received a result_item. This can be either the PID of a From fd9a89e4f670806c40cea3d43d43f8b27f95047a Mon Sep 17 00:00:00 2001 From: Thomas Grainger Date: Sat, 5 Apr 2025 09:30:32 +0100 Subject: [PATCH 33/41] bit of a hack? --- Lib/concurrent/futures/process.py | 7 +++---- 1 file changed, 3 insertions(+), 4 deletions(-) diff --git a/Lib/concurrent/futures/process.py b/Lib/concurrent/futures/process.py index 268aa221d85b67..865d7a08bd2d99 100644 --- a/Lib/concurrent/futures/process.py +++ b/Lib/concurrent/futures/process.py @@ -358,13 +358,12 @@ def run(self): work_item = self.pending_work_items.pop(work_id, None) # work_item can be None if another process terminated (see above) if work_item is not None: - f = work_item.future + f_boxed = [work_item.future] del work_item if exception: - f.set_exception(exception) + f_boxed.pop().set_exception(exception) else: - f.set_result(result) - del f + f_boxed.pop().set_result(result) # Delete reference to exception/result to avoid keeping references # while waiting on new results. From dbaa0448d0cc97a4a92824f812b1d5cf3a52d833 Mon Sep 17 00:00:00 2001 From: ebonnal Date: Mon, 7 Apr 2025 11:52:28 +0100 Subject: [PATCH 34/41] Revert investigations --- Lib/concurrent/futures/process.py | 66 +++++++++----------- Lib/test/test_concurrent_futures/executor.py | 10 +-- 2 files changed, 32 insertions(+), 44 deletions(-) diff --git a/Lib/concurrent/futures/process.py b/Lib/concurrent/futures/process.py index 865d7a08bd2d99..4847550908adab 100644 --- a/Lib/concurrent/futures/process.py +++ b/Lib/concurrent/futures/process.py @@ -346,32 +346,16 @@ def run(self): self.terminate_broken(cause) return if result_item is not None: - exit_pid = result_item.exit_pid - work_id = result_item.work_id - exception = result_item.exception - result = result_item.result - del result_item - # Process the received a result_item. This can be either the PID of a - # worker that exited gracefully or a _ResultItem - - # Received a _ResultItem so mark the future as completed. - work_item = self.pending_work_items.pop(work_id, None) - # work_item can be None if another process terminated (see above) - if work_item is not None: - f_boxed = [work_item.future] - del work_item - if exception: - f_boxed.pop().set_exception(exception) - else: - f_boxed.pop().set_result(result) - - # Delete reference to exception/result to avoid keeping references - # while waiting on new results. - del exception, result + self.process_result_item(result_item) - process_exited = exit_pid is not None + process_exited = result_item.exit_pid is not None if process_exited: - self.processes.pop(exit_pid).join() + p = self.processes.pop(result_item.exit_pid) + p.join() + + # Delete reference to result_item to avoid keeping references + # while waiting on new results. + del result_item if executor := self.executor_reference(): if process_exited: @@ -408,18 +392,15 @@ def add_call_item_to_queue(self): else: work_item = self.pending_work_items[work_id] - try: - if work_item.future.set_running_or_notify_cancel(): - self.call_queue.put(_CallItem(work_id, - work_item.fn, - work_item.args, - work_item.kwargs), - block=True) - else: - del self.pending_work_items[work_id] - continue - finally: - del work_item + if work_item.future.set_running_or_notify_cancel(): + self.call_queue.put(_CallItem(work_id, + work_item.fn, + work_item.args, + work_item.kwargs), + block=True) + else: + del self.pending_work_items[work_id] + continue def wait_result_broken_or_wakeup(self): # Wait for a result to be ready in the result_queue while checking @@ -451,6 +432,19 @@ def wait_result_broken_or_wakeup(self): return result_item, is_broken, cause + def process_result_item(self, result_item): + # Process the received a result_item. This can be either the PID of a + # worker that exited gracefully or a _ResultItem + + # Received a _ResultItem so mark the future as completed. + work_item = self.pending_work_items.pop(result_item.work_id, None) + # work_item can be None if another process terminated (see above) + if work_item is not None: + if result_item.exception: + work_item.future.set_exception(result_item.exception) + else: + work_item.future.set_result(result_item.result) + def is_shutting_down(self): # Check whether we should start shutting down the executor. executor = self.executor_reference() diff --git a/Lib/test/test_concurrent_futures/executor.py b/Lib/test/test_concurrent_futures/executor.py index 251585b45ae97d..32aae3f40939b2 100644 --- a/Lib/test/test_concurrent_futures/executor.py +++ b/Lib/test/test_concurrent_futures/executor.py @@ -6,7 +6,7 @@ from concurrent import futures from operator import add from test import support -from test.support import Py_GIL_DISABLED, disable_gc +from test.support import Py_GIL_DISABLED def mul(x, y): @@ -52,7 +52,6 @@ def test_map(self): list(self.executor.map(pow, range(10), range(10), chunksize=3)), list(map(pow, range(10), range(10)))) - @disable_gc() def test_map_exception(self): i = self.executor.map(divmod, [1, 1, 1, 1], [2, 3, 0, 5]) self.assertEqual(i.__next__(), (0, 1)) @@ -68,13 +67,8 @@ def test_map_exception(self): msg="next should raise a ZeroDivisionError", ) - # TODO: remove only for debugging self.assertFalse( - [ - gc.get_referrers(y) - for x in gc.get_referrers(error) - for y in gc.get_referrers(x) - ], + gc.get_referrers(error), msg="the exception should not have any referrers", ) From 869e4b750255cd2a9cc145d2e2e7718c0a202afe Mon Sep 17 00:00:00 2001 From: ebonnal Date: Mon, 7 Apr 2025 12:00:15 +0100 Subject: [PATCH 35/41] pause before checking referrers --- Lib/test/test_concurrent_futures/executor.py | 2 ++ 1 file changed, 2 insertions(+) diff --git a/Lib/test/test_concurrent_futures/executor.py b/Lib/test/test_concurrent_futures/executor.py index 32aae3f40939b2..ac10118454d504 100644 --- a/Lib/test/test_concurrent_futures/executor.py +++ b/Lib/test/test_concurrent_futures/executor.py @@ -67,6 +67,8 @@ def test_map_exception(self): msg="next should raise a ZeroDivisionError", ) + # free-threading builds need this pause on Ubuntu (ARM) and Windows + time.sleep(1) self.assertFalse( gc.get_referrers(error), msg="the exception should not have any referrers", From 1ee9bf4d089bdcd3ee5e064420e2dec7a2e627e0 Mon Sep 17 00:00:00 2001 From: ebonnal Date: Mon, 7 Apr 2025 14:23:39 +0100 Subject: [PATCH 36/41] add comment about the avoided ref cycle with exc_wrapper --- Lib/concurrent/futures/interpreter.py | 1 + Modules/_opcode-488016f1.o.tmp | 0 2 files changed, 1 insertion(+) create mode 100644 Modules/_opcode-488016f1.o.tmp diff --git a/Lib/concurrent/futures/interpreter.py b/Lib/concurrent/futures/interpreter.py index 1c1187272a26ed..030c36ec9b31f1 100644 --- a/Lib/concurrent/futures/interpreter.py +++ b/Lib/concurrent/futures/interpreter.py @@ -208,6 +208,7 @@ def run(self, task): try: raise pickle.loads(excdata) from exc_wrapper finally: + # avoid a ref cycle where exc_wrapper is captured in its traceback exc_wrapper = None return pickle.loads(res) if pickled else res diff --git a/Modules/_opcode-488016f1.o.tmp b/Modules/_opcode-488016f1.o.tmp new file mode 100644 index 00000000000000..e69de29bb2d1d6 From 95765f678582d668120fd662e1bae74e999f1681 Mon Sep 17 00:00:00 2001 From: ebonnal Date: Mon, 7 Apr 2025 14:27:24 +0100 Subject: [PATCH 37/41] format test --- Lib/test/test_concurrent_futures/executor.py | 11 ++++++----- Modules/_opcode-488016f1.o.tmp | 0 2 files changed, 6 insertions(+), 5 deletions(-) delete mode 100644 Modules/_opcode-488016f1.o.tmp diff --git a/Lib/test/test_concurrent_futures/executor.py b/Lib/test/test_concurrent_futures/executor.py index ac10118454d504..4d59c49e69fa1a 100644 --- a/Lib/test/test_concurrent_futures/executor.py +++ b/Lib/test/test_concurrent_futures/executor.py @@ -57,24 +57,25 @@ def test_map_exception(self): self.assertEqual(i.__next__(), (0, 1)) self.assertEqual(i.__next__(), (0, 1)) - error = None + exception = None try: next(i) except Exception as e: - error = e + exception = e self.assertTrue( - isinstance(error, ZeroDivisionError), + isinstance(exception, ZeroDivisionError), msg="next should raise a ZeroDivisionError", ) # free-threading builds need this pause on Ubuntu (ARM) and Windows time.sleep(1) + self.assertFalse( - gc.get_referrers(error), + gc.get_referrers(exception), msg="the exception should not have any referrers", ) - tb = error.__traceback__ + tb = exception.__traceback__ while (tb := tb.tb_next): self.assertFalse( { diff --git a/Modules/_opcode-488016f1.o.tmp b/Modules/_opcode-488016f1.o.tmp deleted file mode 100644 index e69de29bb2d1d6..00000000000000 From a5c6b17656dbb0332847640e61603935e95e2ed5 Mon Sep 17 00:00:00 2001 From: ebonnal Date: Mon, 7 Apr 2025 18:01:48 +0100 Subject: [PATCH 38/41] make the self-captured exceptions check not rely on var name --- Lib/test/test_concurrent_futures/executor.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/Lib/test/test_concurrent_futures/executor.py b/Lib/test/test_concurrent_futures/executor.py index 4d59c49e69fa1a..82fb5d9c455663 100644 --- a/Lib/test/test_concurrent_futures/executor.py +++ b/Lib/test/test_concurrent_futures/executor.py @@ -82,7 +82,7 @@ def test_map_exception(self): var: val for var, val in tb.tb_frame.f_locals.items() if isinstance(val, Exception) - and var in val.__traceback__.tb_frame.f_locals + and val.__traceback__.tb_frame is tb.tb_frame }, msg=f"the exception's traceback should not contain an exception that captures itself in its own traceback", ) From 0f8748c7dfb5230cce0958846046af74406559e4 Mon Sep 17 00:00:00 2001 From: ebonnal Date: Mon, 7 Apr 2025 20:02:29 +0100 Subject: [PATCH 39/41] use traceback.walk_tb --- Lib/test/test_concurrent_futures/executor.py | 26 +++++++++++--------- 1 file changed, 14 insertions(+), 12 deletions(-) diff --git a/Lib/test/test_concurrent_futures/executor.py b/Lib/test/test_concurrent_futures/executor.py index 82fb5d9c455663..4b15f99dcbc1a8 100644 --- a/Lib/test/test_concurrent_futures/executor.py +++ b/Lib/test/test_concurrent_futures/executor.py @@ -2,9 +2,10 @@ import itertools import threading import time +import traceback import weakref from concurrent import futures -from operator import add +from operator import add, itemgetter from test import support from test.support import Py_GIL_DISABLED @@ -75,17 +76,18 @@ def test_map_exception(self): msg="the exception should not have any referrers", ) - tb = exception.__traceback__ - while (tb := tb.tb_next): - self.assertFalse( - { - var: val - for var, val in tb.tb_frame.f_locals.items() - if isinstance(val, Exception) - and val.__traceback__.tb_frame is tb.tb_frame - }, - msg=f"the exception's traceback should not contain an exception that captures itself in its own traceback", - ) + frames = map(itemgetter(0), traceback.walk_tb(exception.__traceback__)) + next(frames) + self.assertFalse( + [ + (var, val) + for frame in frames + for var, val in frame.f_locals.items() + if isinstance(val, Exception) + and frame in map(itemgetter(0), traceback.walk_tb(val.__traceback__)) + ], + msg=f"the exception's traceback should not contain an exception that captures itself in its own traceback", + ) @support.requires_resource('walltime') def test_map_timeout(self): From 85a3c369fba78367b76b92c46b518d4e623ab8d4 Mon Sep 17 00:00:00 2001 From: ebonnal Date: Mon, 7 Apr 2025 20:20:48 +0100 Subject: [PATCH 40/41] comment the skipping of the current frame --- Lib/test/test_concurrent_futures/executor.py | 3 +++ 1 file changed, 3 insertions(+) diff --git a/Lib/test/test_concurrent_futures/executor.py b/Lib/test/test_concurrent_futures/executor.py index 4b15f99dcbc1a8..f348ef998812ad 100644 --- a/Lib/test/test_concurrent_futures/executor.py +++ b/Lib/test/test_concurrent_futures/executor.py @@ -77,7 +77,10 @@ def test_map_exception(self): ) frames = map(itemgetter(0), traceback.walk_tb(exception.__traceback__)) + + # skip current frame next(frames) + self.assertFalse( [ (var, val) From 82542f398da251e3d3fd9fad0ba8b93a1a92b92f Mon Sep 17 00:00:00 2001 From: ebonnal Date: Wed, 23 Apr 2025 13:13:16 +0100 Subject: [PATCH 41/41] test_map_exception: improve readability --- Lib/test/test_concurrent_futures/executor.py | 25 ++++++++++---------- 1 file changed, 13 insertions(+), 12 deletions(-) diff --git a/Lib/test/test_concurrent_futures/executor.py b/Lib/test/test_concurrent_futures/executor.py index f348ef998812ad..6cdd8f4fd61537 100644 --- a/Lib/test/test_concurrent_futures/executor.py +++ b/Lib/test/test_concurrent_futures/executor.py @@ -5,7 +5,7 @@ import traceback import weakref from concurrent import futures -from operator import add, itemgetter +from operator import add from test import support from test.support import Py_GIL_DISABLED @@ -60,15 +60,15 @@ def test_map_exception(self): exception = None try: - next(i) + i.__next__() except Exception as e: exception = e self.assertTrue( isinstance(exception, ZeroDivisionError), - msg="next should raise a ZeroDivisionError", + msg="should raise a ZeroDivisionError", ) - # free-threading builds need this pause on Ubuntu (ARM) and Windows + # pause needed for free-threading builds on Ubuntu (ARM) and Windows time.sleep(1) self.assertFalse( @@ -76,20 +76,21 @@ def test_map_exception(self): msg="the exception should not have any referrers", ) - frames = map(itemgetter(0), traceback.walk_tb(exception.__traceback__)) - - # skip current frame - next(frames) - self.assertFalse( [ (var, val) - for frame in frames + # go through the frames of the exception's traceback + for frame, _ in traceback.walk_tb(exception.__traceback__) + # skipping the current frame + if frame is not exception.__traceback__.tb_frame + # go through the locals captured in that frame for var, val in frame.f_locals.items() + # check if one of them is an exception if isinstance(val, Exception) - and frame in map(itemgetter(0), traceback.walk_tb(val.__traceback__)) + # check if it is captured in its own traceback + and frame is val.__traceback__.tb_frame ], - msg=f"the exception's traceback should not contain an exception that captures itself in its own traceback", + msg=f"the exception's traceback should not contain an exception captured in its own traceback", ) @support.requires_resource('walltime')