Skip to content

Commit e2f0160

Browse files
gh-143604: Hold strong reference to executor during JIT tracing (GH-143646)
Co-authored-by: Ken Jin <kenjin4096@gmail.com>
1 parent e7f5ffa commit e2f0160

File tree

10 files changed

+41
-8
lines changed

10 files changed

+41
-8
lines changed

Include/internal/pycore_optimizer.h

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -222,7 +222,7 @@ extern void _PyExecutor_Free(_PyExecutorObject *self);
222222

223223
PyAPI_FUNC(int) _PyDumpExecutors(FILE *out);
224224
#ifdef _Py_TIER2
225-
extern void _Py_ClearExecutorDeletionList(PyInterpreterState *interp);
225+
PyAPI_FUNC(void) _Py_ClearExecutorDeletionList(PyInterpreterState *interp);
226226
#endif
227227

228228
int _PyJit_translate_single_bytecode_to_trace(PyThreadState *tstate, _PyInterpreterFrame *frame, _Py_CODEUNIT *next_instr, int stop_tracing_opcode);
@@ -231,7 +231,7 @@ PyAPI_FUNC(int)
231231
_PyJit_TryInitializeTracing(PyThreadState *tstate, _PyInterpreterFrame *frame,
232232
_Py_CODEUNIT *curr_instr, _Py_CODEUNIT *start_instr,
233233
_Py_CODEUNIT *close_loop_instr, int curr_stackdepth, int chain_depth, _PyExitData *exit,
234-
int oparg);
234+
int oparg, _PyExecutorObject *current_executor);
235235

236236
void _PyJit_FinalizeTracing(PyThreadState *tstate);
237237
void _PyJit_TracerFree(_PyThreadStateImpl *_tstate);

Include/internal/pycore_tstate.h

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -31,6 +31,7 @@ typedef struct _PyJitTracerInitialState {
3131
struct _PyExitData *exit;
3232
PyCodeObject *code; // Strong
3333
PyFunctionObject *func; // Strong
34+
struct _PyExecutorObject *executor; // Strong
3435
_Py_CODEUNIT *start_instr;
3536
_Py_CODEUNIT *close_loop_instr;
3637
_Py_CODEUNIT *jump_backward_instr;

Lib/test/test_capi/test_opt.py

Lines changed: 13 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -139,6 +139,19 @@ def f():
139139
exe = get_first_executor(f)
140140
self.assertIsNone(exe)
141141

142+
def test_prev_executor_freed_while_tracing(self):
143+
def f(start, end, way):
144+
for x in range(start, end):
145+
# For the first trace, create a bad branch on purpose to trace into.
146+
# A side exit will form from here on the second trace.
147+
y = way + way
148+
if x >= TIER2_THRESHOLD:
149+
# Invalidate the first trace while tracing the second.
150+
_testinternalcapi.invalidate_executors(f.__code__)
151+
_testinternalcapi.clear_executor_deletion_list()
152+
f(0, TIER2_THRESHOLD, 1)
153+
f(1, TIER2_THRESHOLD + 1, 1.0)
154+
142155

143156
@requires_specialization
144157
@unittest.skipIf(Py_GIL_DISABLED, "optimizer not yet supported in free-threaded builds")
Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,2 @@
1+
Fix a reference counting issue in the JIT tracer where the current executor
2+
could be prematurely freed during tracing.

Modules/_testinternalcapi.c

Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1245,6 +1245,14 @@ invalidate_executors(PyObject *self, PyObject *obj)
12451245
Py_RETURN_NONE;
12461246
}
12471247

