GeezerVison

The World Wide Web, restored to 800 pixels of glory.

GeezerVison is showing github.com — gh-149965: Improve thread-safety in ElementObjects within `element_ass_subscr` by clin1234 · Pull Request #149966 · python/cpython

Conversation

clin1234 commented May 17, 2026 •

edited

Loading

Uh oh!

There was an error while loading. Please reload this page.

Copy link

Copy Markdown

Contributor

Uh oh!

There was an error while loading. Please reload this page.

All reactions

bedevere-app Bot mentioned this pull request

May 17, 2026

Improve thread-safety in ElementObjects #149965

Open

bedevere-app Bot added the awaiting review label

May 17, 2026

clin1234 changed the title ~~gh-149965: Improve thread-safety in ElementObjects~~ gh-149965: Improve thread-safety in ElementObjects within element_ass_subscr

May 17, 2026

sergey-miryanov requested changes

May 18, 2026

View reviewed changes

Comment thread

Modules/_elementtree.c Outdated


		// Pointer-by-pointer memmove for PyObject** arrays that is safe
		// for shared ElementObjects in Py_GIL_DISABLED builds.
		static void

sergey-miryanov May 18, 2026

Copy link

Copy Markdown

Contributor

There was a problem hiding this comment.

Choose a reason for hiding this comment

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

I'm not sure we should duplicate ptr_wise_atomic_memmove here.

Could you please link to the related C-API Working group issue in this PR, and cross-reference it in the original issue?

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

All reactions

clin1234 May 18, 2026

Copy link

Copy Markdown

Contributor Author

There was a problem hiding this comment.

Choose a reason for hiding this comment

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

Since the function is not exposed as part of the public API, I don't think linking a C-API Working Group issue is needed here.

Anyway, it's inspired by #149936

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

All reactions

sergey-miryanov May 18, 2026

Copy link

Copy Markdown

Contributor

There was a problem hiding this comment.

Choose a reason for hiding this comment

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

I'll add a link to capi-workgroup/decisions#106 for traceability.

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

All reactions

Comment thread

Misc/NEWS.d/next/Library/2026-05-17-22-42-54.gh-issue-149965.OFW3ZD.rst Resolved

Uh oh!

There was an error while loading. Please reload this page.

bedevere-app Bot added awaiting core review and removed awaiting review labels

May 18, 2026

dimaqq reviewed

Jun 9, 2026

View reviewed changes

Comment thread

Modules/_elementtree.c Outdated

		// Pointer-by-pointer memmove for PyObject** arrays that is safe
		// for shared ElementObjects in Py_GIL_DISABLED builds.
		static void
		ptr_wise_atomic_memmove(ElementObject a, PyObject dest, PyObject *src, Py_ssize_t n)

dimaqq Jun 9, 2026 •

edited

Loading

Uh oh!

There was an error while loading. Please reload this page.

Copy link

Copy Markdown

Contributor

There was a problem hiding this comment.

Choose a reason for hiding this comment

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

This function needs own set of unit tests, for:

gil disabled
single owner
dest < src
dest > str
dest == src
- crazy, but someone will call it this way
- or an hard assertion forbidding such call
- or a short-circuit path if it's a no-on
dest != src, but ranges overlap

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

All reactions

clin1234 pushed a commit to clin1234/cpython that referenced this pull request

Jun 9, 2026


          pythongh-149966: deduplicate ptr_wise_atomic_memmove into a shared in…

a2b0094

…ternal helper and add unit tests

Extract the duplicated static `ptr_wise_atomic_memmove` from
`Modules/_elementtree.c` and `Objects/listobject.c` into a shared
`static inline _Py_ptr_wise_atomic_memmove(PyObject *a, ...)` in
`Include/internal/pycore_object.h`.  The first parameter is generalised
from a type-specific pointer to `PyObject *` since only `PyObject *`-
level operations (`_Py_IsOwnedByCurrentThread`, `_PyObject_GC_IS_SHARED`)
were ever performed on it.

`listobject.c` previously embedded `_Py_CRITICAL_SECTION_ASSERT_OBJECT_LOCKED`
inside the local helper.  That assertion is preserved by moving it
explicitly to each of the four call sites in `listobject.c` (which are
all called under a critical section).  `_elementtree.c`'s open question
about whether a critical section is needed remains unanswered, so no
assertion is added there.

Unit tests are added in `Modules/_testinternalcapi/test_ptr_wise_memmove.c`
covering: dest < src (forward copy), dest > src (backward copy),
dest == src (no-op), overlapping ranges, and the single-owner fast path.
The single-owner test explicitly clears the GC SHARED bit to guard against
freelist reuse leaving the bit set from a sibling test.

