Skip to content

GH-104584: Fix ENTER_EXECUTOR #106141

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 5 commits into from
Jul 3, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 2 additions & 0 deletions Include/internal/pycore_ceval.h
Original file line number Diff line number Diff line change
Expand Up @@ -152,6 +152,8 @@ extern struct _PyInterpreterFrame* _PyEval_GetFrame(void);

extern PyObject* _Py_MakeCoro(PyFunctionObject *func);

/* Handle signals, pending calls, GIL drop request
and asynchronous exception */
extern int _Py_HandlePending(PyThreadState *tstate);


Expand Down
19 changes: 12 additions & 7 deletions Python/bytecodes.c
Original file line number Diff line number Diff line change
Expand Up @@ -141,8 +141,8 @@ dummy_func(
ERROR_IF(err, error);
next_instr--;
}
else if (_Py_atomic_load_relaxed_int32(&tstate->interp->ceval.eval_breaker) && oparg < 2) {
goto handle_eval_breaker;
else if (oparg < 2) {
CHECK_EVAL_BREAKER();
}
}

Expand All @@ -158,6 +158,9 @@ dummy_func(
next_instr--;
}
else {
if (oparg < 2) {
CHECK_EVAL_BREAKER();
}
_PyFrame_SetStackPointer(frame, stack_pointer);
int err = _Py_call_instrumentation(
tstate, oparg > 0, frame, next_instr-1);
Expand All @@ -168,9 +171,6 @@ dummy_func(
next_instr = frame->prev_instr;
DISPATCH();
}
if (_Py_atomic_load_relaxed_int32(&tstate->interp->ceval.eval_breaker) && oparg < 2) {
goto handle_eval_breaker;
}
}
}

Expand Down Expand Up @@ -2223,6 +2223,7 @@ dummy_func(
}

inst(JUMP_BACKWARD, (--)) {
CHECK_EVAL_BREAKER();
_Py_CODEUNIT *here = next_instr - 1;
assert(oparg <= INSTR_OFFSET());
JUMPBY(1-oparg);
Expand All @@ -2240,7 +2241,6 @@ dummy_func(
goto resume_frame;
}
#endif /* ENABLE_SPECIALIZATION */
CHECK_EVAL_BREAKER();
}

