GV

GeezerVison

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

Home|About|Gaming cabinet|Archives|How it works|Starter sites

On this site

  • Sign in
  • ActionsCI/CD and automation
  • CodespacesCloud development environments
  • All Features
  • Documentation
  • Training
  • Certifications
  • Build with Copilot
  • Customize Copilot
  • Integrations
  • Trending repos
  • GitHub Sponsors
  • Developer articles
  • Enterprises

This view

  • Original page
  • New address

GeezerVison is showing github.com — gh-104812: Run Pending Calls in any Thread by ericsnowcurrently · Pull Request #104813 · python/cpython

Conversation

@ericsnowcurrently

@ericsnowcurrently ericsnowcurrently commented May 23, 2023 •
edited
Loading

Uh oh!

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

Copy link
Copy Markdown
Member

For a while now, pending calls only run in the main thread (in the main interpreter). This PR changes things to allow any thread run a pending call, unless the pending call was explicitly added for the main thread to run.

(For reviewers: almost all of the significant change is in ceval_gil.c. The bulk of this PR's diff is comprised of tests and a long comment about the eval breaker.)

  • Issue: Subinterpreter Pending Calls Never Run  #104812

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
All reactions
@ericsnowcurrently ericsnowcurrently added the skip news label May 23, 2023
@bedevere-bot bedevere-bot mentioned this pull request May 23, 2023
Subinterpreter Pending Calls Never Run #104812
Closed
@ericsnowcurrently
ericsnowcurrently marked this pull request as ready for review May 23, 2023 20:35
@ericsnowcurrently
ericsnowcurrently requested a review from rhettinger as a code owner May 23, 2023 20:35
@bedevere-bot bedevere-bot added the awaiting core review label May 23, 2023
@ericsnowcurrently
ericsnowcurrently removed the request for review from rhettinger May 23, 2023 20:35
@ericsnowcurrently

This comment was marked as outdated.

Sign in to view
@ericsnowcurrently
ericsnowcurrently force-pushed the per-interpreter-pending-calls branch from d05d4a0 to fe6ecc4 Compare June 2, 2023 21:16
@ericsnowcurrently
ericsnowcurrently requested a review from markshannon as a code owner June 2, 2023 21:16
ericsnowcurrently added 16 commits June 2, 2023 15:16
@ericsnowcurrently
Clear each pending call when popping it.
6e46450
@ericsnowcurrently
Call _Py_FinishPendingCalls() in Py_EndInterpreter().
3b18b7d
@ericsnowcurrently
Factor out has_pending_calls().
f8b483f
@ericsnowcurrently
Skip the calls_to_do check in _Py_FinishPendingCalls().
44df0bc
@ericsnowcurrently
Factor out _PyEval_MakePendingCalls().
e2a0281
@ericsnowcurrently
Explicitly restrict Py_MakePendingCalls() to the main thread.
61e5d3e
@ericsnowcurrently
Factor out _make_pending_calls().
5af09f2
@ericsnowcurrently
Add the mainthreadonly arg to _PyEval_AddPendingCall().
72cc242
@ericsnowcurrently
Always use the main interpreter for Py_AddPendingCall().
88f7757
@ericsnowcurrently
Add _PyRuntime.ceval.pending_mainthread.
d24fafb
@ericsnowcurrently
Run per-interpreter pending calls in any thread.
1701fa3
@ericsnowcurrently
Drop _Py_ThreadCanHandlePendingCalls().
c6cfaca
@ericsnowcurrently
Expand the handle_eval_breaker comment.
dc11024
@ericsnowcurrently
Do not require faulthandler for test.support.threading_helper.start_t…
6c3d06c 
…hreads().
@ericsnowcurrently
Add tests.
69ff9e6
@ericsnowcurrently
Add a NEWS entry.
fdde46d
@ericsnowcurrently
ericsnowcurrently force-pushed the per-interpreter-pending-calls branch from fe6ecc4 to fdde46d Compare June 2, 2023 21:19
@ericsnowcurrently ericsnowcurrently added the 🔨 test-with-buildbots Test PR w/ buildbots; report in status section label Jun 2, 2023
@bedevere-bot

bedevere-bot commented Jun 2, 2023

Copy link
Copy Markdown

🤖 New build scheduled with the buildbot fleet by @ericsnowcurrently for commit fdde46d 🤖

If you want to schedule another build, you need to add the 🔨 test-with-buildbots label again.
All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
@bedevere-bot bedevere-bot removed the 🔨 test-with-buildbots Test PR w/ buildbots; report in status section label Jun 2, 2023
@ericsnowcurrently
Merge branch 'main' into per-interpreter-pending-calls
d9924b4
ericsnowcurrently added 3 commits June 12, 2023 17:48
@ericsnowcurrently
Implement the remaining tests.
d5d7b42
@ericsnowcurrently
Drop prints.
37d41cc
@ericsnowcurrently
Ignore the global variable.
fc25a85
markshannon
markshannon reviewed Jun 13, 2023
View reviewed changes

@markshannon markshannon left a comment

Copy link
Copy Markdown
Member

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 seems fine, but I'm not an expert on this, so another review would be appreciated

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
All reactions
Comment thread Python/ceval.c Outdated
* - signal handling (only in the main thread)
*
* When the need for one of the above is detected, the eval loop
* calls _Py_HandlePending() (from ceval_gil.c). Then, if that

@markshannon markshannon Jun 13, 2023

Copy link
Copy Markdown
Member

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 wouldn't explain how it works in terms of specific functions and files, as the explanation will be out of date all too soon.

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
ericsnowcurrently reacted with thumbs up emoji
All reactions
  • 👍 1 reaction

@ericsnowcurrently ericsnowcurrently Jun 13, 2023

Copy link
Copy Markdown
Member 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.

fixed

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
All reactions
Comment thread Python/ceval.c Outdated
*
* One consequence of this approach is that it can be tricky
* to force any specific thread to pick up the eval breaker,
* or for any specific thread to not pick it up.

@markshannon markshannon Jun 13, 2023

Copy link
Copy Markdown
Member

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.

Is this true? The delay from setting the eval_breaker to the running thread responding to it should be in the order of a microsecond in the interpreter.

The problem is long running C extensions that don't check for interrupts. But that was a problem historically as well.

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
ericsnowcurrently reacted with thumbs up emoji
All reactions
  • 👍 1 reaction

@ericsnowcurrently ericsnowcurrently Jun 13, 2023

Copy link
Copy Markdown
Member 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.

I wrote from experience. 😄 It wasn't obvious to me how to write tests that ran pending calls in specific threads. I'll elaborate.

Sorry, something went wrong.

Uh oh!

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

@ericsnowcurrently ericsnowcurrently Jun 13, 2023

Copy link
Copy Markdown
Member 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.

done

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
All reactions
Comment thread Python/ceval.c Outdated
* "handle_eval_breaker" label. Some instructions come here
* explicitly (goto) and some indirectly via the CHECK_EVAL_BREAKER
* macro (see ceval_macros.h). Notably, the check happens at the
* end of the JUMP_BACKWARD instruction, which pretty much applies

@markshannon markshannon Jun 13, 2023

Copy link
Copy Markdown
Member

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.

Say, "on back edges in the control flow graph", rather than mentioning the specific bytecode.

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
ericsnowcurrently reacted with thumbs up emoji
All reactions
  • 👍 1 reaction

@ericsnowcurrently ericsnowcurrently Jun 13, 2023

Copy link
Copy Markdown
Member 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.

done

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
All reactions
Comment thread Python/ceval_gil.c
&& _Py_ThreadCanHandlePendingCalls())
| (_Py_atomic_load_relaxed_int32(&ceval2->pending.calls_to_do))
| (_Py_IsMainThread() && _Py_IsMainInterpreter(interp)
&&_Py_atomic_load_relaxed_int32(&ceval->pending_mainthread.calls_to_do))

