Skip to content

gh-150411: fix gc_generation.count race#150413

Open
LindaSummer wants to merge 3 commits into
python:mainfrom
LindaSummer:gc_count_tsan
Open

gh-150411: fix gc_generation.count race#150413
LindaSummer wants to merge 3 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.

static void
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

Sign up for free to 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.

2 participants