Uh oh!
There was an error while loading. Please reload this page.
- Notifications
You must be signed in to change notification settings - Fork 34k
gh-74028: concurrent.futures.Executor.map: avoid reference cycles when an exception is raised#131701
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base:main
Are you sure you want to change the base?
gh-74028: concurrent.futures.Executor.map: avoid reference cycles when an exception is raised #131701
Changes from all commits
7dc850b19edced1c3d01afa790f1e48d0e099850b86e7801448ed15df59c7493dc24614dce2809bc69b99c781c1a438301ada6140d5e8c7ac4b771a03f8ab4ef3012209b08194fab8600714013f5135ab855c4274504516fc7a569cae64744abb2394bc07c07b7a5e63b00c5f1948eb337b7cb391849647e5300cc5e39ee548d517fac7e89fd9a89edbaa044869e4b7a40b9b81ee9bf495765f69020971a5c6b170f8748c85a3c3682542f3File filter
Filter by extension
Conversations
Uh oh!
There was an error while loading. Please reload this page.
Jump to
Uh oh!
There was an error while loading. Please reload this page.
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -205,8 +205,11 @@ 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 | ||
| try: | ||
| raise pickle.loads(excdata) from exc_wrapper | ||
ebonnal marked this conversation as resolved. Show resolvedHide resolvedUh oh!There was an error while loading. Please reload this page. | ||
| finally: | ||
| # avoid a ref cycle where exc_wrapper is captured in its traceback | ||
| exc_wrapper = None | ||
| return pickle.loads(res) if pickled else res | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -1,6 +1,8 @@ | ||
| import gc | ||
| import itertools | ||
| import threading | ||
| import time | ||
| import traceback | ||
| import weakref | ||
| from concurrent import futures | ||
| from operator import add | ||
| @@ -55,8 +57,41 @@ 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): | ||
Contributor There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This test looks fragile to me, +1 to remove test altogether, the fix is straightforward enough | ||
| exception = None | ||
| try: | ||
| i.__next__() | ||
| except Exception as e: | ||
| exception = e | ||
| self.assertTrue( | ||
| isinstance(exception, ZeroDivisionError), | ||
| msg="should raise a ZeroDivisionError", | ||
| ) | ||
| # pause needed for free-threading builds on Ubuntu (ARM) and Windows | ||
| time.sleep(1) | ||
| self.assertFalse( | ||
| gc.get_referrers(exception), | ||
| msg="the exception should not have any referrers", | ||
| ) | ||
| self.assertFalse( | ||
| [ | ||
| (var, val) | ||
| # 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) | ||
| # 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 captured in its own traceback", | ||
| ) | ||
| @support.requires_resource('walltime') | ||
| def test_map_timeout(self): | ||
| @@ -140,7 +175,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): | ||
Uh oh!
There was an error while loading. Please reload this page.
Uh oh!
There was an error while loading. Please reload this page.