+51 −17

Conversation

xuantengh

Use atomic writes on heapq operation.

@xuantenghxuantengh requested a review from rhettinger as a code owner June 17, 2025 05:22
@bedevere-appbedevere-app bot mentioned this pull request Jun 17, 2025
Open
@xuantenghxuantengh changed the title gh-135557: Fix lock-free list access on heapq gh-135557: Use atomic writes on heapq operations Jun 17, 2025
@xuantengh

I think this could be "skip news"?

@rhettingerrhettinger removed their request for review June 17, 2025 12:51
@ZeroIntensity

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!)

@ZeroIntensityZeroIntensity added the needs backport to 3.14bugs and security fixeslabel Jun 17, 2025
ZeroIntensity
ZeroIntensity reviewed Jun 17, 2025
View reviewed changes
@xuantenghxuantengh force-pushed the heap-list-lock-free-read branch from f7fbb68 to 08770ec Compare June 17, 2025 16:16
ZeroIntensity
ZeroIntensity reviewed Jun 17, 2025
View reviewed changes
kumaraditya303
kumaraditya303 reviewed Jun 17, 2025
View reviewed changes
@python-cla-bot

All commit authors signed the Contributor License Agreement.

CLA signed

@xuantenghxuantengh force-pushed the heap-list-lock-free-read branch from 0184b21 to 463963d Compare June 18, 2025 06:38
kumaraditya303
kumaraditya303 reviewed Jun 18, 2025
View reviewed changes
kumaraditya303
kumaraditya303 reviewed Jun 18, 2025
View reviewed changes
@xuantenghxuantengh force-pushed the heap-list-lock-free-read branch from 463963d to c49ff7d Compare June 19, 2025 00:48
kumaraditya303
kumaraditya303 reviewed Jun 19, 2025
View reviewed changes
Modules/_heapqmodule.c
PyList_SET_ITEM(heap, 0, lastelt);
if (siftup_func((PyListObject *)heap, 0)) {
PyListObject *list = _PyList_CAST(heap);
FT_ATOMIC_STORE_PTR_RELAXED(list->ob_item[0], lastelt);
Copy link
Contributor

Choose a reason for hiding this comment

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

Similar change is needed in heappushpop and heapreplace

Copy link
Contributor

Choose a reason for hiding this comment

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

It is needed in heappushpop_max and min as well

@xuantenghxuantengh force-pushed the heap-list-lock-free-read branch from c49ff7d to 27d6cb6 Compare June 19, 2025 10:31
ZeroIntensity
ZeroIntensity reviewed Jun 19, 2025
View reviewed changes
@xuantenghxuantengh force-pushed the heap-list-lock-free-read branch from 2cc859d to 8cf4794 Compare June 19, 2025 13:17
@xuantenghxuantengh force-pushed the heap-list-lock-free-read branch from 8cf4794 to 32d767f Compare June 19, 2025 13:42
kumaraditya303
kumaraditya303 reviewed Jun 19, 2025
View reviewed changes
Sign up for free to join this conversation on . Already have an account? Sign in to comment

@mpage mpage mpage left review comments

@ZeroIntensity ZeroIntensity ZeroIntensity left review comments

@kumaraditya303 kumaraditya303kumaraditya303 left review comments

No one assigned
needs backport to 3.14bugs and security fixes
None yet
No milestone

Successfully merging this pull request may close these issues.