fix(profiler): indirectly access frame objects for tracebacks for Python 3.11 by Yun-Kim · Pull Request #4895 · DataDog/dd-trace-py

Yun-Kim · 2023-01-12T19:46:57Z

Description

#4511 added support for Python 3.11 for the profiler, but missed some PyFrameObject-related upgrades required to be compatible with Python 3.11, most notably in the stack collector's traceback module. Specifically, PyFrameObjects and its member values are now lazily created/computed, which means we can no longer directly access its member values.
According to https://docs.python.org/3/whatsnew/3.11.html#pyframeobject-3-11-hiding, we need to use get functions to access its member values.

Checklist

Followed the library release note guidelines when writing a release note.
Add additional sections for feat and fix pull requests.
Library documentation and/or Datadog's documentation site is updated. Link to doc PR in description.

Motivation

Design

Testing strategy

Relevant issue(s)

Testing strategy

Reviewer Checklist

Title is accurate.
Description motivates each change.
No unnecessary changes were introduced in this PR.
Avoid breaking API changes unless absolutely necessary.
Tests provided or description of manual testing performed is included in the code or PR.
Release note has been added and follows the library release note guidelines, or else changelog/no-changelog label added.
All relevant GitHub issues are correctly linked.
Backports are identified and tagged with Mergifyio.

Yun-Kim

@P403n1x87 At first glance it looks like it's only the _traceback.pyx module that works with PyFrameObjects, but to be fair this is the only obvious fix relating to Python 3.11. Would love your feedback/second look 🙇

Yun-Kim · 2023-01-17T23:31:04Z

+                lineno = PyFrame_GetLineNumber(<PyFrameObject*> frame)
+                lineno = 0 if lineno is None else lineno
+                frames.append(((<object>code).co_filename, lineno, (<object>code).co_name, _extract_class_name(frame)))
+            frame = <object>PyFrame_GetBack(<PyFrameObject*> frame)


PyFrame_GetBack() returns a strong reference. Should I be calling Py_XDECREF(frame) even though we are iterating through these references?

the frame isn't actually needed anymore after L114 right? since we are creating/returning a list of tuples?

does the frame getting GC'd cause co_filename/co_name/etc to also get GC'd? are we appropriately incref/decref-ing those references?

brettlangdon · 2023-01-19T20:49:16Z

+                frames.append(((<object>code).co_filename, lineno, (<object>code).co_name, _extract_class_name(<object>pyframe)))
+                Py_XDECREF(<PyObject*>code)
+            pyframe = PyFrame_GetBack(pyframe)
+            # FIXME: Where to Py_XDECREF(cframe)?


s/cframe/pyframe/ ?

Thanks for the catch!

I am looking for some suggestions on where to call Py_XDECREF(pyframe) though. Since PyFrame_GetBack(pyframe) returns a strong reference of the PyFrameObject*, should I be decrementing the refcount for every time we call PyFrame_GetBack(pyframe)? But since we're using the result of the PyFrame_GetBack() in the while loop, I don't think I can decrement the refcount without interfering with the rest of the function.

Py_XDECREF(pyframe) has to be called every time you are done with that particular pyframe reference, so within the loop, but before overwriting the variable's value. The cast to <object> is taking care of ref counting when passing values around in Cython, so as long as our ref counting for when we invoke the C API directly is correct we should be good.

brettlangdon · 2023-01-20T14:50:03Z

you might need to do something like:

cdef PyFrameObject* cur_frame = pyframe pyframe = PyFrame_GetBack(cur_frame) Py_XDECREF(cur_frame)

? and then a final Py_XDECREF(pyframe) after the while ?

I think this makes sense. Trying that now.

I would have thought that

Py_XDECREF(pyframe) pyframe = PyFrame_GetBack(pyframe)

is enough, since we are manipulating Python objects and hence we're holding the GIL. But @brettlangdon's suggestion works too.

Yun-Kim · 2023-01-23T20:09:39Z

