gh-123241: Don't modify ref count during visitation by DinoV · Pull Request #142232 · python/cpython · GitHub

traverse functions shouldn't modify the ref count during visitation otherwise they'll cause issues in the free threaded GC. CType_Type_traverse uses _PyStgInfo_FromType_NoState to get back to the state object but this causes an incref to happen via PyType_GetBaseByToken.

Instead of using PyType_GetBaseByToken this just walks to find the CType's solid base. Because PyCType_Type adds fields and immediately descends from PyType_Type it will always be the base immediately before PyType_Type.

Issue: GC in free-threaded build has problems with Py_INCREF()/Py_DECREF() in tp_traverse handlers #123241

traverse functions shouldn't modify the ref count during visitation otherwise they'll cause issues in the free threaded GC.

Is this limitation documented? That sounds like it should be mentioned in the docs, and a free-threading porting guide.

Because PyCType_Type adds fields and immediately descends from PyType_Type it will always be the base immediately before PyType_Type.

There's an assumption I'd like to relax eventually :)
Could we add a _PyType_GetBaseByToken_Borrow instead?
If not, please add an assertion that the token matches.

Was this found from a crash or by inspecting the code? Either way, I think it's worth fixing, but my understanding was that the problematic pattern involved incref/decref around the visit, i.e. something like:

Py_INCREF(obj);
Py_VISIT(obj);
Py_DECREF(obj);

While this code was more like:

Py_INCREF(obj);
Py_DECREF(obj);
Py_VISIT(obj);

I think we can make the free threaded GC less susceptible to this problem, but we should also document the things you shouldn't do in a tp_traverse handler.

We shouldn't create exceptions (or any other object) during tp_traverse handlers either. I think this should be pulled up to the tp_dealloc / tp_clear if we want to print unraisable exceptions there.

Was this found from a crash or by inspecting the code?

I found this from a crash that occurred while porting the parallel GC in cinder (it can call the same traverse function from multiple threads) to 3.14.

…callers print unraisable

There's an assumption I'd like to relax eventually :) Could we add a _PyType_GetBaseByToken_Borrow instead? If not, please add an assertion that the token matches.

Sounds good, I've gone ahead and added the Borrow version and switched to using it instead.

Thanks! Looks great; I left some nitpicks in DinoV#2

_PyType_GetBaseByToken_Borrow can't fail; adjust comments

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

Sorry, @DinoV and @encukou, I could not cleanly backport this to 3.14 due to a conflict.
Please backport using cherry_picker on command line.

cherry_picker da8199f8842c2830fa4015138725849863523de4 3.14

Thank you!

…honGH-142232) (cherry picked from commit da8199f) Co-authored-by: Dino Viehland <dinoviehland@meta.com>

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

#142567) (cherry picked from commit da8199f) Co-authored-by: Dino Viehland <dinoviehland@meta.com>

bedevere-app Bot mentioned this pull request Dec 3, 2025

GC in free-threaded build has problems with Py_INCREF()/Py_DECREF() in tp_traverse handlers #123241

Open

DinoV requested review from encukou and mpage December 3, 2025 18:19

DinoV added the skip news label Dec 3, 2025

DinoV marked this pull request as ready for review December 3, 2025 18:46

bedevere-app Bot added the awaiting core review label Dec 3, 2025

DinoV added the needs backport to 3.14 bugs and security fixes label Dec 3, 2025

colesbury reviewed Dec 4, 2025

View reviewed changes

DinoV added 2 commits December 5, 2025 11:18

Don't modify ref count during visitation

aa4924d

Add _PyType_GetBaseByToken_Borrow, don't raise exception and let the …

f860118

…callers print unraisable

DinoV force-pushed the dont_modify_refcount branch from 346043e to f860118 Compare December 5, 2025 19:18

DinoV requested a review from markshannon as a code owner December 5, 2025 19:18

encukou added 2 commits December 10, 2025 11:17

_PyType_GetBaseByToken_Borrow can't fail

4dfa8ec

