classification
Title: PEP 523: Add private _PyInterpreterState_SetEvalFrameFunc() function to the C API (Python 3.8 regression)
Type: enhancement Stage: resolved
Components: Interpreter Core Versions: Python 3.9
process
Status: closed Resolution: fixed
Dependencies: Superseder:
Assigned To: Nosy List: Mark.Shannon, brett.cannon, dino.viehland, eric.snow, fabioz, gregory.p.smith, lukasz.langa, phsilva, vstinner
Priority: critical Keywords: 3.8regression, patch

Created on 2019-10-16 16:17 by fabioz, last changed 2020-03-19 03:26 by vstinner. This issue is now closed.

Pull Requests
URL Status Linked Edit
PR 17187 closed vstinner, 2019-11-18 19:06
PR 17340 merged vstinner, 2019-11-22 13:05
PR 17352 closed vstinner, 2019-11-22 18:06
Messages (46)
msg354803 - (view) Author: Fabio Zadrozny (fabioz) * Date: 2019-10-16 16:17
In CPython 3.7 it was possible to do:

#include "pystate.h"
...
PyThreadState *ts = PyThreadState_Get();
PyInterpreterState *interp = ts->interp;
interp->eval_frame = my_frame_eval_func;

This is no longer possible because in 3.8 the PyInterpreterState is opaque, so, Py_BUILD_CORE_MODULE needs to be defined defined and "internal/pycore_pystate.h" must be included to set PyInterpreterState.eval_frame.

This works but isn't ideal -- maybe there could be a function to set PyInterpreterState.eval_frame?
msg355845 - (view) Author: Eric Snow (eric.snow) * (Python committer) Date: 2019-11-01 21:24
It depends on how you look at the degree to which you are interacting with the runtime.  This is a fairly low-level hook into the runtime.  So arguably if you are using this API then you should specify being a "core" extension.  That said, getting that clever about it is a bit too much.  The authors or PEP 523 can correct me if I'm wrong, but it seems like there isn't a good reason to restrict access.

So basically, I agree with you. :)

How about one of the following?

* _PyInterpreterState_SetEvalFrame(_PyFrameEvalFunction eval_frame)
* _PyInterpreterState_SetFrameEval(_PyFrameEvalFunction eval_frame)

The underscore basically says "don't use this unless you know what you are doing".  Or perhaps that is overkill too?  "_PyFrameEvalFunction" has an underscore, so perhaps not.

Also, it would make sense to have a matching getter.
msg355849 - (view) Author: Dino Viehland (dino.viehland) * (Python committer) Date: 2019-11-01 22:17
Adding the getter/setters seems perfectly reasonable to me, and I agree they should be underscore prefixed as well.
msg355933 - (view) Author: Mark Shannon (Mark.Shannon) * (Python committer) Date: 2019-11-04 10:03
I'm strongly opposed to this change.

PEP 523 does not specify what the semantics of changing the interpreter frame evaluator actually is:
Is the VM obliged to call the new interpreter?
What happens if the custom evaluator leaves the VM in a inconsistent state?
Does the VM have to roll back any speculative optimisations it has made? What happens if it the evaluator is changed multiple times by different modules?
What if the evaluator is changed when a coroutine or generator is suspended, or in another thread?
I could go on...

IMO this punches a big hole in the Python execution model, but provides no benefit.
msg355934 - (view) Author: Mark Shannon (Mark.Shannon) * (Python committer) Date: 2019-11-04 10:03
Fabio,

OOI what are you trying achieve by changing the frame evaluator?
msg355936 - (view) Author: Fabio Zadrozny (fabioz) * Date: 2019-11-04 11:07
@Mark Shannon what I do is change the code object of the frame about to be evaluated to add a programmatic breakpoint, to avoid the need to have the trace function set at contexts that would need to be traced (after changing the frame.f_code it goes on to call the regular _PyEval_EvalFrameDefault), so, the user-code runs at full speed on all contexts (there's still added overhead on a function call to decide if the code object needs to be changed, but that'd happen on the regular tracing code too).

Note that this does not change the semantics of anything as it calls the regular _PyEval_EvalFrameDefault, so, the worries you're listing shouldn't be a concern in this particular use-case.

Also note that until Python 3.7 this was easy to change, and that's still possible in Python 3.8 (the only thing is that now it's less straightforward).