Co-Authored-By: Claude Sonnet 4.6 <[email protected]>

clin1234 requested review from a team, AA-Turner, emmatyping, erlend-aasland and itamaro as code owners

June 9, 2026 18:08

clin1234 and others added 17 commits

June 9, 2026 14:45


          Update _elementtree.c

c05a60c


          📜🤖 Added by blurb_it.

2f9d79d


          Modify critical section assertion in _elementtree.c

648be0a

Commented out critical section assertion for ElementObject.


          Update Misc/NEWS.d/next/Library/2026-05-17-22-42-54.gh-issue-149965.O…

d8e3e88

…FW3ZD.rst

Co-authored-by: Sergey Miryanov <[email protected]>


          pythongh-149966: deduplicate ptr_wise_atomic_memmove into a shared in…

a865654

…ternal helper and add unit tests

Extract the duplicated static `ptr_wise_atomic_memmove` from
`Modules/_elementtree.c` and `Objects/listobject.c` into a shared
`static inline _Py_ptr_wise_atomic_memmove(PyObject *a, ...)` in
`Include/internal/pycore_object.h`.  The first parameter is generalised
from a type-specific pointer to `PyObject *` since only `PyObject *`-
level operations (`_Py_IsOwnedByCurrentThread`, `_PyObject_GC_IS_SHARED`)
were ever performed on it.

`listobject.c` previously embedded `_Py_CRITICAL_SECTION_ASSERT_OBJECT_LOCKED`
inside the local helper.  That assertion is preserved by moving it
explicitly to each of the four call sites in `listobject.c` (which are
all called under a critical section).  `_elementtree.c`'s open question
about whether a critical section is needed remains unanswered, so no
assertion is added there.

Unit tests are added in `Modules/_testinternalcapi/test_ptr_wise_memmove.c`
covering: dest < src (forward copy), dest > src (backward copy),
dest == src (no-op), overlapping ranges, and the single-owner fast path.
The single-owner test explicitly clears the GC SHARED bit to guard against
freelist reuse leaving the bit set from a sibling test.