@markshannon markshannon Jun 13, 2023

Copy link
Copy Markdown
Member

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.

COMPUTE_EVAL_BREAKER is getting unwieldy. Any thoughts on how to clean it up? (for another PR)

Sorry, something went wrong.

Uh oh!

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

@ericsnowcurrently ericsnowcurrently Jun 13, 2023

Copy link
Copy Markdown
Member 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.

I don't have any recommendations at the moment, but agree it would be worth at least thinking of alternatives. That said, the current code is fairly straightforward (a mostly flat enumeration of conditions), which helps readers to follow it at a glance.

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
All reactions
Comment thread Python/ceval_gil.c Outdated
interp = _PyInterpreterState_Main();
}
return _PyEval_AddPendingCall(interp, func, arg);
/* Legacy users of this API will continue to target the main thread. */

@markshannon markshannon Jun 13, 2023

Copy link
Copy Markdown
Member

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.

Maybe clarify?
This means the "the main thread of the main interpreter", not the "the main thread of the current interpreter"?

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
ericsnowcurrently reacted with thumbs up emoji
All reactions
  • 👍 1 reaction

@ericsnowcurrently ericsnowcurrently Jun 13, 2023

Copy link
Copy Markdown
Member 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.

done

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
All reactions
Comment thread Python/ceval_gil.c