Note that my use is much simpler that the original intent of the frame evaluator -- my use case could be solved by having a callback to change the code object before the frame execution -- but as far as I know, right now, the way to do that is through the frame evaluation API.
msg355971 - (view) Author: Brett Cannon (brett.cannon) * (Python committer) Date: 2019-11-04 18:43
@Mark are you strongly opposed because we're providing an API for changing the eval function in the CPython API and you think it should be in the private API? Or you objecting to PEP 523 all-up (based on your list of objections)? Either way the PEP was accepted and implemented a while ago and so I'm not quite sure what you are expecting as an outcome short of a repeal of PEP 523 which would require a separate PEP.
msg355990 - (view) Author: STINNER Victor (vstinner) * (Python committer) Date: 2019-11-05 01:14
> IMO this punches a big hole in the Python execution model, but provides no benefit.

This PEP is about fixing a Python 3.8 regression. In Python 3.7, it was possible to get and set frame_eval. In Python 3.8, it's no longer possible.

One option to fix the regression would be to again expose PyInterpreterState structure... but we are trying to do the opposite: hide more structures, not expose more structures :-/

IMHO private getter and setter functions are perfectly fine. Please ensure that the setter can report an issue. We have too many setters which cannot report an error which is very painful :-(
msg356196 - (view) Author: Mark Shannon (Mark.Shannon) * (Python committer) Date: 2019-11-07 16:02
Victor,
I don't think this is a regression.
`PyThreadState` is an internal opaque data structure, which means we are free to change it.
That the `eval_frame` is hard to access is a feature not a bug, as it discourages misuse and enables us to remove it easily, when a better approach becomes available.

PEP 523 is quite vague, but the rationale indicates that exposing `eval_frame` is for "a method-level JIT". PEP 523 did not suggest adding an API. If it had (and I had had the time) I would have opposed it more vigorously.

IMO, the correct way to change the code object is to set `function.__code__` which can be done easily from either Python or C code.

@Fabioz, is there anything preventing you from doing that?
msg356197 - (view) Author: Fabio Zadrozny (fabioz) * Date: 2019-11-07 16:14
@Mark I don't want to change the original function code, I just want to change the code to be executed in the frame (i.e.: as breakpoints change things may be different).

Changing the actual function code is a no-go since changing the real function code can break valid user code.
msg356200 - (view) Author: STINNER Victor (vstinner) * (Python committer) Date: 2019-11-07 16:37
By the way, it's still possible to access directly ts->interp if you include "pycore_pystate.h" header, which requires to define Py_BUILD_CORE_MODULE. This header is an internal header.
msg356215 - (view) Author: Brett Cannon (brett.cannon) * (Python committer) Date: 2019-11-07 23:13
PEP 523 was to give user code the ability to change the eval function. While the work was motivated by our JIT work, supporting debugging was another motivating factor: https://www.python.org/dev/peps/pep-0523/#debugging. There's no C API because at the time there was no need as PyInterpreterState was publicly exposed.

I don't think anyone is suggesting to add something to the stable ABI, so this comes down to whether this should be exposed as part of the CPython API or the publicly accessible internal API. Since there is no distinction of "you probably don't want to use this but we won't yank it out from underneath" you I think this belongs in the CPython API somehow. Otherwise someone should propose withdrawing PEP 523 as I think that shifts what the PEP was aiming for. You can also ask on python-dev or the steering council (which I will abstain myself from due to being a co-author of PEP 523).
msg356216 - (view) Author: STINNER Victor (vstinner) * (Python committer) Date: 2019-11-07 23:44
I heard that for debuggers, the PEP 523 is really useful. I recall a huge speedup thanks to this PEP in Jetbrain:
https://blog.jetbrains.com/pycharm/2017/03/inside-the-debugger-interview-with-elizaveta-shashkova/
msg356255 - (view) Author: Mark Shannon (Mark.Shannon) * (Python committer) Date: 2019-11-08 19:10
It sounds to me like `PyInterpreterState.eval_frame` is being used to lazily modify the bytecode to support breakpoints.

I can see no reason why changing the bytecode can't be done via `function.__code__`.
Suppose the code-object with the breakpoint add is `bcode`, then to turn on the breakpoint:
original_code = f.__code__
f.__code__ = bcode

and to turn it off
f.__code__ = original_code


The JVM supports bytecode instrumentation (via class loaders). It works well, as it provides a clear way for third party tools to modify the behaviour of a particular piece of code without violating any of the invariants of the interpreter. 
We don't really advertise setting `function.__code__` as a way to add low-impact breakpoints or profiling, but it is there.

If this use case is important, and it sounds like it is, then a better option would be to offer library support for adding and removing breakpoints/instrumentation.
This would have the advantage of being composable in a way that changing `PyInterpreterState.eval_frame` is not; in other words, it would be possible for one tool to add profiling and another to add breakpoints and have both work correctly.

I can write up a PEP if necessary.
msg356256 - (view) Author: Fabio Zadrozny (fabioz) * Date: 2019-11-08 19:25
@Mark

I can think of many use-cases which may break if the function code is changed (users can change the code in real-use cases and when they do that they'd loose debugging).

So, as long as the function code is part of the public API of Python, the debugger can't really change it for breakpoints (which is a bit different from the frame code, which the user can't really swap and it's not so common to change).
msg356359 - (view) Author: Mark Shannon (Mark.Shannon) * (Python committer) Date: 2019-11-11 12:04
Fabio,

If the user changes the `__code__` attribute of a function then, AFAICT, your debugger does the wrong thing, but bytecode modification does the right thing.

Suppose we have two functions `spam` and `eggs`.
Set a break point in `eggs`, set `spam.__code__ = eggs.__code__`, then call `spam`.
With bytecode modification, we get the correct result. That is, execution breaks in the source code of `eggs` when `spam` is run.
I think your debugger will do the wrong thing as it will execute the original code of `spam`. Could you confirm what it does?

But that's not the main issue, IMO. The big problem is that changing out the interpreter is not composable, unlike bytecode modification.

Suppose we have MyProfiler and YourDebugger.
MyProfiler wants to record calls and YourDebugger wants to support breakpoints.

With bytecode modification, and some care, we can do both.
Swapping out the interpreter is likely to cause all sorts of errors and confusion.
msg356483 - (view) Author: Brett Cannon (brett.cannon) * (Python committer) Date: 2019-11-12 19:38
I would like to bring this issue back on topic as this about how to expose PEP 523 support in Python 3.8, not whether Fabio should be using a different approach when he has been something sanctioned in an accepted PEP and was previously doable that was (accidentally) taken away from him.

If people want to do a separate PEP to roll back PEP 523 that's fine, but until that occurs I think we should move forward with the fact that PEP 523 is active.
msg356963 - (view) Author: Fabio Zadrozny (fabioz) * Date: 2019-11-19 12:03
@Mark @Brett

Well, PEP 523 still works (it's just more inconvenient to use now).

Still, if PEP 523 will still be supported I think that having the setter/getter makes sense.

If it is to be deprecated as @Mark is suggesting it doesn't really make sense to add it (but then, it should really be deprecated and ideally there'd be some replacement for the current debugger use... not sure about other use cases such as a jit, which was the initial target of PEP 523 -- @Mark, do you want to go that route/create a PEP to deprecate it so that this discussion takes place in a proper place?).

p.s.: as a note, bytecode modification on the actual object is not a usable approach for the debugger because users could break that in real-world use cases (i.e.: what happens if the user creates a **new** code and sets it to the code which had the breakpoint? What about breakpoint changes? Right now the debugger evaluates all assumptions just before the frame is executed, so, it's easier to get things right -- the case you posted currently does what's expected on pydevd). Still, a callback before the execution so that it could change the frame code before it's executed without the remainder of PEP 523 would be enough (and maybe it could be adopted in other Python implementations too) -- actually, for the debugger it'd be awesome if the frame code could be changed from inside a trace call and then that stack would restart execution (close to what happens with setting the frame line to be executed), but I guess this would be a completely different request ;)

p.s.: please don't reply to my previous p.s. here (let's move the discussion to another place -- either by @Mark creating a PEP for discussion or acknowledging the issue is ok given the current status quo).
msg356981 - (view) Author: Brett Cannon (brett.cannon) * (Python committer) Date: 2019-11-19 17:41
I think the real question is whether this is part of the CPython public API or the CPython internal API.

