gh-135557: Use atomic writes on heapq operations by xuantengh · Pull Request #135601 · python/cpython

xuantengh · 2025-06-17T05:22:54Z

Use atomic writes on heapq operation.

Issue: data race between lock free list reads and heapq #135557

xuantengh · 2025-06-17T05:56:45Z

I think this could be "skip news"?

ZeroIntensity · 2025-06-17T14:12:53Z

I think this could be "skip news"?

This is user-facing, so this does need a news. Something like "Fix race with lists and heapq on the free threaded build" would be fine. It would also be good to get a test. (Sorry for pushing back on both your PRs!)

Modules/_heapqmodule.c

Misc/NEWS.d/next/Library/2025-06-17-23-13-56.gh-issue-135557.Bfcy4v.rst

Lib/test/test_heapq.py

Modules/_heapqmodule.c

python-cla-bot · 2025-06-18T03:19:00Z

All commit authors signed the Contributor License Agreement.

Lib/test/test_free_threading/test_heapq.py

Modules/_heapqmodule.c

…fcy4v.rst Co-authored-by: Peter Bierma <zintensitydev@gmail.com>

Lib/test/test_free_threading/test_heapq.py

Modules/_heapqmodule.c

ZeroIntensity · 2025-06-20T16:09:31Z

!buildbot nogil

bedevere-bot · 2025-06-20T16:09:35Z

🤖 New build scheduled with the buildbot fleet by @ZeroIntensity for commit a10560e 🤖

Results will be shown at:

https://buildbot.python.org/all/#/grid?branch=refs%2Fpull%2F135601%2Fmerge

The command will test the builders whose names match following regular expression: nogil

The builders matched are:

ARM64 MacOS M1 NoGIL PR
AMD64 Windows Server 2022 NoGIL PR
aarch64 Fedora Rawhide NoGIL refleaks PR
AMD64 Fedora Rawhide NoGIL PR
aarch64 Fedora Rawhide NoGIL PR
ARM64 MacOS M1 Refleaks NoGIL PR
AMD64 CentOS9 NoGIL Refleaks PR
AMD64 Fedora Rawhide NoGIL refleaks PR
s390x Fedora Rawhide NoGIL refleaks PR
PPC64LE Fedora Rawhide NoGIL PR
PPC64LE Fedora Rawhide NoGIL refleaks PR
x86-64 MacOS Intel NoGIL PR
AMD64 CentOS9 NoGIL PR
x86-64 MacOS Intel ASAN NoGIL PR
AMD64 Windows PGO NoGIL PR
s390x Fedora Rawhide NoGIL PR

ZeroIntensity

LGTM as well, assuming buildbots pass.

miss-islington-app · 2025-06-21T08:43:18Z

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

…ading (pythonGH-135601) (cherry picked from commit 13cac83) Co-authored-by: Xuanteng Huang <44627253+xuantengh@users.noreply.github.com>

bedevere-app · 2025-06-21T08:43:27Z

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

YvesDup · 2025-06-21T10:22:01Z

Lib/test/test_free_threading/test_heapq.py

+                    except IndexError:
+                        pass
+
+        self.run_concurrently(worker, (), n_threads * 2)


Test is successful here because the worker function has no arguments.
But shouldn't the second argument args be a tuple?

isn't it an empty tuple here?

