gh-126312: Don't traverse frozen objects on the free-threaded build#126338

ZeroIntensity · 2024-11-02T18:50:54Z

Issue: GC aborts in debug no-gil build #126312

ZeroIntensity · 2024-11-02T18:59:33Z

Apparently, the fix isn't as stable as I thought. Marking as draft temporarily.

Lib/test/test_gc.py

rruuaanng · 2024-11-03T01:09:15Z

It looks good.

Python/gc_free_threading.c

Co-authored-by: Sergey B Kirpichev <skirpichev@gmail.com>

…ensity/cpython into fix-nogil-frozen-objects

colesbury

The changes to gc_free_threading.c look good. A few questions about the test case.

I think we also need to update visit_freeze to avoid marking objects with _PyGC_BITS_UNREACHABLE set as frozen.

Lib/test/test_gc.py

colesbury · 2024-11-05T18:37:59Z

Please also fix visit_freeze to avoid marking objects with _PyGC_BITS_UNREACHABLE

ZeroIntensity · 2024-11-05T18:51:08Z

Please also fix visit_freeze to avoid marking objects with _PyGC_BITS_UNREACHABLE

Done. I'll see if I can come up with a quick test case. If not, it's probably fine to merge that untested, as it's such a small change.

Python/gc_free_threading.c

Lib/test/test_gc.py

colesbury

Thanks, LGTM

vstinner

LGTM.

I was surprised by the "_PyGC_Freeze() no longer freezes unreachable objects" change which is not "directly" related to "Don't traverse frozen objects on the free-threaded build". But this change makes sense.

miss-islington-app · 2024-11-15T10:21:34Z

Thanks @ZeroIntensity for the PR, and @vstinner for merging it 🌮🎉.. I'm working now to backport this PR to: 3.13.
🐍🍒⛏🤖

miss-islington-app · 2024-11-15T10:21:38Z

Sorry, @ZeroIntensity and @vstinner, I could not cleanly backport this to 3.13 due to a conflict.
Please backport using cherry_picker on command line.

cherry_picker d4c72fed8cba8e15ab7bb6c30a92bc9f2c8f0a2c 3.13

vstinner · 2024-11-15T10:22:11Z

@ZeroIntensity: Oh, the automated backport to 3.13 failed. Do you want to attempt to fix the cherry-pick conflict?

vstinner · 2024-11-15T10:22:24Z

Merged, thanks @ZeroIntensity.

…eaded build (pythonGH-126338) Also, _PyGC_Freeze() no longer freezes unreachable objects. (cherry picked from commit d4c72fe) Co-authored-by: Peter Bierma <zintensitydev@gmail.com> Co-authored-by: Sergey B Kirpichev <skirpichev@gmail.com>

bedevere-app · 2024-11-15T12:19:44Z

GH-126866 is a backport of this pull request to the 3.13 branch.