@Fabio how burdensome would it be if we placed this in the internal API that you can get access to but we don't make backwards-compatibility guarantees about? For instance, Victor wants to start passing in thread state which will change the API for 3.9, but we wouldn't expect to change it in a bugfix release.
msg356983 - (view) Author: Fabio Zadrozny (fabioz) * Date: 2019-11-19 18:16
@Bret

I don't really see a problem in breaking the API in major releases (so, having access for it in the internal API but without a backwards-compatibility guarantee seems like a good fit for me).
msg357085 - (view) Author: Brett Cannon (brett.cannon) * (Python committer) Date: 2019-11-20 17:10
OK, then my vote is to provide getter and setter methods that are underscore-prefixed to mark them as "internal" with clear comments specifying that they are part of PEP 523 and thus should not be needlessly broken in a bugfix release.
msg357173 - (view) Author: Mark Shannon (Mark.Shannon) * (Python committer) Date: 2019-11-21 14:55
Brett,
PEP 523 makes no mention of adding a getter or setter.
Adding them is a big change to Python semantics and shouldn't, IMO, be done without a PEP that explicit states they are going to be added.
msg357175 - (view) Author: Mark Shannon (Mark.Shannon) * (Python committer) Date: 2019-11-21 15:08
Fabio,

Can you give me a specific example where changing the bytecode via the `__code__` attribute of a function does not work as expected?