Currently running into a segmentation fault after adding the Py_XDECREF(pyframe) in pyframe_to_frames(). It appears that this segfault appears after pyframe_to_frames() is run with asyncio task frames as the argument:
https://github.com/Yun-Kim/dd-trace-py/blob/0cf1580cfa638155bb7055a1767ca9083f53c24f/ddtrace/profiling/collector/stack.pyx#L338
- https://github.com/Yun-Kim/dd-trace-py/blob/0cf1580cfa638155bb7055a1767ca9083f53c24f/ddtrace/profiling/collector/stack.pyx#L326
- https://github.com/Yun-Kim/dd-trace-py/blob/0cf1580cfa638155bb7055a1767ca9083f53c24f/ddtrace/profiling/collector/_task.pyx#L125
- https://github.com/Yun-Kim/dd-trace-py/blob/0cf1580cfa638155bb7055a1767ca9083f53c24f/ddtrace/profiling/collector/_task.pyx#L52

…ect (#4952) ## Description As #4895 has not been merged yet and we don't want to block the 1.8 release any longer, this PR adds a release note entry publishing the known issue of the profiler running in Python 3.11 with the new `PyFrameObject` opaque struct changes.  ## Checklist - [ ] Followed the [library release note guidelines](https://ddtrace.readthedocs.io/en/stable/contributing.html#Release-Note-Guidelines) when writing a release note. - [ ] Add additional sections for `feat` and `fix` pull requests. - [ ] [Library documentation](https://github.com/DataDog/dd-trace-py/tree/1.x/docs) and/or [Datadog's documentation site](https://github.com/DataDog/documentation/) is updated. Link to doc PR in description.   ## Motivation  ## Design  ## Testing strategy   ## Relevant issue(s)  ## Testing strategy   ## Reviewer Checklist - [ ] Title is accurate. - [ ] Description motivates each change. - [ ] No unnecessary changes were introduced in this PR. - [ ] Avoid breaking [API](https://ddtrace.readthedocs.io/en/stable/versioning.html#interfaces) changes unless absolutely necessary. - [ ] Tests provided or description of manual testing performed is included in the code or PR. - [ ] Release note has been added and follows the [library release note guidelines](https://ddtrace.readthedocs.io/en/stable/contributing.html#Release-Note-Guidelines), or else `changelog/no-changelog` label added. - [ ] All relevant GitHub issues are correctly linked. - [ ] Change contains telemetry where appropriate (logs, metrics, etc.). - [ ] Telemetry is meaningful, actionable and does not have the potential to leak sensitive data.

Yun-Kim · 2023-02-03T00:30:40Z

@P403n1x87 Closing this PR until we can confirm that the issue persists after #5019, in which case I'll reopen this.

Yun-Kim force-pushed the yunkim/profiler-3-11-fix branch from a73455a to afb2b8e Compare January 12, 2023 19:53

Use getter functions to access frame members

3bd4a59

Yun-Kim force-pushed the yunkim/profiler-3-11-fix branch from afb2b8e to 3bd4a59 Compare January 12, 2023 19:54

Yun-Kim commented Jan 12, 2023

View reviewed changes

Comment thread ddtrace/profiling/collector/_traceback.pyx Outdated

Yun-Kim requested a review from P403n1x87 January 12, 2023 20:14

Add cpython references

61dae51

Yun-Kim commented Jan 12, 2023

View reviewed changes

Comment thread ddtrace/profiling/collector/_traceback.pyx Outdated

CPython import PyFrameObject

8840ecb

jerr0328 mentioned this pull request Jan 13, 2023

Python 3.11 profiling causes memory leaks #4899

Closed

P403n1x87 reviewed Jan 13, 2023

View reviewed changes

Comment thread ddtrace/profiling/collector/_traceback.pyx Outdated

mabdinur reviewed Jan 13, 2023

View reviewed changes

Comment thread ddtrace/profiling/collector/_traceback.pyx Outdated

Yun-Kim mentioned this pull request Jan 14, 2023

fix(profiler): decrement refcount given strong reference of frame object in python 3.11 #4901

Merged

11 tasks

miketheman mentioned this pull request Jan 16, 2023

remove ddtrace pypi/warehouse#12819

Closed

Yun-Kim force-pushed the yunkim/profiler-3-11-fix branch from ec59155 to 7bc7816 Compare January 17, 2023 18:09

Fix cpython import

259676f

Yun-Kim force-pushed the yunkim/profiler-3-11-fix branch 9 times, most recently from cb63bdd to d5eb8c6 Compare January 17, 2023 21:41

Fix C API imports

0004bd1

Yun-Kim force-pushed the yunkim/profiler-3-11-fix branch from d5eb8c6 to 0004bd1 Compare January 17, 2023 21:45

Yun-Kim added 2 commits January 17, 2023 16:56

Remove typecasting argname, typecast frame into object

791c730

Use tuple getter function and add decrefs

8f7ea30

Yun-Kim commented Jan 17, 2023

View reviewed changes

Comment thread ddtrace/profiling/collector/_traceback.pyx Outdated

brettlangdon reviewed Jan 17, 2023

View reviewed changes

Comment thread ddtrace/profiling/collector/_traceback.pyx Outdated

Comment thread ddtrace/profiling/collector/_traceback.pyx Outdated

Comment thread ddtrace/profiling/collector/_traceback.pyx Outdated

Comment thread ddtrace/profiling/collector/_traceback.pyx Outdated

Yun-Kim commented Jan 17, 2023

View reviewed changes

Separate pre/post 3.11 code branches entirely

11d1344

mabdinur reviewed Jan 19, 2023

View reviewed changes

Comment thread releasenotes/notes/profiler-3-11-traceback-fix-4c98c3ce96833df6.yaml Outdated

Resolve type errors with PyObject / PyFrameObject

77e1561

brettlangdon reviewed Jan 19, 2023

View reviewed changes

Refactor _extract_class_name

7ad29b7

brettlangdon reviewed Jan 20, 2023

View reviewed changes

Add Py_XDECREF for _extract_class_name, cast code as <object>

0f9e710

Yun-Kim force-pushed the yunkim/profiler-3-11-fix branch from d5b0a33 to 0f9e710 Compare January 20, 2023 20:02

Yun-Kim and others added 2 commits January 23, 2023 14:21

Change lineno check, add Py_XDECREF(frame) while iterating

7672550

Merge branch '1.x' into yunkim/profiler-3-11-fix

f5c5803

mabdinur reviewed Jan 23, 2023

View reviewed changes

Comment thread ddtrace/profiling/collector/_traceback.pyx Outdated

Change imports to only with 3.11+

e2afa7d

Yun-Kim mentioned this pull request Jan 23, 2023

chore(profiler): add known issue changelog for Python 3.11 PyFrameObject #4952

Merged

12 tasks

Yun-Kim mentioned this pull request Jan 25, 2023

Support Python 3.11 #4149

Closed

Yun-Kim closed this Feb 3, 2023

Yun-Kim deleted the yunkim/profiler-3-11-fix branch April 4, 2023 20:40

Uh oh!

Conversation

Yun-Kim commented Jan 12, 2023

Description

Checklist

Motivation

Design

Testing strategy

Relevant issue(s)

Testing strategy

Reviewer Checklist

Uh oh!

Yun-Kim left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Yun-Kim Jan 17, 2023

Choose a reason for hiding this comment

Uh oh!

brettlangdon Jan 18, 2023

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

brettlangdon Jan 19, 2023

Choose a reason for hiding this comment

Uh oh!

Yun-Kim Jan 19, 2023

Choose a reason for hiding this comment

Uh oh!

P403n1x87 Jan 20, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

brettlangdon Jan 20, 2023

Choose a reason for hiding this comment

Uh oh!

Yun-Kim Jan 20, 2023

Choose a reason for hiding this comment

Uh oh!

P403n1x87 Jan 21, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Yun-Kim commented Jan 23, 2023

Uh oh!

Yun-Kim commented Feb 3, 2023

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

P403n1x87 Jan 20, 2023 •

edited

Loading

P403n1x87 Jan 21, 2023 •

edited

Loading