GH-93503: Add thread-specific APIs to set profiling and tracing functions in the C-API

[pablogsal]

closes: #93503

• Issue: Add public APIs to set trace and profile function in other threads. #93503

[markshannon]

I think this is too error prone, for a couple of reasons.

• The main reason is that this sets the expectation that C-API functions taking PyThreadState can take any thread state.
  However, most C-API functions that take PyThreadState are likely to leave the VM in an invalid state if passed anything other than the current thread state.
• What happens when the thread state belongs to a different sub-interpreter from obj?

To be fair, this isn't just an issue with these functions, but any function that takes a PyThreadState parameter.

#93503 is about adding an API to set tracing/profiling on all threads.
Why not add an API that does that?
void PyEval_SetThreadProfileAllThreads(Py_tracefunc func, PyObject *obj)

Also, I don't like offering operations in the C-API that aren't available in Python. We shouldn't be forcing people to write C.

[pablogsal]

Why not add an API that does that?

I am fine with that. Do you want me to update this PR or create another one?

Also, I don't like offering operations in the C-API that aren't available in Python. We shouldn't be forcing people to write C.

That ship already sailed. Do mean that you would like to also expose such an API in the Python layer?

[vstinner]

The threading module has setprofile() and settrace() functions.

[pablogsal]

The threading module has setprofile() and settrace() functions.

But those only work for newly created threads as I noted in the issue. They are used only when new threads are bootstrapped. What we are trying to do is to add APIs that affect all current threads.

We also cannot change those for backwards compatibility, but we could place the new APIs in the thread module for Python access.

[pablogsal]

@markshannon you ok with this plan?

• Changing the proposed C APIs to set the trace/profile functions in all threads.
• Add a python API in the thread module that calls the C API.

[markshannon]

@markshannon you ok with this plan?

* Changing the proposed C APIs to set the trace/profile functions in all threads.

* Add a python API in the thread module that calls the C API.

Yes. Sounds good to me.

[vstinner]

But those only work for newly created threads as I noted in the issue. They are used only when new threads are bootstrapped. What we are trying to do is to add APIs that affect all current threads.

Would it make sense to add a parameter to these threading functions or to sys functions to apply the new tracing/profiling functions to existing threads?

[pablogsal]

I am ambivalent, but the new parameter sounds good if you prefer it that way.

[pablogsal]

Ok, I have made a first version of the idea proposed by @markshannon. Please @vstinner @markshannon review again.

[vstinner]

I suggest all_threads=True rather than running_threads=True: in your implementation, threading.settrace() changes old and new threads, all threads, not only old ("running") threads.

With the PR, we have:

• sys.settrace(): current thread
• threading.settrace(): new threads
• threading.settrace(all_threads=True): all threads (old, new and current)

[pablogsal]

@vstinner I'm going to change this PR to follow @markshannon request and make it a separate function in Python instead of a parameter.

[vstinner]

@vstinner I'm going to change this PR to follow @markshannon request and make it a separate function in Python instead of a parameter.

Sure, as you want. What would be the API in that case?

[pablogsal]

I have updated the APIs to use separate functions.

[pablogsal]

What would be the API in that case?

threading.settrace_all_threads and threading.setprofile_all_threads

[vstinner]

It's not great that the function cannot report errors. While not returning -1 on error and pass the exception to the caller, rather than handling it with _PyErr_WriteUnraisableMsg()?

[pablogsal]

I am mimicking what we do already in PyEval_SetProfile and to avoid surprises I think we should keep these two as synchronized as possible.

[vstinner]

We cannot fix the API of old functions, but we can avoid past mistakes in newly added functions.

[vstinner]

If you want to ignore exceptions on purpose, please mention it in the function documentation.

[pablogsal]

If you want to ignore exceptions on purpose, please mention it in the function documentation.

Yeah, I want to do that because I don't want to stop setting the profile function on other threads if an exception in one fails. Unless you strongly disagree, I will document this.

[vstinner]

Honestly, I don't know if HEAD_LOCK/HEAD_UNLOCK is needed, I'm not sure if the GIL protects us here. The PyThreadState_Next() API is not well documented for thread safety :-(

[pablogsal]

I plan to land this this week or the next one to avoid merge conflicts. @markshannon can you give it a last pass?

[markshannon]

One minor issue with the docs, otherwise LGTM.

[vstinner]

Nice enhancement, thanks.

[vstinner]

You should document the newly added C API functions to: https://docs.python.org/dev/whatsnew/3.12.html#c-api-changes

You should also document the new threading functions in the same document, in a threading section, near: https://docs.python.org/dev/whatsnew/3.12.html#improved-modules

[pablogsal]

You should document the newly added C API functions to: docs.python.org/dev/whatsnew/3.12.html#c-api-changes

You should also document the new threading functions in the same document, in a threading section, near: docs.python.org/dev/whatsnew/3.12.html#improved-modules

#96681