I am assuming that adding a breakpoint on a line is equivalent to adding `breakpoint();` at the beginning of that line. If the bytecode of a function is modified dynamically, say by a decorator, then it is unclear what should be done, and it is hard to claim that any particular approach is more correct.
However, assuming that the change is well defined and keeps the mapping to the original code, then adding a breakpoint to the modified code should work just as well before or after modification.

As an example, consider http://code.activestate.com/recipes/277940-decorator-for-bindingconstants-at-compile-time/ which provides a means of converting global variable reads to constants.  In that case the order in which the decorator and breakpoint insertion are applied shouldn't matter.

I propose a new method on code objects `withCallAtLine(callable: Callable[], line:int)` which returns a new code object with calls to the given callable at the given line.
A breakpoint can then be inserted at line L in function f with `f.__code__ = f.__code__.withCallAtLine(sys.breakpoint, L)`.
msg357183 - (view) Author: Fabio Zadrozny (fabioz) * Date: 2019-11-21 17:01
@Mark

First you have to explain to me how you envision changing the method code reliably in the debugger... 

Import hooks don't work (they'd break with something as simple as the code below)

def method():
   a = 10

mod = reload(old_mod)
old_mod.method.__code__ = mod.method.__code__

using the tracing also doesn't work (it's too late to change the code)

Note: consider the reload just an example, not the only use case (say, the user could pickle code objects to remote execution and the debugger should still work).

Also, consider you have to change the bytecode of methods which are only internal to a function (and thus can't be accessed from the outside).

Then, if you have a reliable way to do it, how do you keep track of those code objects to reapply the patching when breakpoints change? What if it adds a breakpoint to a new method, how do you locate it? Creating strong references to methods isn't an option because it would prevent things from being garbage collected (and you'd have to track all objects containing code objects for it to be reliable).

As a note, pydevd does have some support for hot-reloading, but there are too many corner-cases and it doesn't work 100% for live coding (it's an unsolved problem is Python) -- and I can't really envision it working for regular breakpoints as there are too many corner cases to handle.
msg357208 - (view) Author: Brett Cannon (brett.cannon) * (Python committer) Date: 2019-11-21 19:57
@Mark

> PEP 523 makes no mention of adding a getter or setter.
> Adding them is a big change to Python semantics and shouldn't, IMO, be done without a PEP that explicit states they are going to be added.

Adding getters or setters for something that was previously doable that we accidentally took away from users is not an expansion of semantics; it's fixing a backwards-compatibility break in a way that lets us keep the goal of making PyInterpreterState opaque.

I'm considering this issue at a stalemate and so I'm going to loop in python-dev to help settle this. If that still locks up then we can bring this to the steering council.
msg357212 - (view) Author: Brett Cannon (brett.cannon) * (Python committer) Date: 2019-11-21 20:07
Posted https://mail.python.org/archives/list/python-dev@python.org/thread/4UZJYAZL3NHRAGN5WAMJC4IHAHEXF3QF/ to see if anyone else wants to weigh in.
msg357245 - (view) Author: Gregory P. Smith (gregory.p.smith) * (Python committer) Date: 2019-11-22 06:29
+1 to re-exposing a way to do PEP-523.