Add blurb

37f8fbf

DinoV and others added 3 commits December 10, 2025 14:45

Merge pull request #2 from encukou/dont_modify_refcount

f8648ab

_PyType_GetBaseByToken_Borrow can't fail; adjust comments

Always initialize result

9861a58

Merge branch 'main' into dont_modify_refcount

147c76d

encukou merged commit da8199f into python:main Dec 11, 2025
46 checks passed

bedevere-app Bot removed the awaiting core review label Dec 11, 2025

miss-islington-app Bot assigned encukou Dec 11, 2025

encukou pushed a commit to encukou/cpython that referenced this pull request Dec 11, 2025

[3.14] pythongh-123241 : Don't modify ref count during visitation (pyt…

232ff23

…honGH-142232) (cherry picked from commit da8199f) Co-authored-by: Dino Viehland <dinoviehland@meta.com>

bedevere-app Bot removed the needs backport to 3.14 bugs and security fixes label Dec 11, 2025

DinoV added a commit that referenced this pull request Dec 11, 2025

[3.14] gh-123241 : Don't modify ref count during visitation (GH-142232 ) (

e09c4de

#142567) (cherry picked from commit da8199f) Co-authored-by: Dino Viehland <dinoviehland@meta.com>

gh-123241: Don't modify ref count during visitation by DinoV · Pull Request #142232 · python/cpython · GitHub

Repository navigation

gh-123241: Don't modify ref count during visitation - #142232

gh-123241: Don't modify ref count during visitation#142232
encukou merged 7 commits into
python:mainfrom
DinoV:dont_modify_refcount

DinoV commented Dec 3, 2025 •
edited by bedevere-app Bot
Loading

Uh oh!

Uh oh!

encukou commented Dec 4, 2025

Uh oh!

colesbury left a comment

Uh oh!

colesbury Dec 4, 2025

Uh oh!

mpage commented Dec 4, 2025

Uh oh!

DinoV commented Dec 5, 2025

Uh oh!

encukou commented Dec 10, 2025

Uh oh!

Uh oh!

miss-islington-app Bot commented Dec 11, 2025

Uh oh!

miss-islington-app Bot commented Dec 11, 2025

Uh oh!

encukou commented Dec 11, 2025

Uh oh!

bedevere-app Bot commented Dec 11, 2025

Uh oh!

Reviewers

Uh oh!

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

                   if (PyCType_Type == NULL) {
                       PyErr_Format(PyExc_TypeError, "expected a ctypes type, got '%N'", type);
                       return NULL;
                   }

gh-123241: Don't modify ref count during visitation by DinoV · Pull Request #142232 · python/cpython · GitHub

Repository navigation

gh-123241: Don't modify ref count during visitation - #142232

gh-123241: Don't modify ref count during visitation#142232encukou merged 7 commits intopython:mainpython/cpython:mainfrom DinoV:dont_modify_refcountDinoV/cpython:dont_modify_refcountCopy head branch name to clipboard

Conversation

DinoV commented Dec 3, 2025 • edited by bedevere-app Bot Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

encukou commented Dec 4, 2025

Uh oh!

colesbury left a comment

Choose a reason for hiding this comment

Uh oh!

colesbury Dec 4, 2025

Choose a reason for hiding this comment

Uh oh!

mpage commented Dec 4, 2025

Uh oh!

DinoV commented Dec 5, 2025

Uh oh!

encukou commented Dec 10, 2025

Uh oh!

Uh oh!

miss-islington-app Bot commented Dec 11, 2025

Uh oh!

miss-islington-app Bot commented Dec 11, 2025

Uh oh!

encukou commented Dec 11, 2025

Uh oh!

bedevere-app Bot commented Dec 11, 2025

Uh oh!

Reviewers

Uh oh!

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

gh-123241: Don't modify ref count during visitation#142232
encukou merged 7 commits into
python:mainfrom
DinoV:dont_modify_refcount

DinoV commented Dec 3, 2025 •
edited by bedevere-app Bot
Loading