Skip to content

gh-127065: Make methodcaller thread-safe and re-entrant (v2) - #127746

Merged
colesbury merged 29 commits into
python:mainfrom
eendebakpt:methodcaller_ft_v2
Dec 11, 2024
Merged

gh-127065: Make methodcaller thread-safe and re-entrant (v2)#127746
colesbury merged 29 commits into
python:mainfrom
eendebakpt:methodcaller_ft_v2

Conversation

@eendebakpt

@eendebakpteendebakpt commented Dec 8, 2024

Copy link
Copy Markdown
Contributor

Comment threadModules/_operator.c
Py_VISIT(mc->args);
Py_VISIT(mc->kwds);
Py_VISIT(mc->vectorcall_args);
Py_VISIT(mc->vectorcall_kwnames);

Copy link
Copy Markdown
ContributorAuthor

Choose a reason for hiding this comment

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

Note mc->vectorcall_kwnames is a tuple of strings, but the strings can be subclasses of str, so we need to visit this.

@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.

Looks good. A few small comments below

Comment threadModules/_operator.c Outdated
Comment threadModules/_operator.c Outdated
Comment threadModules/_operator.c Outdated
Comment threadModules/_operator.c 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. Thanks for fixing this!

@colesbury
colesbury merged commit b0f278f into python:mainDec 11, 2024
srinivasreddy pushed a commit to srinivasreddy/cpython that referenced this pull request Jan 8, 2025
…GH-127746)
The function `operator.methodcaller` was not thread-safe since the additional
of the vectorcall method in pythongh-89013. In the free threading build the issue
is easy to trigger, for the normal build harder.
This makes the `methodcaller` safe by:
* Replacing the lazy initialization with initialization in the constructor.
* Using a stack allocated space for the vectorcall arguments and falling back
to `tp_call` for calls with more than 8 arguments.
Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants

@eendebakpt@colesbury