PEP-523 added a public API, we unintentionally hid it behind the mask of Py_BUILD_CORE_MODULE in 3.8.  We shouldn't remove PEP-523's abilities without a deprecation cycle.  But given the use cases tend to be "deep" knowledge places that can adapt to API change and version checks, I think re-exposing it without the need for Py_BUILD_CORE_MODULE is reasonable.

Are you proposing this for 3.8.1?

Or is 3.8 burned for this 3.6 & 3.7 feature :sadface: (but we know how to work around it - so if our RM says it is, we'll deal) and re-exposing in some manner it only happens in 3.9?
msg357277 - (view) Author: STINNER Victor (vstinner) * (Python committer) Date: 2019-11-22 13:12
I wrote PR 17340 to add Add PyInterpreterState_GetEvalFrameFunc() and PyInterpreterState_SetEvalFrameFunc() functions.

Since the PEP 523 has been approved, for me, these functions must be public, not private. My implementation adds these functions to Include/cpython/pystate.h: "non-portable" API specific to CPython (exclude from the limited C API).

PyInterpreterState_GetEvalFrameFunc() is needed if you want to inject a "hook" and not really replaced the whole function. For example, execute code before or after the call. I guess that PyCharm debugger works like that.
msg357281 - (view) Author: STINNER Victor (vstinner) * (Python committer) Date: 2019-11-22 13:39
New question: if we add a public API to Python 3.8.1, would it make sense to directly change eval_frame type to add a new "tstate" parameter? See bpo-38818 for the rationale.

I already proposed a change for the master branch: https://github.com/python/cpython/pull/17187

Maybe I should combine PR 17340 (add PyInterpreterState_SetEvalFrameFunc()) with PR 17187 (add tstate parameter).
msg357308 - (view) Author: Brett Cannon (brett.cannon) * (Python committer) Date: 2019-11-22 17:58
@greg I hope this goes into 3.8.1 since it was a breaking change.

@victor if we are going to ask folks to start using a setter and getter I say we might as well get it right the first time and start taking the tstate now.
msg357310 - (view) Author: STINNER Victor (vstinner) * (Python committer) Date: 2019-11-22 18:07
> @victor if we are going to ask folks to start using a setter and getter I say we might as well get it right the first time and start taking the tstate now.

Done: New PR 17352 combines my two other PRs.
msg357461 - (view) Author: STINNER Victor (vstinner) * (Python committer) Date: 2019-11-25 21:53
Fabio: "I don't really see a problem in breaking the API in major releases (so, having access for it in the internal API but without a backwards-compatibility guarantee seems like a good fit for me)."

In Python, the internal C API now means that you have to define Py_BUILD_CORE_MODULE macro and include a special "internal/pycore_pystate.h" header file.

If you are fine with doing that in PyCharm, what is the purpose of this issue?

Do you mean in *internal* API or a *private* API? Private API is basically a public API with weaker backward compatibility warranties and with a "_" prefix (_Py, _PY prefix).
msg363833 - (view) Author: STINNER Victor (vstinner) * (Python committer) Date: 2020-03-10 17:33
I marked bpo-38818 "Modify PyInterpreterState.eval_frame to pass tstate (PyThreadState)" as a duplicate of this issue.

It has been said earlier that if we choose to add a public C API to set the frame evaluation function, it's better to design it properly from the start: so include tstate.

See https://vstinner.github.io/cpython-pass-tstate.html for the rationale behind passing tstate everywhere. See also multiple discussions on python-dev about passing explicitly tstate.
msg363834 - (view) Author: STINNER Victor (vstinner) * (Python committer) Date: 2020-03-10 17:45
I plan to merge my PR 17340 at the end of week to not miss Python 3.9 feature freeze deadline, unless someone speaks up and find a good reason to not merge this PR. The PR adds a public C API PyInterpreterState_SetEvalFrameFunc() and now pass tstate to the frame evaluation function.

--

Mark Shannon is against the idea of providing a way to set the frame evaluation function (PEP 523), but Dino Viehland, Eric Snow, Brett Cannon and me want to provide a C API for that.

--

I propose to properly fix this issue in Python 3.9:

* Add a public C API to get and set the frame evaluation function
* Pass tstate to this frame evaluation function

Pass tstate is a backward incompatible change. But it's unclear to me if its API was part of the "public" C API in Python 3.7. In Python 3.8, PyInterpreterState structure moved to the internal C API which is clearly excluded from backward compatibility warranties of the public C API.

Anyway, I expect that they are less than 10 projects in the world which use the frame evaluation function, which it should be doable to update all of them to support the C API for Python 3.9 that I'm proposing.

--

> This is no longer possible because in 3.8 the PyInterpreterState is opaque, so, Py_BUILD_CORE_MODULE needs to be defined defined and "internal/pycore_pystate.h" must be included to set PyInterpreterState.eval_frame.

The status quo is that Python 3.8.0, 3.8.1 and 3.8.2 have been released with that. If someone wants to support "Python 3.8", using Py_BUILD_CORE_MODULE to access the internal C API is the way to go. If we wanted to add a public C API in Python 3.8, IMHO we had to do it *before Python 3.8.0 release. Now it's way too late.
msg363835 - (view) Author: STINNER Victor (vstinner) * (Python committer) Date: 2020-03-10 18:06
> Posted https://mail.python.org/archives/list/python-dev@python.org/thread/4UZJYAZL3NHRAGN5WAMJC4IHAHEXF3QF/ to see if anyone else wants to weigh in.

Mark Shannon listed flaws in the PEP 532 and suggest to withdraw this PEP. Honestly, I'm open to any solution. But this issue is a concrete regression of Python 3.8 with a concrete use case, whereas Mark only lists theoretical enhancements.

I would like to fix the regression first. There are users of the PEP 532, we cannot simply ignore them, withdraw the PEP and remove the feature immediately without warning users.

If someone wants to withdraw the PEP, it has to go through the regular deprecation process with a slow transition. Last time we pushed too many incompatible changes (Python 2 to Python 3 transition), it was a mess. Moreover, I would prefer to have at least one Python release (if not two or more) which provides two options, deprecated way and a new better way, to have a smooth transition.
msg363837 - (view) Author: Brett Cannon (brett.cannon) * (Python committer) Date: 2020-03-10 18:18
> Mark Shannon listed flaws in the PEP 532 and suggest to withdraw this PEP.

I think you mean PEP 352? And a more formal proposal to withdraw would need to be made and that has not happened, so assume the PEP is still accepted and final.
msg363838 - (view) Author: STINNER Victor (vstinner) * (Python committer) Date: 2020-03-10 18:21
> I think you mean PEP 352? And a more formal proposal to withdraw would need to be made and that has not happened, so assume the PEP is still accepted and final.

I mean PEP 523 "Adding a frame evaluation API to CPython" as I wrote in the title, sorry for the typo :-)
msg364009 - (view) Author: Mark Shannon (Mark.Shannon) * (Python committer) Date: 2020-03-12 10:48
I'm not suggesting immediate removal of PEP 532.

