GeezerVison

Note that this wouldn't solve the cold executor issue. It would only prevent an assertion failure in case we branch here (but if we branch here, we're already in a very bad state)

@picnixz Thanks for the context regarding the "cold executor issue".
Just to clarify the scope of this PR: my understanding is that this is a larger, separate problem to be handled in a follow-up. Is that correct, or should I look into addressing it here as well?

It's a larger and more complex issue. The fact that we have an immortal cold executor seems to make normal executors leak. I don't know if it's because we reset executors in exit sides to a cold executor that then makes that specific executor not garbage-collectable (this is concerning). Your PR would still be fine but I don't know if it won't be eventually rechanged because of the fix we need to do on the other side.

Sorry but I have the same PR up here https://github.com/python/cpython/pull/137016/files, since July of this year

Considering this is a duplicate of gh-137016, I'm closing this PR. Sorry!

For the other leaks, please open a separate PR. please wait for Mark and other JIT experts to give their opinion because we can't jump on the task without understanding how to properly solve it.

@Fidget-Spinner Do you mind incorporating some observations from this PR in yours? I don't know whether the change is correct here when changing current_executor etc.

Hi @Fidget-Spinner ,

Thanks for pointing that out and linking your PR. I've taken a close look at both.

While our changes overlap in optimizer.c, they actually address two different underlying issues. Your PR #137016 correctly fixes a GC assertion failure by moving _PyObject_GC_TRACK to an earlier point. It's a great catch!

However, this PR primarily addresses a critical reference counting leak that was detectable with ASan. The root cause was that the executor's refcount was not decremented when Tier 2 code exited with a Python exception set (but didn't fatally error out by returning NULL). The main fix here is adding Py_DECREF(EXECUTOR) inside the TIER1_TO_TIER2 macro in ceval_macros.h to ensure the reference is always released.

My PR also includes the same _PyObject_GC_TRACK move that you proposed, so it appears to fix both the memory leak from issue #139540 and the GC assertion issue that your PR addresses.

I believe this PR provides a more complete solution for the stability issues in this code path.

I don't know whether the change is correct here when changing current_executor etc

These changes are correct and necessary. tstate->current_executor acts as a temporary channel to pass the executor object from the _ENTER_EXECUTOR instruction to the new Tier 2 frame, which is set up in _PyEval_EvalFrameDefault.

The intended lifecycle is:

1. _ENTER_EXECUTOR (in bytecodes.c) sets tstate->current_executor.
2. _PyEval_EvalFrameDefault consumes it at the very start of the new frame, creates a PyStackRef for it, and immediately sets tstate->current_executor back to NULL.

The other NULL/CLEAR additions in ceval.c and pystate.c are for robustness, ensuring we don't leak the executor if a frame or thread is torn down unexpectedly. This appears to be the established design for this handover.

The root cause was that the executor's refcount was not decremented when Tier 2 code exited with a Python exception set (but didn't fatally error out by returning NULL). The main fix here is adding Py_DECREF(EXECUTOR) inside the TIER1_TO_TIER2 macro in ceval_macros.h to ensure the reference is always released.

Does it fix the leak as in the issue? that is, does it fix the refcounting? I can't check this now, but please check whether the reproducer (without ASAN) is fixed when you run ./python -X showrefcount repro.py.