gh-74028: `concurrent.futures.Executor.map`: avoid reference cycles when an exception is raised by ebonnal · Pull Request #131701 · python/cpython · GitHub

A failed future stores the raised exception in its ._exception attribute. We get a reference cycle if this exception's __traceback__ refers back to the future.

A particular care has been taken to avoid that (e.g. by @graingert in #95169)

This PR tests this behavior.

(follows up discussion in #131467)

Edit:
We extend this PR to ensure that no reference to an Exception is captured in the error's traceback frames.

Issue: Make Executor.map work with infinite/large inputs correctly #74028

…le from failed future captured in its exception’s traceback

only failing for Ubuntu (free-threading) / build and test (ubuntu-24.04-arm) and I cannot reproduce on my ubuntu arm VM

Edit: actually I did reproduce it after a couple attempts

…f-cycles

@ebonnal thanks for the test! Your first test was good, and it found a bug https://github.com/python/cpython/actions/runs/14048391667/job/39333996238?pr=131701#step:22:632

I've pushed a commit to revert to the original style of test - which includes the references in the error message, and fixed the bug.

… to an Exception or a failed Future

I've pushed a commit to revert to the original style of test - which includes the references in the error message, and fixed the bug.

Thanks @graingert !

probably we also need to finally: del exc_wrapper here too

Nice, I have cleaned up this exc_wrapper and added an additional test that goes through the error's traceback frames and search for any reference to an Exception.

(I have renamed the PR to show that we have extended its scope)

ok I can't repeat it locally, even disabling the GC - sorry for hijacking your PR but I think I need to test it in CI to repeat

Hey @graingert, update from my investigations with a free-threading build on an Ubuntu ARM machine:
It fails like 50% of the time, looking like some GC race condition, but the test is always successful if we pause for a bit with a time.sleep(1) before checking the referrers.
Maybe for the scope of this PR we should go back to the state of 03f8ab4 and just add this little pause?

I don't think it's a gc race condition as the problem still happens with the GC disabled. I think the problem is that the main thread is resumed before the background thread can delete the future.

I'm happy for you to revert back to 03f8ab4

…f-cycles

Clear, thanks for the help @graingert, I appreciate it! Merging this test will definitely help prevent future regressions (e.g. in #131467).

Should we open a follow up issue summarizing the behavior we have found with free-threading builds on some platforms? Happy to do it!

…f-cycles

ready for your final review fyi @graingert 🙏🏻

This test looks fragile to me, +1 to remove test altogether, the fix is straightforward enough

This PR is stale because it has been open for 30 days with no activity.

test_concurrent_futures.test_map_exception: assert no reference cyc…

7dc850b

…le from failed future captured in its exception’s traceback

bedevere-app Bot added the awaiting review label Mar 24, 2025

ebonnal mentioned this pull request Mar 24, 2025

gh-131466: concurrent.futures.Executor.map: avoid temporarily exceeding buffersize while collecting the next result #131467

Open

ebonnal changed the title test_concurrent_futures.test_map_exception: assert no reference cycle from failed future captured in its exception’s traceback test_concurrent_futures: Executor.map: assert no reference cycle from failed future captured in its exception’s traceback Mar 25, 2025

ebonnal changed the title test_concurrent_futures: Executor.map: assert no reference cycle from failed future captured in its exception’s traceback concurrent.futures.Executor.map test: assert no reference cycle from failed future captured in its exception’s traceback Mar 25, 2025

ebonnal changed the title concurrent.futures.Executor.map test: assert no reference cycle from failed future captured in its exception’s traceback concurrent.futures.Executor.map: test no reference cycle from failed future captured in its exception’s traceback Mar 25, 2025

ebonnal added 5 commits March 25, 2025 00:33

test there is no future in referrers

19edced

switch to assertTrue(all(...))

1c3d01a

add test msg

fa790f1

format msgs

e48d0e0

switch to assert list empty for a better assertion error message

99850b8

ebonnal added 2 commits March 26, 2025 22:49

edit comment

6e78014

Merge remote-tracking branch 'cpython/main' into test/executor-map-re…

48ed15d

…f-cycles

picnixz changed the title concurrent.futures.Executor.map: test no reference cycle from failed future captured in its exception’s traceback gh-74028: concurrent.futures.Executor.map: test no reference cycle from failed future captured in its exception’s traceback Mar 30, 2025

bedevere-app Bot mentioned this pull request Mar 30, 2025

Make Executor.map work with infinite/large inputs correctly #74028

Closed

picnixz added the skip news label Mar 30, 2025

picnixz requested a review from graingert March 30, 2025 11:36

graingert reviewed Mar 30, 2025

View reviewed changes

Comment thread Lib/test/test_concurrent_futures/executor.py Outdated

graingert added 3 commits March 30, 2025 13:39

Merge branch 'main' into test/executor-map-ref-cycles

f59c749

assert there are no reference cycles at all

3dc2461

avoid a reference cycle in subinterpreter pools

4dce280

graingert reviewed Mar 30, 2025

View reviewed changes

Comment thread Lib/concurrent/futures/interpreter.py

ebonnal changed the title gh-74028: concurrent.futures.Executor.map: test no reference cycle from failed future captured in its exception’s traceback gh-74028: concurrent.futures.Executor.map: test there are no reference cycles when an exception is raised Mar 30, 2025

ebonnal added 3 commits March 30, 2025 21:20

test that traceback frames should not contain any variables referring…

9bc69b9

… to an Exception or a failed Future

interpreter.WorkerContext.run: finally unassign exc_wrapper

9c781c1

fix msg grammar

a438301

fix access to exception

ada6140

more gc referrers

3b00c5f

graingert added 8 commits April 3, 2025 09:00

even more gc referrers

1948eb3

Merge branch 'main' into test/executor-map-ref-cycles

37b7cb3

delete a ref to work_item

9184964

delete reference to result_item before joining process

7e5300c

unpack ResultItem and WorkItem

c5e39ee

fix typo

548d517

Merge branch 'main' into test/executor-map-ref-cycles

fac7e89

bit of a hack?

fd9a89e

ebonnal added 3 commits April 7, 2025 12:08

Revert investigations

dbaa044

pause before checking referrers

869e4b7

Merge remote-tracking branch 'cpython/main' into test/executor-map-re…

a40b9b8

…f-cycles

ebonnal marked this pull request as ready for review April 7, 2025 12:45

bedevere-app Bot added the awaiting review label Apr 7, 2025

ebonnal added 6 commits April 7, 2025 14:23

add comment about the avoided ref cycle with exc_wrapper

1ee9bf4

format test

95765f6

Merge remote-tracking branch 'cpython/main' into test/executor-map-re…

9020971

…f-cycles

make the self-captured exceptions check not rely on var name

a5c6b17

use traceback.walk_tb

0f8748c

comment the skipping of the current frame

85a3c36

ebonnal changed the title gh-74028: concurrent.futures.Executor: avoid reference cycles when an exception is raised gh-74028: concurrent.futures.Executor.map: avoid reference cycles when an exception is raised Apr 12, 2025

test_map_exception: improve readability

82542f3

kumaraditya303 reviewed Aug 13, 2025

View reviewed changes

github-actions Bot added the stale Stale PR or inactive for long period of time. label Apr 22, 2026

                       self.assertEqual(i.__next__(), (0, 1))
                       self.assertEqual(i.__next__(), (0, 1))
-                      with self.assertRaises(ZeroDivisionError):

gh-74028: `concurrent.futures.Executor.map`: avoid reference cycles when an exception is raised by ebonnal · Pull Request #131701 · python/cpython · GitHub

Uh oh!

Uh oh!

gh-74028: concurrent.futures.Executor.map: avoid reference cycles when an exception is raised - #131701

gh-74028: concurrent.futures.Executor.map: avoid reference cycles when an exception is raised#131701ebonnal wants to merge 49 commits intopython:mainpython/cpython:mainfrom ebonnal:test/executor-map-ref-cyclesebonnal/cpython:test/executor-map-ref-cyclesCopy head branch name to clipboard

Conversation

ebonnal commented Mar 24, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

ebonnal commented Mar 25, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Uh oh!

graingert commented Mar 30, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Uh oh!

ebonnal commented Mar 30, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

graingert commented Apr 3, 2025

Uh oh!

ebonnal commented Apr 5, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

graingert commented Apr 7, 2025

Uh oh!

ebonnal commented Apr 7, 2025

Uh oh!

ebonnal commented Apr 26, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

kumaraditya303 Aug 13, 2025

Choose a reason for hiding this comment

Uh oh!

github-actions Bot commented Apr 22, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

5 participants

gh-74028: concurrent.futures.Executor.map: avoid reference cycles when an exception is raised#131701
ebonnal wants to merge 49 commits into
python:mainfrom
ebonnal:test/executor-map-ref-cycles

ebonnal commented Mar 24, 2025 •
edited
Loading

ebonnal commented Mar 25, 2025 •
edited
Loading

graingert commented Mar 30, 2025 •
edited
Loading

ebonnal commented Mar 30, 2025 •
edited
Loading

ebonnal commented Apr 5, 2025 •
edited
Loading

ebonnal commented Apr 26, 2025 •
edited
Loading