I am requesting that you don't add a new function to the C API that will prevent implementation of many meaningful optimizations to CPython.
PEP 532 does not add any API functions.

I understand that you want to fix the regression.
Why not do so by reverting the change that caused it?

Adding a new API function is not fixing a regression.
msg364017 - (view) Author: STINNER Victor (vstinner) * (Python committer) Date: 2020-03-12 12:50
> Why not do so by reverting the change that caused it?

Making PyInterpreterState structure private was motivated by the subinterpreters work but also by the work on cleaning the C API.

Over time, PyInterpreterState became more and more complex because many other structures have been moved there. For example, the GIL, GC state, warnings state, parser state, etc.

I would really avoid exposing the GIL state in the C API since it uses the "pycore_atomic.h" header which caused a *lot* of compiler errors in the past. Most errors were on including the header file, even if the C extension didn't use any atomic variable.

I'm really happy that we managed to move atomic variables into the internal C API: we got less error coming from that. I'm strongly opposed to move PyInterpreterState structure back into the Include/cpython/ C API. That would be a big mistake for various reasons.

Even in CPython, pycore_pystate.h is causing a lot of troubles since PyInterpreterState became very complex. Example in posixmodule.c:
---
#include "Python.h"
#ifdef MS_WINDOWS
   /* include <windows.h> early to avoid conflict with pycore_condvar.h:

        #define WIN32_LEAN_AND_MEAN
        #include <windows.h>

      FSCTL_GET_REPARSE_POINT is not exported with WIN32_LEAN_AND_MEAN. */
#  include <windows.h>
#endif