Co-Authored-By: Claude Sonnet 4.6 <[email protected]>


          pythonGH-54732: Tweak wording around empty lines in argument files (p…

1bfa4fd

…ython#150980)


          pythonGH-61082: Clarify nargs='*' positional default behavior (python…

0bf42e2

…#150989)


          pythongh-150285: Fix too long docstrings in the asyncio package (pyth…

42e2817

…onGH-151074)


          pythongh-150285: Fix too long docstrings in the concurrent package (p…

ace91ac

…ythonGH-151076)


          pythongh-151126: Add missing PyErr_NoMemory in _winapi module (py…

7853e5b

…thon#151154)


          pythonGH-59633: Clarify dest collisions in argparse docs (python#150987)

aa1a8c5


          pythongh-151159: Update Windows builds to use OpenSSL 3.5.7 (pythonGH…

208fe27

…-151183)


          pythongh-151159: Update macOS installer to use OpenSSL 3.5.7. (python…

b403a41

…#151171)


          pythongh-151159: Bump OpenSSL versions for iOS and Android (pythonGH-…

1a7e057

…151164)


          pythongh-151159: Update CI to use latest SSL library versions (python…

371a716

…#151176)


          pythongh-139819: rlcompleter – avoid suggesting attributes not access…

864de8d

…ible on instances (pythonGH-139820)

Co-authored-by: Ethan Furman <[email protected]>


          pythongh-151163: Update Android, macOS installer, and Windows builds …

f506aa2

…to SQLite 3.53.2 (pythonGH-151187)

clin1234 commented Jun 9, 2026

Copy link

Copy Markdown

Contributor Author

@ericsnowcurrently, mind taking a look on helping me on resolving the mutable global variable issue in Modules/_testinternalcapi/test_ptr_wise_memmove.c?

All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.


          Revert previous changes

6d506b6

picnixz commented Jun 9, 2026

Copy link

Copy Markdown

Member

I tzhink the merge was messed up. There are unrelated changes. Please re-open a cleaner PR.

All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

picnixz closed this

Jun 9, 2026

hugovk removed request for a team, 1st1, AA-Turner, asvetlov, emmatyping, ericsnowcurrently, erlend-aasland, ezio-melotti, freakboy3742, gpshead, hugovk, itamaro, kumaraditya303, mhsmith, picnixz, savannahostrowski, sethmlarson, webknjaz and willingc

June 9, 2026 22:20

clin1234 pushed a commit to clin1234/cpython that referenced this pull request

Jun 10, 2026


          pythongh-149966: deduplicate ptr_wise_atomic_memmove into a shared in…

a4c9efb

…ternal helper and add unit tests

Extract the duplicated static `ptr_wise_atomic_memmove` from
`Modules/_elementtree.c` and `Objects/listobject.c` into a shared
`static inline _Py_ptr_wise_atomic_memmove(PyObject *a, ...)` in
`Include/internal/pycore_object.h`.  The first parameter is generalised
from a type-specific pointer to `PyObject *` since only `PyObject *`-
level operations (`_Py_IsOwnedByCurrentThread`, `_PyObject_GC_IS_SHARED`)
were ever performed on it.

`listobject.c` previously embedded `_Py_CRITICAL_SECTION_ASSERT_OBJECT_LOCKED`
inside the local helper.  That assertion is preserved by moving it
explicitly to each of the four call sites in `listobject.c` (which are
all called under a critical section).  `_elementtree.c`'s open question
about whether a critical section is needed remains unanswered, so no
assertion is added there.

Unit tests are added in `Modules/_testinternalcapi/test_ptr_wise_memmove.c`
covering: dest < src (forward copy), dest > src (backward copy),
dest == src (no-op), overlapping ranges, and the single-owner fast path.
The single-owner test explicitly clears the GC SHARED bit to guard against
freelist reuse leaving the bit set from a sibling test.

Co-Authored-By: Claude Sonnet 4.6 <[email protected]>

clin1234 mentioned this pull request

Jun 10, 2026

gh-149965: promote ptr_wise_atomic_memmove to internal header #151255

Closed

clin1234 pushed a commit to clin1234/cpython that referenced this pull request

Oct 11, 2026


          pythongh-149966: deduplicate ptr_wise_atomic_memmove into a shared in…

7afcb6a

…ternal helper and add unit tests

Extract the duplicated static `ptr_wise_atomic_memmove` from
`Modules/_elementtree.c` and `Objects/listobject.c` into a shared
`static inline _Py_ptr_wise_atomic_memmove(PyObject *a, ...)` in
`Include/internal/pycore_object.h`.  The first parameter is generalised
from a type-specific pointer to `PyObject *` since only `PyObject *`-
level operations (`_Py_IsOwnedByCurrentThread`, `_PyObject_GC_IS_SHARED`)
were ever performed on it.

`listobject.c` previously embedded `_Py_CRITICAL_SECTION_ASSERT_OBJECT_LOCKED`
inside the local helper.  That assertion is preserved by moving it
explicitly to each of the four call sites in `listobject.c` (which are
all called under a critical section).  `_elementtree.c`'s open question
about whether a critical section is needed remains unanswered, so no
assertion is added there.

Unit tests are added in `Modules/_testinternalcapi/test_ptr_wise_memmove.c`
covering: dest < src (forward copy), dest > src (backward copy),
dest == src (no-op), overlapping ranges, and the single-owner fast path.
The single-owner test explicitly clears the GC SHARED bit to guard against
freelist reuse leaving the bit set from a sibling test.

Co-Authored-By: Claude Sonnet 4.6 <[email protected]>

clin1234 pushed a commit to clin1234/cpython that referenced this pull request

Oct 11, 2026


          pythongh-149966: deduplicate ptr_wise_atomic_memmove into a shared in…

c8c434c

…ternal helper and add unit tests

Extract the duplicated static `ptr_wise_atomic_memmove` from
`Modules/_elementtree.c` and `Objects/listobject.c` into a shared
`static inline _Py_ptr_wise_atomic_memmove(PyObject *a, ...)` in
`Include/internal/pycore_object.h`.  The first parameter is generalised
from a type-specific pointer to `PyObject *` since only `PyObject *`-
level operations (`_Py_IsOwnedByCurrentThread`, `_PyObject_GC_IS_SHARED`)
were ever performed on it.

`listobject.c` previously embedded `_Py_CRITICAL_SECTION_ASSERT_OBJECT_LOCKED`
inside the local helper.  That assertion is preserved by moving it
explicitly to each of the four call sites in `listobject.c` (which are
all called under a critical section).  `_elementtree.c`'s open question
about whether a critical section is needed remains unanswered, so no
assertion is added there.

Unit tests are added in `Modules/_testinternalcapi/test_ptr_wise_memmove.c`
covering: dest < src (forward copy), dest > src (backward copy),
dest == src (no-op), overlapping ranges, and the single-owner fast path.
The single-owner test explicitly clears the GC SHARED bit to guard against
freelist reuse leaving the bit set from a sibling test.

Co-Authored-By: Claude Sonnet 4.6 <[email protected]>

clin1234 added a commit to clin1234/cpython that referenced this pull request

Oct 11, 2026


          pythongh-149966: deduplicate ptr_wise_atomic_memmove into a shared in…

b0024d0

…ternal helper and add unit tests

Extract the duplicated static `ptr_wise_atomic_memmove` from
`Modules/_elementtree.c` and `Objects/listobject.c` into a shared
`static inline _Py_ptr_wise_atomic_memmove(PyObject *a, ...)` in
`Include/internal/pycore_object.h`.  The first parameter is generalised
from a type-specific pointer to `PyObject *` since only `PyObject *`-
level operations (`_Py_IsOwnedByCurrentThread`, `_PyObject_GC_IS_SHARED`)
were ever performed on it.

`listobject.c` previously embedded `_Py_CRITICAL_SECTION_ASSERT_OBJECT_LOCKED`
inside the local helper.  That assertion is preserved by moving it
explicitly to each of the four call sites in `listobject.c` (which are
all called under a critical section).  `_elementtree.c`'s open question
about whether a critical section is needed remains unanswered, so no
assertion is added there.

Unit tests are added in `Modules/_testinternalcapi/test_ptr_wise_memmove.c`
covering: dest < src (forward copy), dest > src (backward copy),
dest == src (no-op), overlapping ranges, and the single-owner fast path.
The single-owner test explicitly clears the GC SHARED bit to guard against
freelist reuse leaving the bit set from a sibling test.

Co-Authored-By: Claude Sonnet 4.6 <[email protected]>

clin1234 added a commit to clin1234/cpython that referenced this pull request

Oct 11, 2026


          pythongh-149966: deduplicate ptr_wise_atomic_memmove into a shared in…

1443d4d

…ternal helper and add unit tests

Extract the duplicated static `ptr_wise_atomic_memmove` from
`Modules/_elementtree.c` and `Objects/listobject.c` into a shared
`static inline _Py_ptr_wise_atomic_memmove(PyObject *a, ...)` in
`Include/internal/pycore_object.h`.  The first parameter is generalised
from a type-specific pointer to `PyObject *` since only `PyObject *`-
level operations (`_Py_IsOwnedByCurrentThread`, `_PyObject_GC_IS_SHARED`)
were ever performed on it.

`listobject.c` previously embedded `_Py_CRITICAL_SECTION_ASSERT_OBJECT_LOCKED`
inside the local helper.  That assertion is preserved by moving it
explicitly to each of the four call sites in `listobject.c` (which are
all called under a critical section).  `_elementtree.c`'s open question
about whether a critical section is needed remains unanswered, so no
assertion is added there.

Unit tests are added in `Modules/_testinternalcapi/test_ptr_wise_memmove.c`
covering: dest < src (forward copy), dest > src (backward copy),
dest == src (no-op), overlapping ranges, and the single-owner fast path.
The single-owner test explicitly clears the GC SHARED bit to guard against
freelist reuse leaving the bit set from a sibling test.

Co-Authored-By: Claude Sonnet 4.6 <[email protected]>

Reviewers

sergey-miryanov

sergey-miryanov requested changes

+1 more reviewer

dimaqq

dimaqq left review comments

Reviewers whose approvals may not affect merge requirements

Uh oh!

There was an error while loading. Please reload this page.

Assignees

No one assigned

Labels

awaiting core review

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

Uh oh!

There was an error while loading. Please reload this page.

10 participants

Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.

You are visitor 01402503 — GeezerVison does not store the pages you look up.
Best viewed with Netscape Navigator 4.7 or Internet Explorer 6 at 800×600. No cookies required.

GeezerVison

Conversation

clin1234 commented May 17, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

sergey-miryanov May 18, 2026

Choose a reason for hiding this comment

Uh oh!

clin1234 May 18, 2026

Choose a reason for hiding this comment

Uh oh!

sergey-miryanov May 18, 2026

Choose a reason for hiding this comment

Uh oh!

Uh oh!

dimaqq Jun 9, 2026 • 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!

clin1234 commented Jun 9, 2026

Uh oh!

picnixz commented Jun 9, 2026

Uh oh!

Reviewers

Uh oh!

Assignees

Labels

Projects

Milestone

Development

Uh oh!

10 participants

clin1234 commented May 17, 2026 •

edited

Loading

Uh oh!

There was an error while loading. Please reload this page.

dimaqq Jun 9, 2026 •

edited

Loading

Uh oh!

There was an error while loading. Please reload this page.