Do I understand correctly that the CodeLike type is expected to be provided by user code?

I think we should provide CodeLike in some form, but for now I put it in the text file so we can talk about how and where we want it to be. The offset to location mapping should also be part of this.

I don't see why this public API should live in an internal header file. And why the underscore names? We only use(d) them for non-public functions.

@markshannon did you intend for these to be private?

@markshannon did you intend for these to be private?

My thinking was that it is better to keep any function private unless we need it to be public. It sounds like they need to be public.

How about this?

• The declaration of the implementation function,_PyMonitoring_FirePyStartEvent(), goes in the semi-privatecpython/ folder.
• We expose an inline function calling for the unstable API and a non-inline one for the stable API in the public header.

Something like this:

#ifndefPy_LIMITED_APIstaticinlineintPyMonitoring_FirePyStartEvent(PyMonitoringState*state,PyObject*codelike,uint32_toffset){if (state->active)return_PyMonitoring_FirePyStartEvent(state,codelike,offset);elsereturn0;}#elseexternintPyMonitoring_FirePyStartEvent(PyMonitoringState*state,PyObject*codelike,uint32_toffset);#endif

The "code like" object is entirely opaque to the monitoring machinery. As far as it is concerned, a location is a pair of an object and an integer index.
Debuggers, profilers, etc need to present that object, int as a meaningful location. If the object is a code object, they can already do that usingco.co_filename andco.co_positions().
The point of a "code like" object is to provide the necessary interface for tools (debuggers, profilers, etc) to turnobject, int into a meaningful source location.

So, no C struct, but a Python abstract base class.

So, the only events that should happen with the current exception set are RAISE, RERAISE, STOP_ITERATION, C_RAISE and UNWIND.

That leavesEXCEPTION_HANDLED andPY_THROW.

Looking through the code for the interpeter implementation of firing events about exceptions, it looks like the exception is usually passed explicitly.

And considering that the concept of there being one (and only one) "current exception" doesn't always make sense, I think all API functions for events where the callback receives the exception should have an exception parameter.

Looking through the code for the interpeter implementation of firing events about exceptions, it looks like the exception is usually passed explicitly.

What do you mean? I see all those functions calldo_monitor_exc without an exception, and it callsPyErr_GetRaisedException to get the exception.

What do you mean?

Good question. I mean that the exception is passed explicitly to any callback function.

FromPEP 669:

func(code: CodeType, instruction_offset: int,exception: BaseException) -> DISABLE | Any

do_monitor_exc may not take an exception as an argument, but it calls_Py_call_instrumentation_arg() which does.

Passing the exception as an argument might require the user of the API to do a bit more work, but it removes a lot of uncertainty about the "current exception".

For example, consider a C extension that is about to raise an exception. To raise an exception, it must have a reference to that exception. Since it has a reference to the exception is straightforward to pass it as an argument:

PyObject*exception;/* Strong reference to the exception to be raised */if (PyMonitoring_FireRaiseEvent(state,codelike,offset,exception)<0) {/* Would could chain the exceptions, but I'm lazy, so I'm just going to discard it */Py_DECREF(exception);returnNULL;}PyErr_SetRaisedException(exception);returnNULL;

The c api also passes the exception to the callbacks. It's the Fire functions that we removed it from.

We need to merge this pretty much now, so last call for strong objections. Otherwise we can iterate via bugfixes.

No strong objections.

the only events that should happen with the current exception set areRAISE,RERAISE,STOP_ITERATION,C_RAISE andUNWIND.

DoesSTOP_ITERATION really belong into this group? Isn't the idea rather that generators donot need to create aStopIteration in order to save time on termination?

Does CPython really create aStopIteration instance here just for monitoring it?

EDIT: it seems that it does:

That said, this is a detail that can still be decided during the beta phase, so feel free to merge this PR as it is if there's remaining need for discussion.

Does CPython really create a StopIteration instance here just for monitoring it?

Yes, but only if you ask for it (it has zero cost if you don't ask for it, thanks to the adaptive interpreter)

https://github.com/python/cpython/blob/main/Python/bytecodes.c#L291

It seems that you already found that 🙂

Does CPython really create aStopIteration instance here just for monitoring it?

Yes, but only if you ask for it (it has zero cost if you don't ask for it, thanks to the adaptive interpreter)

Ok … then I think it would be best if CPython's monitoring infrastructure created aStopIteration instance automatically when it detects that it needs to notify about it without having one. It's in the best position to detect whether it really needs an exception instance or not. Event sources shouldn't be forced to a) figure that out themselves or b) create an instance just for it to be thrown away.

Basically, move the existing code from the byte code handlers all the way to the other side into the notification mechanism.

Basically, move the existing code from the byte code handlers all the way to the other side into the notification mechanism.

That might suggest that the signature for theStopIteration event should not receive an exception or use the current one, but should better receive theStopIteration value directly (with or without a live exception).

I'll merge, please create an issue to iron out the StopIteration business.

I'll merge, please create an issue to iron out the StopIteration business.

Or discuss it on#111997.

In order to avoid any confusion about blocker tickets during the beta release, I created a new ticket to discuss theStopIteration behaviour/functions here:#118692