Skip to content

Commit 99e5f4d

Browse files
fix(profiler): decrement refcount given strong reference of frame object in python 3.11 [backport] (#4951)
## Description Backports for #4901 and #4937 to 1.7. <!-- If this is a breaking change, explain why it is necessary. Breaking changes must append `!` after the type/scope. See https://ddtrace.readthedocs.io/en/stable/contributing.html for more details. --> ## 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. <!-- Copy and paste the relevant snippet based on the type of pull request --> <!-- START feat --> ## Motivation <!-- Expand on why the change is required, include relevant context for reviewers --> ## Design <!-- Include benefits from the change as well as possible drawbacks and trade-offs --> ## Testing strategy <!-- Describe the automated tests and/or the steps for manual testing. <!-- END feat --> <!-- START fix --> ## Relevant issue(s) <!-- Link the pull request to any issues related to the fix. Use keywords for links to automate closing the issues once the pull request is merged. --> ## Testing strategy <!-- Describe any added regression tests and/or the manual testing performed. --> <!-- END fix --> ## 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. Co-authored-by: Brett Langdon <[email protected]>
1 parent ef7ee6c commit 99e5f4d

File tree

3 files changed

+27
-2
lines changed

3 files changed

+27
-2
lines changed

ddtrace/profiling/collector/stack.pyx

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -257,6 +257,7 @@ cdef collect_threads(thread_id_ignore_list, thread_time, thread_span_links) with
257257
if exc_tb:
258258
current_exceptions[tstate.thread_id] = (<object>exc_type, <object>exc_tb)
259259
Py_XDECREF(exc_tb)
260+
Py_XDECREF(frame)
260261
ELSE:
261262
frame = tstate.frame
262263
if frame:
Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,5 @@
1+
---
2+
fixes:
3+
- |
4+
profiling: This fix resolves an issue in Python 3.11 where a PyFrameObject strong reference count was not properly
5+
decremented in the stack collector.

tests/profiling/collector/test_stack.py

Lines changed: 21 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,10 +1,12 @@
11
# -*- encoding: utf-8 -*-
22
import collections
3+
import gc
34
import os
45
import sys
56
import threading
67
import time
78
import timeit
9+
from types import FrameType
810
import typing
911
import uuid
1012

@@ -400,7 +402,7 @@ def test_exception_collection():
400402
assert e.sampling_period > 0
401403
assert e.thread_id == nogevent.thread_get_ident()
402404
assert e.thread_name == "MainThread"
403-
assert e.frames == [(__file__, 394, "test_exception_collection", "")]
405+
assert e.frames == [(__file__, 396, "test_exception_collection", "")]
404406
assert e.nframes == 1
405407
assert e.exc_type == ValueError
406408

@@ -432,7 +434,7 @@ def test_exception_collection_trace(
432434
assert e.sampling_period > 0
433435
assert e.thread_id == nogevent.thread_get_ident()
434436
assert e.thread_name == "MainThread"
435-
assert e.frames == [(__file__, 421, "test_exception_collection_trace", "")]
437+
assert e.frames == [(__file__, 423, "test_exception_collection_trace", "")]
436438
assert e.nframes == 1
437439
assert e.exc_type == ValueError
438440
assert e.span_id == span.span_id
@@ -743,3 +745,20 @@ def _nothing():
743745
# assert (exact_time * 0.7) <= values.pop() <= (exact_time * 1.3)
744746

745747
assert values.pop() > 0
748+
749+
750+
@pytest.mark.skipif(sys.version_info < (3, 11, 0), reason="PyFrameObjects are lazy-created objects in Python 3.11+")
751+
def test_collect_ensure_all_frames_gc():
752+
# Regression test for memory leak with lazy PyFrameObjects in Python 3.11+
753+
def _foo():
754+
pass
755+
756+
r = recorder.Recorder()
757+
s = stack.StackCollector(r)
758+
759+
with s:
760+
for _ in range(100):
761+
_foo()
762+
763+
gc.collect() # Make sure we don't race with gc when we check frame objects
764+
assert sum(isinstance(_, FrameType) for _ in gc.get_objects()) == 0

0 commit comments

Comments
 (0)