Skip to content

gh-150411: fix gc_generation.count race - #150413

Merged
kumaraditya303 merged 4 commits into
python:mainfrom
LindaSummer:gc_count_tsan
Jun 6, 2026
Merged

gh-150411: fix gc_generation.count race#150413
kumaraditya303 merged 4 commits into
python:mainfrom
LindaSummer:gc_count_tsan

Conversation

@LindaSummer

Copy link
Copy Markdown
Contributor

Issue

gh-150411

Root Cause

Refer the analytics in issue gh-150411, it should be a gc_generation.count update during the cyclic object allocation triggered the local allocation count migrated to young generation. Meantime we try to read the gc_generation.count without sync caused the race.

staticvoid
record_allocation(PyThreadState*tstate)
{
struct_gc_thread_state*gc=&((_PyThreadStateImpl*)tstate)->gc;
// We buffer the allocation count to avoid the overhead of atomic
// operations for every allocation.
gc->alloc_count++;
if (gc->alloc_count >= LOCAL_ALLOC_COUNT_THRESHOLD) {
// TODO: Use Py_ssize_t for the generation count.
GCState*gcstate=&tstate->interp->gc;
_Py_atomic_add_int(&gcstate->young.count, (int)gc->alloc_count);
gc->alloc_count=0;
if (gc_should_collect(gcstate) &&
!_Py_atomic_load_int_relaxed(&gcstate->collecting))
{
_Py_ScheduleGC(tstate);
}
}
}

I find this problem during proposing gh-150356. So they have similar reproduce way.

Proposed Changes

I added an atomic relax load guard for the gc_generation.count.
It was protected in other places expect current one.

@pablogsal

Copy link
Copy Markdown
Member

Same concerns as #150356 (comment)

CC @nascheme

Comment threadModules/gcmodule.c Outdated
gcstate->old[0].count,
gcstate->old[1].count);
_Py_atomic_load_int_relaxed(&gcstate->young.count),
_Py_atomic_load_int_relaxed(&gcstate->old[0].count),

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Does gcstate->old[0].count need to be atomic here? I think it is only ever written under stw so it is unnecessary.

Copy link
Copy Markdown
ContributorAuthor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hi @kumaraditya303 ,

Thanks very much for your review! ❤️
Yes, I searched the usage of count and find that only the young.count is updated without STW guard.
old[generation].count are all protected by STW.
So we only need to make young.count use atomic action and old[generation].count read is safe.

@LindaSummer

Copy link
Copy Markdown
ContributorAuthor

Hi @pablogsal and @kumaraditya303 ,

Thanks very much for your review and suggestions!

I have updated the patch with only atomic for young.count.
The old[generation].count write is protected by STW.
The young.count is a frequently updated counter and in this patch we only read its snapshot.
So I think the result is consistent since we only synced one variable.

Please correct me if I made any mistake or misunderstanding.

Wish you a good day! 🌞

Comment threadLib/test/test_free_threading/test_gc.py Outdated
@kumaraditya303
kumaraditya303 enabled auto-merge (squash) June 6, 2026 16:36
@kumaraditya303
kumaraditya303 merged commit 12af26d into python:mainJun 6, 2026
59 checks passed
philthompson10 pushed a commit to philthompson10/cpython that referenced this pull request Jun 17, 2026
@naschemenascheme added needs backport to 3.14 bugs and security fixes needs backport to 3.15 pre-release feature fixes, bugs and security fixes labels Jul 16, 2026
@miss-islington-app

Copy link
Copy Markdown

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

@miss-islington-app

Copy link
Copy Markdown

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

@bedevere-app

Copy link
Copy Markdown

GH-153787 is a backport of this pull request to the 3.14 branch.

@bedevere-appbedevere-appBot removed the needs backport to 3.14 bugs and security fixes label Jul 16, 2026
@bedevere-app

Copy link
Copy Markdown

GH-153788 is a backport of this pull request to the 3.15 branch.

@bedevere-appbedevere-appBot removed the needs backport to 3.15 pre-release feature fixes, bugs and security fixes label Jul 16, 2026
kumaraditya303 pushed a commit that referenced this pull request Jul 16, 2026
…H-150413) (#153788)
gh-150411: fix `gc_generation.count` race in free-threading (GH-150413)
(cherry picked from commit 12af26d)
Co-authored-by: Edward Xu <xuxiangad@gmail.com>
nascheme pushed a commit that referenced this pull request Jul 17, 2026
…H-150413) (GH-153787)
(cherry picked from commit 12af26d)
Co-authored-by: Edward Xu <xuxiangad@gmail.com>
Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants

@LindaSummer@pablogsal@kumaraditya303@nascheme