static inline void
SIGNAL_PENDING_CALLS(PyInterpreterState *interp)
SIGNAL_PENDING_CALLS(struct _pending_calls *pending, PyInterpreterState *interp)

@markshannon markshannon Jun 13, 2023

Copy link
Copy Markdown
Member

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.

Want to make this and the unsignal version less SHOUTY, as it isn't a macro anymore?

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
ericsnowcurrently reacted with thumbs up emoji
All reactions
  • 👍 1 reaction

@markshannon markshannon Jun 13, 2023

Copy link
Copy Markdown
Member

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.

Also, the name "SIGNAL" is a bit confusing as this doesn't handle signals, but code around here does.

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
ericsnowcurrently reacted with thumbs up emoji
All reactions
  • 👍 1 reaction

@ericsnowcurrently ericsnowcurrently Jun 13, 2023

Copy link
Copy Markdown
Member 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.

The same applies to all the other static inline functions that used to be macros. I'd rather leave renaming them all to a separate change. I'm not sure the churn is worth it either.

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
All reactions
Comment thread Misc/NEWS.d/next/Core and Builtins/2023-06-02-15-15-41.gh-issue-104812.dfZiG5.rst Outdated
calls are still only done in the main thread, ergo in the main interpreter.
This change does not affect the existing public C-API
(``Py_AddPendingCall()``) which still only targets the main thread. The new
functionality is meant strictly for internal use. This change brings the

@markshannon markshannon Jun 13, 2023

Copy link
Copy Markdown
Member

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.

Why is it "meant strictly for internal use"? What bad things happen if a third party uses it?

Sorry, something went wrong.

Uh oh!

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

@ericsnowcurrently ericsnowcurrently Jun 13, 2023

Copy link
Copy Markdown
Member 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.

Pending calls can run anything and cause state changes outside the normal flow of execution. Consequently, it needs to be used carefully because it isn't clear what the potential risks are. So until we take the time to figure all that out, we are keeping it strictly internal-only.

Sorry, something went wrong.

Uh oh!

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

@ericsnowcurrently ericsnowcurrently Jun 13, 2023

Copy link
Copy Markdown
Member 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.

I've clarified the note.

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
All reactions
@ericsnowcurrently
Clarify comments.
177f161
@ericsnowcurrently

ericsnowcurrently commented Jun 13, 2023

Copy link
Copy Markdown
Member Author

Unless there are any objections, I plan on merging this later today.
All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
@ericsnowcurrently
ericsnowcurrently merged commit 757b402 into python:main Jun 13, 2023
@bedevere-bot bedevere-bot removed the awaiting core review label Jun 13, 2023
@miss-islington

miss-islington commented Jun 13, 2023

Copy link
Copy Markdown
Contributor

Thanks @ericsnowcurrently for the PR 🌮🎉.. I'm working now to backport this PR to: 3.12.
🐍🍒⛏🤖
All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
@ericsnowcurrently
ericsnowcurrently deleted the per-interpreter-pending-calls branch June 13, 2023 21:02
@ericsnowcurrently ericsnowcurrently added needs backport to 3.12 only security fixes and removed needs backport to 3.12 only security fixes labels Jun 13, 2023
@miss-islington

miss-islington commented Jun 13, 2023

Copy link
Copy Markdown
Contributor

Thanks @ericsnowcurrently for the PR 🌮🎉.. I'm working now to backport this PR to: 3.12.
🐍🍒⛏🤖
All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
@miss-islington

miss-islington commented Jun 13, 2023

Copy link
Copy Markdown
Contributor

Sorry @ericsnowcurrently, I had trouble checking out the 3.12 backport branch.
Please retry by removing and re-adding the "needs backport to 3.12" label.
Alternatively, you can backport using cherry_picker on the command line.
cherry_picker 757b402ea1c2c6b925a55a08fd844b065b6e082f 3.12
All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
@miss-islington miss-islington assigned ericsnowcurrently Jun 13, 2023
ericsnowcurrently added a commit to ericsnowcurrently/cpython that referenced this pull request Jun 13, 2023
@ericsnowcurrently
pythongh-104812: Run Pending Calls in any Thread (pythongh-104813)
0fb86c7 
For a while now, pending calls only run in the main thread (in the main interpreter).  This PR changes things to allow any thread run a pending call, unless the pending call was explicitly added for the main thread to run.
@bedevere-bot

bedevere-bot commented Jun 13, 2023

Copy link
Copy Markdown

