bpo-43760: Document PyThreadState.use_tracing removal #28527
Conversation
Doc/whatsnew/3.10.rst
Outdated
| @@ -2194,6 +2194,11 @@ Porting to Python 3.10 | |||
| limited API. The function is mainly useful for custom builds of Python. | |||
| (Contributed by Petr Viktorin in :issue:`26241`) | |||
|
|
|||
| * The ``PyThreadState.use_tracing`` member has been removed to optimize Python: | |||
| ``tstate->use_tracing`` must be replaced with | |||
| ``tstate->cframe->use_tracing``. | |||
There was a problem hiding this comment.
This may be dangerous, state->cframe is a stack-allocated structure so I am not sure if is valid in all accesses of tstate->cframe and has the same semantics. Maybe @markshannon can clarify
There was a problem hiding this comment.
Cython and greenlet now uses tstate->cframe->use_tracing. Do you mean that it's a bug?
There was a problem hiding this comment.
It may be. That is only valid as long as the executing frame is alive. For instance a for generators that may segfault
Actually, I am wrong, if tstate->cframe->use_tracing is not NULL then it must be valid.
The other question here is that tstate->cframe->use_tracing is still private, so I don't like recommending using it because it can change yet again.
There was a problem hiding this comment.
This may be dangerous for two reasons:
tstate->use_tracinghad global effect before, whiletstate->cframe->use_tracingonly affects the last executing frame. Modifiying this value only affects that particular frame. The semantics are different.tstate->cframepoints to a stack allocated variable in ceval. Accessing this from heap objects may be dangerous.
There was a problem hiding this comment.
Hum. I will design an API to abstract access to use_tracing. For now, I update my PR for the bare minimum: only document that the member is removed.
There was a problem hiding this comment.
Reading from, or writing to, tstate->cframe->use_tracing is OK, what is dangerous is hanging on to a pointer to tstate->cframe. use_tracing must always be accessed through tstate (and with the GIL held).
However, it may change in the future, we should make it clear that this advice only applies to 3.10.
There was a problem hiding this comment.
Reading from, or writing to,
tstate->cframe->use_tracingis OK,
There is also change in semantics, no? before tstate->use_tracing had global effect but this only affects the current frame.
There was a problem hiding this comment.
I may be wrong, but AFAIR, it was used as a flag to disable tracing in the current frame. That would also motivate why it was moved out of the thread state now.
There was a problem hiding this comment.
Right, but it would have effect on the whole call stack. Imagine you set it to false, then all callers would stopped tracing when they get to run again after the callee returns. iIUC now that won't happen because you are only modifying the one in the frame running and all its callees
There was a problem hiding this comment.
Could setting the tracefunc to NULL be the alternative? There are some differences in 3.9 between setting use_trace to False and tracefunc to NULL, but I suspect the latter can be used. Keep in mind that this is implementation & version specific functionality so it's probably ok not to support all cases that 3.9 did.
|
Thanks @vstinner for the PR 🌮🎉.. I'm working now to backport this PR to: 3.10. |
|
GH-28529 is a backport of this pull request to the 3.10 branch. |
(cherry picked from commit f4ccb79) Co-authored-by: Victor Stinner <[email protected]>
…28529) (cherry picked from commit f4ccb79) Co-authored-by: Victor Stinner <[email protected]>
…28529) (cherry picked from commit f4ccb79) Co-authored-by: Victor Stinner <[email protected]>
https://bugs.python.org/issue43760