pseudo(JUMP) = {
Expand All @@ -2254,8 +2254,13 @@ dummy_func(
};

inst(ENTER_EXECUTOR, (--)) {
CHECK_EVAL_BREAKER();

PyCodeObject *code = _PyFrame_GetCode(frame);
_PyExecutorObject *executor = (_PyExecutorObject *)code->co_executors->executors[oparg&255];
int original_oparg = executor->vm_data.oparg | (oparg & 0xfffff00);
JUMPBY(1-original_oparg);
frame->prev_instr = next_instr - 1;
Py_INCREF(executor);
frame = executor->execute(executor, frame, stack_pointer);
if (frame == NULL) {
Expand Down Expand Up @@ -3570,8 +3575,8 @@ dummy_func(
}

inst(INSTRUMENTED_JUMP_BACKWARD, ( -- )) {
INSTRUMENTED_JUMP(next_instr-1, next_instr+1-oparg, PY_MONITORING_EVENT_JUMP);
CHECK_EVAL_BREAKER();
INSTRUMENTED_JUMP(next_instr-1, next_instr+1-oparg, PY_MONITORING_EVENT_JUMP);
}

inst(INSTRUMENTED_POP_JUMP_IF_TRUE, ( -- )) {
Expand Down
70 changes: 1 addition & 69 deletions Python/ceval.c
Original file line number Diff line number Diff line change
Expand Up @@ -763,68 +763,6 @@ _PyEval_EvalFrameDefault(PyThreadState *tstate, _PyInterpreterFrame *frame, int

DISPATCH();

handle_eval_breaker:

/* Do periodic things, like check for signals and async I/0.
* We need to do reasonably frequently, but not too frequently.
* All loops should include a check of the eval breaker.
* We also check on return from any builtin function.
*
* ## More Details ###
*
* The eval loop (this function) normally executes the instructions
* of a code object sequentially. However, the runtime supports a
* number of out-of-band execution scenarios that may pause that
* sequential execution long enough to do that out-of-band work
* in the current thread using the current PyThreadState.
*
* The scenarios include:
*
* - cyclic garbage collection
* - GIL drop requests
* - "async" exceptions
* - "pending calls" (some only in the main thread)
* - signal handling (only in the main thread)
*
* When the need for one of the above is detected, the eval loop
* pauses long enough to handle the detected case. Then, if doing
* so didn't trigger an exception, the eval loop resumes executing
* the sequential instructions.
*
* To make this work, the eval loop periodically checks if any
* of the above needs to happen. The individual checks can be
* expensive if computed each time, so a while back we switched
* to using pre-computed, per-interpreter variables for the checks,
* and later consolidated that to a single "eval breaker" variable
* (now a PyInterpreterState field).
*
* For the longest time, the eval breaker check would happen
* frequently, every 5 or so times through the loop, regardless
* of what instruction ran last or what would run next. Then, in
* early 2021 (gh-18334, commit 4958f5d), we switched to checking
* the eval breaker less frequently, by hard-coding the check to
* specific places in the eval loop (e.g. certain instructions).
* The intent then was to check after returning from calls
* and on the back edges of loops.
*
* In addition to being more efficient, that approach keeps
* the eval loop from running arbitrary code between instructions
* that don't handle that well. (See gh-74174.)
*
* Currently, the eval breaker check happens here at the
* "handle_eval_breaker" label. Some instructions come here
* explicitly (goto) and some indirectly. Notably, the check
* happens on back edges in the control flow graph, which
* pretty much applies to all loops and most calls.
* (See bytecodes.c for exact information.)
*
* One consequence of this approach is that it might not be obvious
* how to force any specific thread to pick up the eval breaker,
* or for any specific thread to not pick it up. Mostly this
* involves judicious uses of locks and careful ordering of code,
* while avoiding code that might trigger the eval breaker
* until so desired.
*/
if (_Py_HandlePending(tstate) != 0) {
goto error;
}
Expand Down Expand Up @@ -2794,13 +2732,7 @@ _PyUopExecute(_PyExecutorObject *executor, _PyInterpreterFrame *frame, PyObject
PyThreadState *tstate = _PyThreadState_GET();
_PyUOpExecutorObject *self = (_PyUOpExecutorObject *)executor;

// Equivalent to CHECK_EVAL_BREAKER()
_Py_CHECK_EMSCRIPTEN_SIGNALS_PERIODICALLY();
if (_Py_atomic_load_relaxed_int32(&tstate->interp->ceval.eval_breaker)) {
if (_Py_HandlePending(tstate) != 0) {
goto error;
}
}
CHECK_EVAL_BREAKER();

OBJECT_STAT_INC(optimization_traces_executed);
_Py_CODEUNIT *ip_offset = (_Py_CODEUNIT *)_PyFrame_GetCode(frame)->co_code_adaptive - 1;
Expand Down
61 changes: 59 additions & 2 deletions Python/ceval_gil.c
Original file line number Diff line number Diff line change
Expand Up @@ -1052,8 +1052,65 @@ _PyEval_FiniState(struct _ceval_state *ceval)
}
}

/* Handle signals, pending calls, GIL drop request
and asynchronous exception */

/* Do periodic things, like check for signals and async I/0.
* We need to do reasonably frequently, but not too frequently.
* All loops should include a check of the eval breaker.
* We also check on return from any builtin function.
*
* ## More Details ###
*
* The eval loop (this function) normally executes the instructions
* of a code object sequentially. However, the runtime supports a
* number of out-of-band execution scenarios that may pause that
* sequential execution long enough to do that out-of-band work
* in the current thread using the current PyThreadState.
*
* The scenarios include:
*
* - cyclic garbage collection
* - GIL drop requests
* - "async" exceptions
* - "pending calls" (some only in the main thread)
* - signal handling (only in the main thread)
*
* When the need for one of the above is detected, the eval loop
* pauses long enough to handle the detected case. Then, if doing
* so didn't trigger an exception, the eval loop resumes executing
* the sequential instructions.
*
* To make this work, the eval loop periodically checks if any
* of the above needs to happen. The individual checks can be
* expensive if computed each time, so a while back we switched
* to using pre-computed, per-interpreter variables for the checks,
* and later consolidated that to a single "eval breaker" variable
* (now a PyInterpreterState field).
*
* For the longest time, the eval breaker check would happen
* frequently, every 5 or so times through the loop, regardless
* of what instruction ran last or what would run next. Then, in
* early 2021 (gh-18334, commit 4958f5d), we switched to checking
* the eval breaker less frequently, by hard-coding the check to
* specific places in the eval loop (e.g. certain instructions).
* The intent then was to check after returning from calls
* and on the back edges of loops.
*
* In addition to being more efficient, that approach keeps
* the eval loop from running arbitrary code between instructions
* that don't handle that well. (See gh-74174.)
*
* Currently, the eval breaker check happens on back edges in
* the control flow graph, which pretty much applies to all loops,
* and most calls.
* (See bytecodes.c for exact information.)
*
* One consequence of this approach is that it might not be obvious
* how to force any specific thread to pick up the eval breaker,
* or for any specific thread to not pick it up. Mostly this
* involves judicious uses of locks and careful ordering of code,
* while avoiding code that might trigger the eval breaker
* until so desired.
*/
int
_Py_HandlePending(PyThreadState *tstate)
{
Expand Down
4 changes: 3 additions & 1 deletion Python/ceval_macros.h
Original file line number Diff line number Diff line change
Expand Up @@ -117,7 +117,9 @@
#define CHECK_EVAL_BREAKER() \
_Py_CHECK_EMSCRIPTEN_SIGNALS_PERIODICALLY(); \
if (_Py_atomic_load_relaxed_int32(&tstate->interp->ceval.eval_breaker)) { \
goto handle_eval_breaker; \
if (_Py_HandlePending(tstate) != 0) { \
goto error; \
} \
}


Expand Down
Loading