Skip to content

gh-149816: Fix race in subtype_getweakref in free-threading build - #150247

Merged
nascheme merged 2 commits into
python:mainfrom
Abhi210:gh-149816-61
Oct 12, 2026
Merged

nascheme merged 2 commits into
python:mainfrom
Abhi210:gh-149816-61

Conversation

@Abhi210

@Abhi210 Abhi210 commented May 22, 2026 •

Copy link
Copy Markdown
Contributor

This PR resolves finding 61 from the weakref race tracking issue: "Racy weakref head load before incref in Objects/typeobject.c".

Description

In free-threaded builds, subtype_getweakref() had a race condition where the weakref list head (*weaklistptr) was loaded and immediately passed to Py_NewRef(). If another thread concurrently triggered weakref_dealloc(), the weakref could be freed before the incref, leading to a use-after-free (UAF).

This PR fixes the issue by:

  1. Wrapping the pointer load and incref sequence inside the existing LOCK_WEAKREFS(obj) / UNLOCK_WEAKREFS(obj) critical section.
  2. Replacing Py_NewRef() with _Py_TryIncref() to safely handle weakrefs whose refcount has reached zero during the race window.
  3. Returning Py_NewRef(Py_None) when the list is empty or the weakref is no longer live, preserving the existing API behavior.

Testing

Added regression tests in Lib/test/test_free_threading/test_weakref.py that:

  • Exercise obj.__weakref__ concurrently from reader and mutator threads synchronized with threading.Barrier.
  • Verify correct return values (None versus weakref.ref).
  • Check refcount stability across repeated accesses to detect leaks or incorrect incref/decref behavior.

AI Tool Disclosure:

Claude Code was used to help understand the relevant code paths and to improve the clarity and documentation of the proposed tests. It was not used to modify Objects/typeobject.c; all code changes and final decisions were made by me.

@python-cla-bot

python-cla-bot Bot commented May 22, 2026 •

Copy link
Copy Markdown

All commit authors signed the Contributor License Agreement.

CLA signed

@nascheme

nascheme commented Jul 3, 2026 •

Copy link
Copy Markdown
Member

This looks like a real issue. Comments on PR:

  • use test.support.threading_helper.run_concurrently() rather than custom run_in_threads()
  • avoid using os.cpu_count(), might be large number on big machines, use a hard-coded small number of threads instead, should be sufficient to show issue
  • unit test file contains too much verbose comments, IMHO. This is typical of AI generated code in my experience. Don't explain what the code is doing if the code is clear (which it should be if all possible).
  • The if (*weaklistptr == NULL) branch should have braces to conform to modern CPython C style
  • The comment about LOCK_WEAKREFS() is overly verbose. I'd suggest: "Synchronize with weakref creation/destruction so the list head cannot be removed and freed between loading it and taking a reference."

@Abhi210

Abhi210 commented Jul 12, 2026

Copy link
Copy Markdown
Contributor Author

Hello @nascheme! Thank you for the comments. I have resolved those issues. Let me know if there are any other changes to be made.

@github-actions

Copy link
Copy Markdown

This PR is stale because it has been open for 90 days with no activity.

@github-actions github-actions Bot added the stale Stale PR or inactive for long period of time. label Oct 11, 2026
@nascheme
nascheme merged commit f674db1 into python:main Oct 12, 2026
54 checks passed
@nascheme nascheme added needs backport to 3.14 bugs and security fixes needs backport to 3.15 bugs and security fixes labels Oct 13, 2026
@miss-islington-app

Copy link
Copy Markdown

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

@miss-islington-app

Copy link
Copy Markdown

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

@bedevere-app

bedevere-app Bot commented Oct 13, 2026

Copy link
Copy Markdown

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

@bedevere-app bedevere-app Bot removed the needs backport to 3.14 bugs and security fixes label Oct 13, 2026
@bedevere-app

bedevere-app Bot commented Oct 13, 2026

Copy link
Copy Markdown

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

@bedevere-app bedevere-app Bot removed the needs backport to 3.15 bugs and security fixes label Oct 13, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

stale Stale PR or inactive for long period of time.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

22 free-threading race conditions

2 participants