…eading (GH-135601) (#135787) gh-135557: use atomic stores in `heapq` operations in free-threading (GH-135601) (cherry picked from commit 13cac83) Co-authored-by: Xuanteng Huang <44627253+xuantengh@users.noreply.github.com>

YvesDup · 2025-06-23T11:59:28Z

isn't it an empty tuple here?

Oh yes it is.
Reading an empty tuple for arguments when creating new threads destabilized me (and then silly). This is because when creating a new Thread with no arguments, the args argument is always omitted (the default is an empty tuple).
Sorry for the noise.

…ading (python#135601)

xuantengh requested a review from rhettinger as a code owner June 17, 2025 05:22

bedevere-app bot added the awaiting review label Jun 17, 2025

bedevere-app bot mentioned this pull request Jun 17, 2025

data race between lock free list reads and heapq #135557

Closed

xuantengh changed the title ~~gh-135557: Fix lock-free list access on heapq~~ gh-135557: Use atomic writes on heapq operations Jun 17, 2025

rhettinger removed their request for review June 17, 2025 12:51

ZeroIntensity added the needs backport to 3.14 bugs and security fixes label Jun 17, 2025

ZeroIntensity reviewed Jun 17, 2025

View reviewed changes

Modules/_heapqmodule.c Outdated Show resolved Hide resolved

xuantengh force-pushed the heap-list-lock-free-read branch from f7fbb68 to 08770ec Compare June 17, 2025 16:16

ZeroIntensity reviewed Jun 17, 2025

View reviewed changes

kumaraditya303 reviewed Jun 17, 2025

View reviewed changes

Modules/_heapqmodule.c Outdated Show resolved Hide resolved

xuantengh force-pushed the heap-list-lock-free-read branch from 0184b21 to 463963d Compare June 18, 2025 06:38

kumaraditya303 reviewed Jun 18, 2025

View reviewed changes

Lib/test/test_free_threading/test_heapq.py Outdated Show resolved Hide resolved

kumaraditya303 reviewed Jun 18, 2025

View reviewed changes

Modules/_heapqmodule.c Outdated Show resolved Hide resolved

xuantengh force-pushed the heap-list-lock-free-read branch from 463963d to c49ff7d Compare June 19, 2025 00:48

kumaraditya303 reviewed Jun 19, 2025

View reviewed changes

Modules/_heapqmodule.c Show resolved Hide resolved

xuantengh and others added 10 commits June 19, 2025 18:30

fix lock-free list access on heapq

86d3fbb

use FT_ATOMIC macro to store ptr

5f24742

add news entry

06328a1

add test

3b8ea80

add missing header

481ef22

Update Misc/NEWS.d/next/Library/2025-06-17-23-13-56.gh-issue-135557.B…

d00d698

…fcy4v.rst Co-authored-by: Peter Bierma <zintensitydev@gmail.com>

use relaxed atomic store

dd8f5ce

update tests

b7528bd

reduce test num elements

925a093

use ft store in heappushpop and heapreplace as well

27d6cb6

xuantengh force-pushed the heap-list-lock-free-read branch from c49ff7d to 27d6cb6 Compare June 19, 2025 10:31

ZeroIntensity reviewed Jun 19, 2025

View reviewed changes

Lib/test/test_free_threading/test_heapq.py Outdated Show resolved Hide resolved

Lib/test/test_free_threading/test_heapq.py Outdated Show resolved Hide resolved

xuantengh force-pushed the heap-list-lock-free-read branch from 2cc859d to 8cf4794 Compare June 19, 2025 13:17

use run_concurrent to spawn threads

32d767f

xuantengh force-pushed the heap-list-lock-free-read branch from 8cf4794 to 32d767f Compare June 19, 2025 13:42

fix

4fb9389

kumaraditya303 reviewed Jun 19, 2025

View reviewed changes

Modules/_heapqmodule.c Outdated Show resolved Hide resolved

fix style

a10560e

kumaraditya303 approved these changes Jun 20, 2025

View reviewed changes

bedevere-app bot added awaiting merge and removed awaiting review labels Jun 20, 2025

ZeroIntensity approved these changes Jun 20, 2025

View reviewed changes

kumaraditya303 merged commit 13cac83 into python:main Jun 21, 2025
58 checks passed

bedevere-app bot removed the awaiting merge label Jun 21, 2025

bedevere-app bot removed the needs backport to 3.14 bugs and security fixes label Jun 21, 2025

YvesDup reviewed Jun 21, 2025

View reviewed changes

xuantengh deleted the heap-list-lock-free-read branch June 23, 2025 16:26

kumaraditya303 added the topic-free-threading label Jun 24, 2025

Pranjal095 pushed a commit to Pranjal095/cpython that referenced this pull request Jul 12, 2025

pythongh-135557: use atomic stores in heapq operations in free-thre…

442de3b

…ading (python#135601)

taegyunkim pushed a commit to taegyunkim/cpython that referenced this pull request Aug 4, 2025

pythongh-135557: use atomic stores in heapq operations in free-thre…

bb95a55

…ading (python#135601)

Agent-Hellboy pushed a commit to Agent-Hellboy/cpython that referenced this pull request Aug 19, 2025

pythongh-135557: use atomic stores in heapq operations in free-thre…

a25c15e

…ading (python#135601)

Uh oh!

Conversation

xuantengh commented Jun 17, 2025 • edited by bedevere-app bot Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

xuantengh commented Jun 17, 2025

Uh oh!

ZeroIntensity commented Jun 17, 2025

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

python-cla-bot bot commented Jun 18, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

ZeroIntensity commented Jun 20, 2025

Uh oh!

bedevere-bot commented Jun 20, 2025

Uh oh!

ZeroIntensity left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

miss-islington-app bot commented Jun 21, 2025

Uh oh!

bedevere-app bot commented Jun 21, 2025

Uh oh!

YvesDup Jun 21, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

kumaraditya303 Jun 21, 2025

Choose a reason for hiding this comment

Uh oh!

YvesDup commented Jun 23, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

6 participants

xuantengh commented Jun 17, 2025 •

edited by bedevere-app bot

Loading

python-cla-bot bot commented Jun 18, 2025 •

edited

Loading

YvesDup Jun 21, 2025 •

edited

Loading

YvesDup commented Jun 23, 2025 •

edited

Loading