-
-
Notifications
You must be signed in to change notification settings - Fork 33.2k
gh-140431: Fix GC crash due to partially initialized coroutines #140470
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
Conversation
The `make_gen()` function creates and tracks generator/coro objects, but doesn't fully initialize all the fields. At a minimum, we need to initialize all the fields that may be accessed by gen_traverse because the call to `compute_cr_origin()` can trigger a GC.
|
For what it's worth, with this patch, I am no longer able to reproduce the bug. With this patch, I was not able to reproduce the bug after an hour of running the tests. Without this patch, the segfault occurs on average after < 4 minutes. I have used the following script to check: #!/usr/bin/env bash
set -e
time_start=$(date +%s)
git clean -dfx
CC=clang CXX=clang++ ./configure --with-address-sanitizer --with-pydebug --disable-gil
make -j12
./python -X dev -m test -R 9:9 test_asyncio.test_free_threading
./python -X dev -m test -R 9:9 test_asyncio.test_free_threading
./python -X dev -m test -R 9:9 test_asyncio.test_free_threading
./python -X dev -m test -R 9:9 test_asyncio.test_free_threading
./python -X dev -m test -R 9:9 test_asyncio.test_free_threading
./python -X dev -m test -R 9:9 test_asyncio.test_free_threading
./python -X dev -m test -R 9:9 test_asyncio.test_free_threading
./python -X dev -m test -R 9:9 test_asyncio.test_free_threading
./python -X dev -m test -R 9:9 test_asyncio.test_free_threading
./python -X dev -m test -R 9:9 test_asyncio.test_free_threading
time_end=$(date +%s)
delta=$((time_end - time_start))
minutes=$((delta / 60))
seconds=$((delta % 60))
echo
echo
echo "Elapsed time: ${minutes}m ${seconds}s"
echo
echo -n "Possibly good commit: "
git rev-parse HEAD
git log --oneline | head -n 1
echo |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM.
We definitely need to set correct value of gen->gi_iframe.f_executable in make_gen.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
|
Thanks @colesbury for the PR 🌮🎉.. I'm working now to backport this PR to: 3.14. |
…pythongh-140470) The `make_gen()` function creates and tracks generator/coro objects, but doesn't fully initialize all the fields. At a minimum, we need to initialize all the fields that may be accessed by gen_traverse because the call to `compute_cr_origin()` can trigger a GC. (cherry picked from commit 574405c) Co-authored-by: Sam Gross <colesbury@gmail.com>
|
GH-140504 is a backport of this pull request to the 3.14 branch. |
|
gh-140470) (gh-140504) The `make_gen()` function creates and tracks generator/coro objects, but doesn't fully initialize all the fields. At a minimum, we need to initialize all the fields that may be accessed by gen_traverse because the call to `compute_cr_origin()` can trigger a GC. (cherry picked from commit 574405c) Co-authored-by: Sam Gross <colesbury@gmail.com>
The
make_gen()function creates and tracks generator/coro objects, but doesn't fully initialize all the fields. At a minimum, we need to initialize all the fields that may be accessed by gen_traverse because the call tocompute_cr_origin()can trigger a GC.