#include "pycore_ceval.h"     /* _PyEval_ReInitThreads() */
#include "pycore_import.h"    /* _PyImport_ReInitLock() */
#include "pycore_pystate.h"   /* _PyRuntime */
---

pycore_condvar.h is used by the GIL state.
msg364030 - (view) Author: STINNER Victor (vstinner) * (Python committer) Date: 2020-03-12 17:05
For me, the *short term* goal is to find a way to limit the number of broken C extension module while we modify the C API to make it more opaque.

We cannot reach all goals at once (opaque C API, subinterpreter, more optimizations, etc). We have to move step by step.

For me it's ok to deprecate or even remove PyInterpreterState_SetEvalFrameFunc() later, once we will have a good reason for that.

I'm also ok with having an alternative Python implementation which doesn't support PyInterpreterState_SetEvalFrameFunc(). Users would be able to make a choice: faster Python without PyInterpreterState_SetEvalFrameFunc(), or regular "slow" Python with PyInterpreterState_SetEvalFrameFunc(). That's part of my larger https://pythoncapi.readthedocs.io/ goal: the ability to use different Python "runtimes".

From what I understood, to be able to provide multiple Python runtimes, we have first to "fix" the C API. The HPy project is another approach to this problem. Making the C API opaque makes CPython closer to this goal.
msg364032 - (view) Author: STINNER Victor (vstinner) * (Python committer) Date: 2020-03-12 17:41
Somehow related, I just created bpo-39946: "Is it time to remove _PyThreadState_GetFrame() hook?".
msg364035 - (view) Author: STINNER Victor (vstinner) * (Python committer) Date: 2020-03-12 17:48
Mark Shannon: `PyThreadState` is an internal opaque data structure, which means we are free to change it.

I guess that you mean PyInterpreterState which was moved to the internal C API in Python 3.8. It was part of public C API in Python 3.7. See commit be3b295838547bba267eb08434b418ef0df87ee0 of bpo-35886.

By the way, I just created bpo-39947: "Make the PyThreadState structure opaque (move it to the internal C API)".
msg364036 - (view) Author: STINNER Victor (vstinner) * (Python committer) Date: 2020-03-12 17:52
Mark:
> PEP 523 is quite vague, but the rationale indicates that exposing `eval_frame` is for "a method-level JIT". PEP 523 did not suggest adding an API.

I disagree, the PEP is quite explicit: "Third-party code may then set their own frame evaluation function instead to control the execution of Python code." That's the whole point of the PEP: let third-party code set eval_frame to use the feature.

The PEP was written in 2016, when the PyInterpreterState structure was part of the public C API. But PyInterpreterState was moved to the internal C API, after the PEP was approved.
msg364046 - (view) Author: STINNER Victor (vstinner) * (Python committer) Date: 2020-03-12 20:42
I took Mark's concerns in account and I updated my PR to only add *private* functions to the C API, not *public* functions.
msg364047 - (view) Author: STINNER Victor (vstinner) * (Python committer) Date: 2020-03-12 20:51
Mark: Would you mind to open a separated issue for your following idea?

> I propose a new method on code objects `withCallAtLine(callable: Callable[], line:int)` which returns a new code object with calls to the given callable at the given line.
> A breakpoint can then be inserted at line L in function f with `f.__code__ = f.__code__.withCallAtLine(sys.breakpoint, L)`.