GH-105752 is a backport of this pull request to the 3.12 branch.
All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
@bedevere-bot bedevere-bot removed the needs backport to 3.12 only security fixes label Jun 13, 2023
ericsnowcurrently added a commit to ericsnowcurrently/cpython that referenced this pull request Jun 13, 2023
@ericsnowcurrently
pythongh-104812: Run Pending Calls in any Thread (pythongh-104813)
f95336b 
For a while now, pending calls only run in the main thread (in the main interpreter).  This PR changes things to allow any thread run a pending call, unless the pending call was explicitly added for the main thread to run.
@ericsnowcurrently

ericsnowcurrently commented Jun 14, 2023

Copy link
Copy Markdown
Member Author

FYI, I'm fixing the wasm buildbots.
All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
@ericsnowcurrently ericsnowcurrently mentioned this pull request Jun 14, 2023
gh-104812: Skip Pending Calls Tests if No Threading #105761
Merged
ericsnowcurrently added a commit that referenced this pull request Jun 14, 2023
@ericsnowcurrently
[3.12] gh-104812: Run Pending Calls in any Thread (gh-104813) (gh-105752
33d3069 
)

For a while now, pending calls only run in the main thread (in the main interpreter).  This PR changes things to allow any thread run a pending call, unless the pending call was explicitly added for the main thread to run.
(cherry picked from commit 757b402)
@swtaarrs swtaarrs mentioned this pull request Jan 23, 2024
Move the eval_breaker to PyThreadState #112175
Closed
@ZeroIntensity ZeroIntensity mentioned this pull request Sep 18, 2025
Document Py_AddPendingCall() change with subinterpreters in 3.12 #139117
Merged
ZeroIntensity added a commit that referenced this pull request Sep 18, 2025
@ZeroIntensity
Document Py_AddPendingCall() change with subinterpreters in 3.12 (G…
89ff88b 
…H-139117)

Prior to 3.9, Py_AddPendingCall() would always run pending calls in the main interpreter, but then each interpreter got their own ceval state, and they were scheduled for any interpreter. In GH-104813, this was undone, so Py_AddPendingCall() would always schedule for the main interpreter.
miss-islington pushed a commit to miss-islington/cpython that referenced this pull request Sep 18, 2025
@ZeroIntensity @miss-islington
Document Py_AddPendingCall() change with subinterpreters in 3.12 (p…
ea36d03 
…ythonGH-139117)

Prior to 3.9, Py_AddPendingCall() would always run pending calls in the main interpreter, but then each interpreter got their own ceval state, and they were scheduled for any interpreter. In pythonGH-104813, this was undone, so Py_AddPendingCall() would always schedule for the main interpreter.
(cherry picked from commit 89ff88b)

Co-authored-by: Peter Bierma <[email protected]>
This was referenced Sep 18, 2025
[3.14] Document Py_AddPendingCall() change with subinterpreters in 3.12 (GH-139117) #139118
Merged
[3.13] Document Py_AddPendingCall() change with subinterpreters in 3.12 (GH-139117) #139119
Merged
miss-islington pushed a commit to miss-islington/cpython that referenced this pull request Sep 18, 2025
@ZeroIntensity @miss-islington
Document Py_AddPendingCall() change with subinterpreters in 3.12 (p…
a1ca2f5 
…ythonGH-139117)

Prior to 3.9, Py_AddPendingCall() would always run pending calls in the main interpreter, but then each interpreter got their own ceval state, and they were scheduled for any interpreter. In pythonGH-104813, this was undone, so Py_AddPendingCall() would always schedule for the main interpreter.
(cherry picked from commit 89ff88b)

Co-authored-by: Peter Bierma <[email protected]>
ZeroIntensity added a commit that referenced this pull request Sep 18, 2025
@miss-islington @ZeroIntensity
[3.13] Document Py_AddPendingCall() change with subinterpreters in …
24b6992 
…3.12 (GH-139117) (GH-139119)

Document `Py_AddPendingCall()` change with subinterpreters in 3.12 (GH-139117)

Prior to 3.9, Py_AddPendingCall() would always run pending calls in the main interpreter, but then each interpreter got their own ceval state, and they were scheduled for any interpreter. In GH-104813, this was undone, so Py_AddPendingCall() would always schedule for the main interpreter.
(cherry picked from commit 89ff88b)

Co-authored-by: Peter Bierma <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@markshannon markshannon markshannon left review comments
@vstinner vstinner Awaiting requested review from vstinner

Uh oh!

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

Assignees

@ericsnowcurrently ericsnowcurrently

Labels

skip news

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.

4 participants

@ericsnowcurrently @bedevere-bot @miss-islington @markshannon
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.