1248+
static PyObject *
1249+
clear_executor_deletion_list(PyObject *self, PyObject *Py_UNUSED(ignored))
1250+
{
1251+
PyInterpreterState *interp = PyInterpreterState_Get();
1252+
_Py_ClearExecutorDeletionList(interp);
1253+
Py_RETURN_NONE;
1254+
}
1255+
12481256
static PyObject *
12491257
get_exit_executor(PyObject *self, PyObject *arg)
12501258
{
@@ -2562,6 +2570,7 @@ static PyMethodDef module_functions[] = {
25622570
#ifdef _Py_TIER2
25632571
{"add_executor_dependency", add_executor_dependency, METH_VARARGS, NULL},
25642572
{"invalidate_executors", invalidate_executors, METH_O, NULL},
2573+
{"clear_executor_deletion_list", clear_executor_deletion_list, METH_NOARGS, NULL},
25652574
{"get_exit_executor", get_exit_executor, METH_O, NULL},
25662575
#endif
25672576
{"pending_threadfunc", _PyCFunction_CAST(pending_threadfunc),

Python/bytecodes.c

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -2990,7 +2990,7 @@ dummy_func(
29902990
oparg >>= 8;
29912991
insert_exec_at--;
29922992
}
2993-
int succ = _PyJit_TryInitializeTracing(tstate, frame, this_instr, insert_exec_at, next_instr, STACK_LEVEL(), 0, NULL, oparg);
2993+
int succ = _PyJit_TryInitializeTracing(tstate, frame, this_instr, insert_exec_at, next_instr, STACK_LEVEL(), 0, NULL, oparg, NULL);
29942994
if (succ) {
29952995
ENTER_TRACING();
29962996
}
@@ -5525,7 +5525,7 @@ dummy_func(
55255525
// Note: it's safe to use target->op.arg here instead of the oparg given by EXTENDED_ARG.
55265526
// The invariant in the optimizer is the deopt target always points back to the first EXTENDED_ARG.
55275527
// So setting it to anything else is wrong.
5528-
int succ = _PyJit_TryInitializeTracing(tstate, frame, target, target, target, STACK_LEVEL(), chain_depth, exit, target->op.arg);
5528+
int succ = _PyJit_TryInitializeTracing(tstate, frame, target, target, target, STACK_LEVEL(), chain_depth, exit, target->op.arg, previous_executor);
55295529
exit->temperature = restart_backoff_counter(exit->temperature);
55305530
if (succ) {
55315531
GOTO_TIER_ONE_CONTINUE_TRACING(target);

Python/ceval.c

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1475,7 +1475,7 @@ stop_tracing_and_jit(PyThreadState *tstate, _PyInterpreterFrame *frame)
14751475
tracer->initial_state.jump_backward_instr[1].counter = initial_jump_backoff_counter(&_tstate->policy);
14761476
}
14771477
}
1478-
else {
1478+
else if (tracer->initial_state.executor->vm_data.valid) {
14791479
// Likewise, we hold a strong reference to the executor containing this exit, so the exit is guaranteed
14801480
// to be valid to access.
14811481
if (err <= 0) {

Python/executor_cases.c.h

Lines changed: 1 addition & 1 deletion
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

Python/generated_cases.c.h

Lines changed: 1 addition & 1 deletion
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

Python/optimizer.c

Lines changed: 9 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -138,6 +138,12 @@ _PyOptimizer_Optimize(
138138
// return immediately without optimization.
139139
return 0;
140140
}
141+
_PyExecutorObject *prev_executor = _tstate->jit_tracer_state->initial_state.executor;
142+
if (prev_executor != NULL && !prev_executor->vm_data.valid) {
143+
// gh-143604: If we are a side exit executor and the original executor is no
144+
// longer valid, don't compile to prevent a reference leak.
145+
return 0;
146+
}
141147
assert(!interp->compiling);
142148
assert(_tstate->jit_tracer_state->initial_state.stack_depth >= 0);
143149
#ifndef Py_GIL_DISABLED
@@ -1015,7 +1021,7 @@ Py_NO_INLINE int
10151021
_PyJit_TryInitializeTracing(
10161022
PyThreadState *tstate, _PyInterpreterFrame *frame, _Py_CODEUNIT *curr_instr,
10171023
_Py_CODEUNIT *start_instr, _Py_CODEUNIT *close_loop_instr, int curr_stackdepth, int chain_depth,
1018-
_PyExitData *exit, int oparg)
1024+
_PyExitData *exit, int oparg, _PyExecutorObject *current_executor)
10191025
{
10201026
_PyThreadStateImpl *_tstate = (_PyThreadStateImpl *)tstate;
10211027
if (_tstate->jit_tracer_state == NULL) {
@@ -1062,6 +1068,7 @@ _PyJit_TryInitializeTracing(
10621068
tracer->initial_state.close_loop_instr = close_loop_instr;
10631069
tracer->initial_state.code = (PyCodeObject *)Py_NewRef(code);
10641070
tracer->initial_state.func = (PyFunctionObject *)Py_NewRef(func);
1071+
tracer->initial_state.executor = (_PyExecutorObject *)Py_XNewRef(current_executor);
10651072
tracer->initial_state.exit = exit;
10661073
tracer->initial_state.stack_depth = curr_stackdepth;
10671074
tracer->initial_state.chain_depth = chain_depth;
@@ -1089,6 +1096,7 @@ _PyJit_FinalizeTracing(PyThreadState *tstate)
10891096
_PyJitTracerState *tracer = _tstate->jit_tracer_state;
10901097
Py_CLEAR(tracer->initial_state.code);
10911098
Py_CLEAR(tracer->initial_state.func);
1099+
Py_CLEAR(tracer->initial_state.executor);
10921100
Py_CLEAR(tracer->prev_state.instr_code);
10931101
tracer->prev_state.code_curr_size = CODE_SIZE_EMPTY;
10941102
tracer->prev_state.code_max_size = UOP_MAX_TRACE_LENGTH/2 - 1;

0 commit comments

Comments
 (0)