I prefer to restrict this issue to the PEP 523.
msg364055 - (view) Author: STINNER Victor (vstinner) * (Python committer) Date: 2020-03-12 22:18
New changeset 0b72b23fb0c130279f65f3bcd23521acf4a98c88 by Victor Stinner in branch 'master':
bpo-38500: Add _PyInterpreterState_SetEvalFrameFunc() (GH-17340)
https://github.com/python/cpython/commit/0b72b23fb0c130279f65f3bcd23521acf4a98c88
History
Date User Action Args
2020-03-19 03:26:02vstinnersetstatus: open -> closed
resolution: fixed
stage: patch review -> resolved
2020-03-12 22:18:42vstinnersetmessages: + msg364055
2020-03-12 20:51:03vstinnersetmessages: + msg364047
2020-03-12 20:42:18vstinnersetmessages: + msg364046
2020-03-12 20:41:25vstinnersettitle: PEP 523: Add PyInterpreterState_SetEvalFrameFunc() to the public C API (Python 3.8 regression) -> PEP 523: Add private _PyInterpreterState_SetEvalFrameFunc() function to the C API (Python 3.8 regression)
2020-03-12 17:52:25vstinnersetmessages: + msg364036
2020-03-12 17:48:59vstinnersetmessages: + msg364035
2020-03-12 17:41:23vstinnersetmessages: + msg364032
2020-03-12 17:05:45vstinnersetmessages: + msg364030
2020-03-12 12:50:44vstinnersetmessages: + msg364017
2020-03-12 10:48:52Mark.Shannonsetmessages: + msg364009
2020-03-10 18:21:26vstinnersetmessages: + msg363838
2020-03-10 18:18:32brett.cannonsetmessages: + msg363837
2020-03-10 18:07:43vstinnersettitle: Provide a way to get/set PyInterpreterState.frame_eval without needing to access interpreter internals -> PEP 523: Add PyInterpreterState_SetEvalFrameFunc() to the public C API (Python 3.8 regression)
versions: - Python 3.8
2020-03-10 18:06:56vstinnersetmessages: + msg363835
2020-03-10 17:45:43vstinnersetmessages: + msg363834
2020-03-10 17:33:49vstinnersetmessages: + msg363833
2020-03-10 17:32:12vstinnerlinkissue38818 superseder
2019-11-28 23:53:10phsilvasetnosy: + phsilva
2019-11-25 21:53:56vstinnersetmessages: + msg357461
2019-11-23 02:20:43ned.deilysetkeywords: + 3.8regression
priority: normal -> critical

nosy: + lukasz.langa
2019-11-22 18:07:36vstinnersetmessages: + msg357310
2019-11-22 18:06:23vstinnersetpull_requests: + pull_request16836
2019-11-22 17:58:40brett.cannonsetmessages: + msg357308
2019-11-22 13:39:33vstinnersetmessages: + msg357281
2019-11-22 13:12:21vstinnersetmessages: + msg357277
2019-11-22 13:05:40vstinnersetpull_requests: + pull_request16824
2019-11-22 06:29:28gregory.p.smithsetmessages: + msg357245
versions: + Python 3.9
2019-11-22 05:53:23gregory.p.smithsetnosy: + gregory.p.smith
2019-11-21 20:07:00brett.cannonsetmessages: + msg357212
2019-11-21 19:57:18brett.cannonsetmessages: + msg357208
2019-11-21 17:01:22fabiozsetmessages: + msg357183
2019-11-21 15:08:06Mark.Shannonsetmessages: + msg357175
2019-11-21 14:56:00Mark.Shannonsetmessages: + msg357173
2019-11-20 17:10:56brett.cannonsetmessages: + msg357085
2019-11-19 18:16:40fabiozsetmessages: + msg356983
2019-11-19 17:41:04brett.cannonsetmessages: + msg356981
2019-11-19 12:03:41fabiozsetmessages: + msg356963
2019-11-18 19:06:52vstinnersetkeywords: + patch
stage: patch review
pull_requests: + pull_request16739
2019-11-12 19:38:47brett.cannonsetmessages: + msg356483
2019-11-11 12:04:53Mark.Shannonsetmessages: + msg356359
2019-11-08 19:25:53fabiozsetmessages: + msg356256
2019-11-08 19:10:01Mark.Shannonsetmessages: + msg356255
2019-11-07 23:44:26vstinnersetmessages: + msg356216
2019-11-07 23:13:29brett.cannonsetmessages: + msg356215
2019-11-07 16:37:22vstinnersetmessages: + msg356200
2019-11-07 16:14:46fabiozsetmessages: + msg356197
2019-11-07 16:02:10Mark.Shannonsetmessages: + msg356196
2019-11-05 01:14:22vstinnersetmessages: + msg355990
2019-11-04 18:43:59brett.cannonsetmessages: + msg355971
2019-11-04 11:07:34fabiozsetmessages: + msg355936
2019-11-04 10:03:49Mark.Shannonsetmessages: + msg355934
2019-11-04 10:03:10Mark.Shannonsetnosy: + Mark.Shannon
messages: + msg355933
2019-11-01 22:17:57dino.viehlandsetmessages: + msg355849
2019-11-01 21:24:45eric.snowsetnosy: + brett.cannon, dino.viehland
messages: + msg355845
2019-10-21 14:04:04vstinnersetnosy: + eric.snow
2019-10-16 16:17:06fabiozcreate