gh-148925: Add safe PyUnstable_* APIs for call-stack iteration#148930
gh-148925: Add safe PyUnstable_* APIs for call-stack iteration#148930danielsn wants to merge 4 commits into
Conversation
danielsn
requested review from
a team,
AA-Turner,
ZeroIntensity,
emmatyping and
erlend-aasland
as code owners
|
All commit authors signed the Contributor License Agreement. |
|
Will try to review this week. Heads up that we are very close to beta freeze so we would need to land this this week or next week at the latest |
There was a problem hiding this comment.
I'm not sure the cost/benefit pencils out here.
This is a meaningful amount of new code to carry: a new public-ish struct with fixed buffers, a new format_ascii encoder that largely duplicates _Py_DumpASCII, a new collect/print pair, plus a refactor of dump_traceback. The safety story is subtle enough that it requires a maintainer-contract banner just to keep future contributors from breaking it (no malloc, no refcount, no GIL, no non-AS-safe libc, etc.), and even with that discipline in place the contract is admittedly best-effort as CollectCallStack itself documents that it "may even rash itself."
A third-party extension can already do all of this today: include the CPython headers, build against Py_BUILD_CORE-style internals if needed, and walk frames using the existing PyUnstable_InterpreterFrame_* primitives. That's always possible. But landing it here flips the direction of the maintenance burden instead of profiler vendors maintaining their own stack-walker against each CPython version (which they already do for everything else they care about), we take on maintaining it for them, with the additional constraint that the AS-safety invariants must be preserved by every future contributor touching traceback.c. That's a real cost, not a free PyUnstable_* escape hatch.
It's also worth noting that this only helps a fairly narrow class of tools: in-process profilers and crash handlers running inside the target interpreter. The majority of production Python profilers are out-of-process. Those tools get nothing from this API; they need stable struct layouts and offsets they can read remotely, not an in-process C function. So we'd be adding a non-trivial maintenance surface to CPython to serve a subset of in-process tooling, while the dominant profiler architecture is unaffected.
That said, I want to think about this more before taking a firm position, and I'd like to hear other core devs weigh in.
|
A Python core developer has requested some changes be made to your pull request before we can consider merging it. If you could please address their requests along with any other requests in other reviews from core developers that would be appreciated. Once you have made the requested changes, please leave a comment on this pull request containing the phrase |
|
It was proposed to make PyUnstable_DumpTraceback() public: see PR gh-148145. IMO it would already be a first step forward. I prefer to wait until that's done to consider adding another similar-but-different API. |
|
Thanks for the review @pablogsal and @vstinner. I talked with @pablogsal offline, and I think we can indeed push most of the work onto the caller. The one remaining piece is a signal-safe way to walk the stack frames themselves, which I think can be done with a fairly small API change. I'll test this out and get back to you with an updated diff :) |
…ration Add four new public APIs for walking the Python call stack without allocating memory, changing reference counts, or acquiring the GIL, making them safe to call from signal handlers and custom memory allocator hooks: - PyUnstable_ThreadState_GetInterpreterFrame(tstate): returns the innermost complete interpreter frame, skipping entry trampolines and pre-RESUME frames automatically. - PyUnstable_InterpreterFrame_GetNextComplete(frame): returns the next complete calling frame, skipping incomplete frames. Mirrors PyFrame_GetBack semantics without allocating. - PyUnstable_InterpreterFrame_GetCodeSafe(frame): returns a borrowed reference to the code object, or NULL if freed memory is detected. - PyUnstable_InterpreterFrame_GetLineSafe(frame): returns the current line number, validating the instruction offset rather than asserting, safe when the frame may be partially torn down. All four use _Py_NO_SANITIZE_THREAD to suppress intentional racy reads and heuristics (_PyMem_IsPtrFreed) to detect freed memory. Switch tracemalloc and the fatal error traceback printer to use the new APIs instead of internal helpers. Add tests in Lib/test/test_capi/test_misc.py and test helpers in Modules/_testinternalcapi.c.
| PyUnstable_ThreadState_GetInterpreterFrame(PyThreadState *tstate) | ||
| { | ||
| _PyInterpreterFrame *frame = tstate->current_frame; | ||
| while (frame != NULL) { |
There was a problem hiding this comment.
We could add a "hop counter" to ensure that this function would deterministically exit
Documentation build overview
8 files changed ·
|
danielsn
changed the title
|
@pablogsal I have reworked the proposal to add the missing iterator function, with the rest of the work able to happen on the client side. LMK if this looks any better :) |
danielsn
changed the title
4 participants
#145559 proposes a signal-safe mechanism to print backtraces. This is useful for human debugging, but not ideal for tools because it requires parsing human-readable text. For example, a crash-tracker may wish to export and analyze structured backtraces. Similarly, a profiler can benefit from the ability to generate structure stack-traces in pure C, without needing to reason about the safety of reentering the python interpreter.
📚 Documentation preview 📚: https://cpython-previews--148930.org.readthedocs.build/