Skip to content

gh-112087: Make PyList_{Append,Size,GetSlice} to be thread-safe - #114651

Merged
corona10 merged 7 commits into
python:mainfrom
corona10:gh-112087-append
Jan 31, 2024
Merged

gh-112087: Make PyList_{Append,Size,GetSlice} to be thread-safe#114651
corona10 merged 7 commits into
python:mainfrom
corona10:gh-112087-append

Conversation

@corona10

@corona10corona10 commented Jan 27, 2024

Copy link
Copy Markdown
Member

@corona10

corona10 commented Jan 27, 2024

Copy link
Copy Markdown
MemberAuthor

@colesbury

  • Py_SET_SIZE: Since Py_SET_SIZE is used for mutable collections, it is worth making it the atomic API.
  • PyList_Size: This API becomes the atomic API.
  • _Py_SET_ITEMREF is added: Too many patterns exist in the list object.c, worth adding it as the utility macro. But I'm not sure that it is located in the proper place.
  • PyList_Append, PyList_GetSlice becomes thread-safe.

@corona10corona10 changed the title gh-112087: Make PyList_{Append,GetItem,Size,GetSlice} to be thread-safegh-112087: Make PyList_{Append,Size,GetSlice} to be thread-safeJan 27, 2024
Comment threadObjects/listobject.c
_PyList_AppendTakeRefListResize(PyListObject *self, PyObject *newitem)
{
Py_ssize_t len = PyList_GET_SIZE(self);
Py_ssize_t len = Py_SIZE(self);

@corona10corona10Jan 27, 2024

Copy link
Copy Markdown
MemberAuthor

Choose a reason for hiding this comment

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

It is only called by _PyList_AppendTakeRef, so it's safe.

Comment threadObjects/listobject.c Outdated
Comment threadObjects/listobject.c Outdated
Comment threadObjects/listobject.c Outdated
Comment threadInclude/object.h Outdated

@colesburycolesbury left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

LGTM

@corona10
corona10 merged commit 7b9d406 into python:mainJan 31, 2024
@corona10
corona10 deleted the gh-112087-append branch January 31, 2024 23:58
Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants

@corona10@colesbury