…build (GH-126338) (#126866) * Fix merge conflicts. * [3.13] gh-126312: Don't traverse frozen objects on the free-threaded build (GH-126338) Also, _PyGC_Freeze() no longer freezes unreachable objects. (cherry picked from commit d4c72fe) Co-authored-by: Peter Bierma <zintensitydev@gmail.com> Co-authored-by: Sergey B Kirpichev <skirpichev@gmail.com> --------- Co-authored-by: Sergey B Kirpichev <skirpichev@gmail.com>

…uild (python#126338) Also, _PyGC_Freeze() no longer freezes unreachable objects. Co-authored-by: Sergey B Kirpichev <skirpichev@gmail.com>

ZeroIntensity added 4 commits November 2, 2024 14:39

Ignore frozen objects when traversing.

2768acd

Add blurb.

08e2b7b

Add test.

149dd64

Clarify comment.

7cad645

ZeroIntensity added topic-free-threading needs backport to 3.13 bugs and security fixes labels Nov 2, 2024

ZeroIntensity requested a review from colesbury November 2, 2024 18:50

bedevere-app bot added the awaiting review label Nov 2, 2024

bedevere-app bot mentioned this pull request Nov 2, 2024

GC aborts in debug no-gil build #126312

Closed

ZeroIntensity marked this pull request as draft November 2, 2024 18:59

bedevere-app bot removed the awaiting review label Nov 2, 2024

ZeroIntensity added 2 commits November 2, 2024 16:25

Make test better.

5a58035

Put a bandaid on the test.

754f44a

ZeroIntensity marked this pull request as ready for review November 2, 2024 21:37

bedevere-app bot added the awaiting review label Nov 2, 2024

tomasr8 reviewed Nov 2, 2024

View reviewed changes

Lib/test/test_gc.py Outdated Show resolved Hide resolved

Use assert_python_ok()

cf5d2cd

ZeroIntensity requested a review from tomasr8 November 3, 2024 01:50

rruuaanng approved these changes Nov 3, 2024

View reviewed changes

Python/gc_free_threading.c Outdated Show resolved Hide resolved

bedevere-app bot added awaiting core review and removed awaiting review labels Nov 3, 2024

skirpichev reviewed Nov 3, 2024

View reviewed changes

Python/gc_free_threading.c Outdated Show resolved Hide resolved

ZeroIntensity and others added 5 commits November 3, 2024 01:21

Update gc_free_threading.c

a7b8d36

Co-authored-by: Sergey B Kirpichev <skirpichev@gmail.com>

Fix deferred reference counts.

66460b0

Switch the test around.

d69eb07

Merge branch 'fix-nogil-frozen-objects' of https://github.com/ZeroInt…

6d949cc

…ensity/cpython into fix-nogil-frozen-objects

Make NEWS entry more accurate.

2add712

colesbury reviewed Nov 4, 2024

View reviewed changes

Lib/test/test_gc.py Outdated Show resolved Hide resolved

Lib/test/test_gc.py Show resolved Hide resolved

Lib/test/test_gc.py Show resolved Hide resolved

ZeroIntensity added 2 commits November 5, 2024 13:46

Add clarification comment.

5282cd2

Don't freeze unreachable objects.

d2a88d3

ZeroIntensity requested a review from colesbury November 5, 2024 18:48

vstinner reviewed Nov 6, 2024

View reviewed changes

Python/gc_free_threading.c Outdated Show resolved Hide resolved

vstinner reviewed Nov 6, 2024

View reviewed changes

Lib/test/test_gc.py Show resolved Hide resolved

ZeroIntensity added 2 commits November 6, 2024 18:00

Use gc_is_frozen()

0592953

Seperate tests.

ea2493c

ZeroIntensity requested a review from vstinner November 6, 2024 23:04

colesbury approved these changes Nov 6, 2024

View reviewed changes

bedevere-app bot added awaiting merge and removed awaiting core review labels Nov 6, 2024

vstinner approved these changes Nov 15, 2024

View reviewed changes

vstinner merged commit d4c72fe into python:main Nov 15, 2024

bedevere-app bot removed the awaiting merge label Nov 15, 2024

miss-islington-app bot assigned vstinner Nov 15, 2024

ZeroIntensity assigned ZeroIntensity and unassigned vstinner Nov 15, 2024

bedevere-app bot removed the needs backport to 3.13 bugs and security fixes label Nov 15, 2024

ZeroIntensity deleted the fix-nogil-frozen-objects branch November 15, 2024 14:00

Uh oh!

Conversation

ZeroIntensity commented Nov 2, 2024 • edited by bedevere-app bot Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

ZeroIntensity commented Nov 2, 2024

Uh oh!

Uh oh!

rruuaanng commented Nov 3, 2024

Uh oh!

Uh oh!

Uh oh!

colesbury left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

colesbury commented Nov 5, 2024

Uh oh!

ZeroIntensity commented Nov 5, 2024

Uh oh!

Uh oh!

Uh oh!

colesbury left a comment

Choose a reason for hiding this comment

Uh oh!

vstinner left a comment

Choose a reason for hiding this comment

Uh oh!

miss-islington-app bot commented Nov 15, 2024

Uh oh!

miss-islington-app bot commented Nov 15, 2024

Uh oh!

vstinner commented Nov 15, 2024

Uh oh!

vstinner commented Nov 15, 2024

Uh oh!

bedevere-app bot commented Nov 15, 2024

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

6 participants

ZeroIntensity commented Nov 2, 2024 •

edited by bedevere-app bot

Loading