GeezerVisonThe World Wide Web, restored to 800 pixels of glory. | ||||||||
| Home|About|Gaming cabinet|Archives|How it works|Starter sites | ||||||||
GeezerVison is showing github.com — gh-74028: `concurrent.futures.Executor.map`: avoid reference cycles when an exception is raised by ebonnal · Pull Request #131701 · python/cpython Conversation
|
|
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 |
Sorry, something went wrong.
There was an error while loading. Please reload this page.
concurrent.futures.Executor.map: test no reference cycle from failed future captured in its exception’s tracebackconcurrent.futures.Executor.map: test no reference cycle from failed future captured in its exception’s traceback
There was an error while loading. Please reload this page.
There was an error while loading. Please reload this page.
|
@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. |
Sorry, something went wrong.
There was an error while loading. Please reload this page.
There was an error while loading. Please reload this page.
concurrent.futures.Executor.map: test no reference cycle from failed future captured in its exception’s tracebackconcurrent.futures.Executor.map: test there are no reference cycles when an exception is raised
There was an error while loading. Please reload this page.
Thanks @graingert !
Nice, I have cleaned up this (I have renamed the PR to show that we have extended its scope) |
Sorry, something went wrong.
There was an error while loading. Please reload this page.
concurrent.futures.Executor.map: test there are no reference cycles when an exception is raisedconcurrent.futures.Executor.map: test there are no reference cycles after an exception is raised
concurrent.futures.Executor.map: test there are no reference cycles after an exception is raisedconcurrent.futures.Executor.map: ensure there are no reference cycles after an exception is raised
concurrent.futures.Executor.map: ensure there are no reference cycles after an exception is raisedconcurrent.futures.Executor.map: avoid reference cycles after an exception is raised
concurrent.futures.Executor.map: avoid reference cycles after an exception is raisedconcurrent.futures.Executor.map: avoid reference cycles when an exception is raised
There was an error while loading. Please reload this page.
concurrent.futures.Executor.map: avoid reference cycles when an exception is raisedconcurrent.futures.Executor: avoid reference cycles when an exception is raised
There was an error while loading. Please reload this page.
|
@graingert via my recent cosmetic changes I have noticed that the
-> as a deterministic workaround in those situations, I have added an explicit gc collection before the test: if ( '--disable-gil' in get_config_var("CONFIG_ARGS") and sys.platform in ("linux", "win32") ): gc.collect() I wonder if this GC issue with free-threading builds is a known issue. I also wonder if we should investigate this further as part of this PR or if we should open an issue 🤔 Let me know your thoughts! |
Sorry, something went wrong.
There was an error while loading. Please reload this page.
|
When debugging these sorts of things I usually use a mix of the packages |
Sorry, something went wrong.
There was an error while loading. Please reload this page.
|
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 |
Sorry, something went wrong.
There was an error while loading. Please reload this page.
There was an error while loading. Please reload this page.
|
Hey @graingert, update from my investigations with a free-threading build on an Ubuntu ARM machine: |
Sorry, something went wrong.
There was an error while loading. Please reload this page.
|
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 |
Sorry, something went wrong.
There was an error while loading. Please reload this page.
82542f3 to
fb0c522
Compare
…hongh-74028) Hold exc across 'raise exc' keeps the frame in the exception's referrers via its traceback; clear it in a finally like the interpreter.py fix so the strengthened test_map_exception passes for all pool types. Co-authored-by: Muse Code (Meta Muse Spark)
05ba98a to
e0a031d
Compare
The thread that completed the future can still hold the exception (via the future) when the main thread wakes up. Poll with a deadline instead of sleeping a fixed second so the test stays fast normally and robust under load, especially on free-threaded builds. Co-authored-by: Muse Code (Meta Muse Spark)
There was an error while loading. Please reload this page.
There was an error while loading. Please reload this page.
Successfully merging this pull request may close these issues.
There was an error while loading. Please reload this page.