Skip to content

Conversation

@Fidget-Spinner
Copy link
Member

@Fidget-Spinner Fidget-Spinner commented Dec 23, 2025

@Fidget-Spinner Fidget-Spinner changed the title gh-143123: Protect against re-entrant tracer calls gh-143123: Protect against recursive tracer calls/finalization Dec 23, 2025
@markshannon
Copy link
Member

Can we have a test for this?

@Fidget-Spinner
Copy link
Member Author

Can we have a test for this?

The reproducer provided by the OP requires installation of a package to crash. I don't think we can unit test this if that's the case.

Copy link
Member

@markshannon markshannon left a comment

Choose a reason for hiding this comment

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

What is the root issue here, and does this fix it?

Is the problem that we are unable to distinguish between tracing happening in an invocation of _PyEval_EvalFrameDefault higher up the call stack and tracing happening in this invocation of _PyEval_EvalFrameDefault?

So we start/stop tracing when we shouldn't?

return 0;
}
if (_tstate->jit_tracer_state.initial_state.func == NULL) {
// gh-143123: It is possible for another function to finalize the current
Copy link
Member

Choose a reason for hiding this comment

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

What do you mean by "another function"? Which other function?

// return immediately without optimization.
return 0;
}
if (_tstate->jit_tracer_state.initial_state.func == NULL) {
Copy link
Member

Choose a reason for hiding this comment

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

We shouldn't be able to get into this state, should we?
This seems like it should be an assert and the fix should be elsewhere.

@Fidget-Spinner
Copy link
Member Author

Is the problem that we are unable to distinguish between tracing happening in an invocation of _PyEval_EvalFrameDefault higher up the call stack and tracing happening in this invocation of _PyEval_EvalFrameDefault?

Yes I suspect that's the case.

@markshannon
Copy link
Member

@Fidget-Spinner
We could add a "jit_depth" field to the thread state, initialized to 0.

  • On entering _PyEval_EvalFrameDefault: if jit_depth is non-zero, increase it by one.
  • On leaving: if jit_depth is non-zero, decrease it by one
  • Don't start tracing if jit_depth is non-zero.
  • When starting tracing, set it to 1.
  • Tracing is active if jit _depth == 1
  • Assert that it is 1 anywhere we call the optimizer, start or stop tracing, etc.

Sign up for free to 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