| FazBrowse GitHub Viewer | Trending | | Home |
| Tools: [Download Repo ZIP] [Original HTTPS Page] |
Sorry, something went wrong.
…FW3ZD.rst Co-authored-by: Sergey Miryanov <sergey.miryanov@gmail.com>
…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 <noreply@anthropic.com>
Commented out critical section assertion for ElementObject.
…e-149965.edk2ut.rst
This reverts commit a3a7eee.
|
@ZeroIntensity, I've reverted the header change and unit test additions, so my current PR is focused on only one file. |
Sorry, something went wrong.
|
I think you misunderstood what I meant. You can move the function to a header (and in fact, you should, because otherwise we're duplicating code), but it was weird to do it in its own PR. Just combine this PR with #151255 and it'll be fine. |
Sorry, something went wrong.
|
@ZeroIntensity, mind taking a look again? |
Sorry, something went wrong.
There was a problem hiding this comment.
Can you add a multithreaded test stressing this path?
Sorry, something went wrong.
| // Pointer-by-pointer memmove for PyObject** arrays that is safe for shared | ||
| // objects in Py_GIL_DISABLED builds. Locking is the caller's responsibility. | ||
| static inline void | ||
| _Py_ptr_wise_atomic_memmove(PyObject *a, PyObject **dest, PyObject **src, |
There was a problem hiding this comment.
We should probably prefix this with _PyObject instead of plain _Py.
Sorry, something went wrong.
| _Py_ptr_wise_atomic_memmove( | ||
| (PyObject *)self, |
There was a problem hiding this comment.
To avoid needing to cast on every use, we can turn this into a macro that uses _PyObject_CAST. Here's an outline:
#define _Py_ptr_wise_atomic_memmove(a, dest, src, n) _Py_ptr_wise_atomic_memmove(_PyObject_CAST(a), dest, src, n)
Sorry, something went wrong.
Where would the multithreaded test lie within the source tree, considering that it's an internal function not exported to the public headers? |
Sorry, something went wrong.
|
Do you think a News fragment is needed here? |
Sorry, something went wrong.
I meant for the _elementtree code that you're fixing.
Yes; xml.etree.ElementTree is a user-facing module. |
Sorry, something went wrong.
Any pointers on creating such a multi-threaded test in the native _elementtree module? |
Sorry, something went wrong.
|
Add something to test_xml_etree.py or test_xml_etree_c.py. |
Sorry, something went wrong.
| Back | FazBrowse Home | New Git URL |
See also capi-workgroup/decisions#106