Skip to content

Commit 512a7c7

Browse files
committed
fix(profiling): remove unnecessary access to frame.f_locals (#15434)
We've been seeing a low number of crashes that have the following stack traces. ``` PyFrame_FastToLocalsWithError _PyObject_GenericGetAttrWithDict __pyx_f_7ddtrace_9profiling_9collector_10_traceback__extract_class_name.constprop.0 __pyx_pw_7ddtrace_9profiling_9collector_10_traceback_5pyframe_to_frames ``` which corresponds to `value = frame.f_locals[argname]` in `extract_class_name()` function. Fore more details see notebook, https://app.datadoghq.com/notebook/13515661/pyframe-to-frames-crashes The reason this could lead to a crash is that the `frame` could be in an invalid state, partially deallocated, or corrupted. And this doesn't happen on our stack sampler (Echion) as it doesn't use it, and just use `co_qualname` (Python 3.11+) or `co_name` (Python < 3.11) This code is no longer needed as we don't use `class_name` field from `DDFrame` named tuple. Memory profiler has been putting an empty string. Lock profiler populated it but then it didn't export it in the sample. More appropriate way to populate the class name for Python versions 3.11+ would be using `co_qualname` as Echion does. I'll follow up with this in a separate PR. And this is why we see fully qualified name on our CPU time view, but function name only on memory/lock views. CPU time view <img width="206" height="94" alt="Screenshot 2025-11-26 at 2 02 50 PM" src="https://github.com/user-attachments/assets/a56efc8a-0dd2-41c2-8fab-5cc786d3a737" /> Memory view <img width="141" height="94" alt="Screenshot 2025-11-26 at 2 02 35 PM" src="https://github.com/user-attachments/assets/efa917f8-40c8-4818-88e7-9c157bdfd5cf" /> <!-- Provide an overview of the change and motivation for the change --> <!-- Describe your testing strategy or note what tests are included --> <!-- Note any risks associated with this change, or "None" if no risks --> <!-- Any other information that would be helpful for reviewers --> (cherry picked from commit 124b19d)
1 parent ea3cfae commit 512a7c7

File tree

8 files changed

+15
-56
lines changed

8 files changed

+15
-56
lines changed

ddtrace/profiling/collector/_memalloc_tb.cpp

Lines changed: 1 addition & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -13,8 +13,6 @@
1313
/* A string containing "<unknown>" just in case we can't store the real function
1414
* or file name. */
1515
static PyObject* unknown_name = NULL;
16-
/* A string containing "" */
17-
static PyObject* empty_string = NULL;
1816

1917
#define TRACEBACK_SIZE(NFRAME) (sizeof(traceback_t) + sizeof(frame_t) * (NFRAME - 1))
2018

@@ -135,12 +133,6 @@ memalloc_tb_init(uint16_t max_nframe)
135133
PyUnicode_InternInPlace(&unknown_name);
136134
}
137135

138-
if (empty_string == NULL) {
139-
empty_string = PyUnicode_FromString("");
140-
if (empty_string == NULL)
141-
return -1;
142-
PyUnicode_InternInPlace(&empty_string);
143-
}
144136
return 0;
145137
}
146138

@@ -297,7 +289,7 @@ traceback_to_tuple(traceback_t* tb)
297289
PyObject* stack = PyTuple_New(tb->nframe);
298290

299291
for (uint16_t nframe = 0; nframe < tb->nframe; nframe++) {
300-
PyObject* frame_tuple = PyTuple_New(4);
292+
PyObject* frame_tuple = PyTuple_New(3);
301293

302294
frame_t* frame = &tb->frames[nframe];
303295

@@ -306,9 +298,6 @@ traceback_to_tuple(traceback_t* tb)
306298
PyTuple_SET_ITEM(frame_tuple, 1, PyLong_FromUnsignedLong(frame->lineno));
307299
Py_INCREF(frame->name);
308300
PyTuple_SET_ITEM(frame_tuple, 2, frame->name);
309-
/* Class name */
310-
Py_INCREF(empty_string);
311-
PyTuple_SET_ITEM(frame_tuple, 3, empty_string);
312301

313302
// Try to set the class. If we cannot (e.g., if the sofile is reloaded
314303
// without module initialization), then this will result in an error if

ddtrace/profiling/collector/_traceback.pyx

Lines changed: 2 additions & 27 deletions
Original file line numberDiff line numberDiff line change
@@ -8,31 +8,6 @@ from ddtrace.profiling.event import DDFrame
88
log = get_logger(__name__)
99

1010

11-
cpdef _extract_class_name(frame):
12-
# type: (...) -> str
13-
"""Extract class name from a frame, if possible.
14-
15-
:param frame: The frame object.
16-
"""
17-
code = frame.f_code
18-
if code.co_argcount > 0:
19-
# Retrieve the name of the first argument, if the code object has any
20-
argname = code.co_varnames[0]
21-
try:
22-
value = frame.f_locals[argname]
23-
except Exception:
24-
log.debug("Unable to extract class name from frame %r", frame, exc_info=True)
25-
return ""
26-
try:
27-
if argname == "self":
28-
return object.__getattribute__(type(value), "__name__") # use type() and object.__getattribute__ to avoid side-effects
29-
if argname == "cls":
30-
return object.__getattribute__(value, "__name__")
31-
except AttributeError:
32-
return ""
33-
return ""
34-
35-
3611
cpdef traceback_to_frames(traceback, max_nframes):
3712
"""Serialize a Python traceback object into a list of tuple of (filename, lineno, function_name).
3813
@@ -48,7 +23,7 @@ cpdef traceback_to_frames(traceback, max_nframes):
4823
frame = tb.tb_frame
4924
code = frame.f_code
5025
lineno = 0 if frame.f_lineno is None else frame.f_lineno
51-
frames.insert(0, DDFrame(code.co_filename, lineno, code.co_name, _extract_class_name(frame)))
26+
frames.insert(0, DDFrame(code.co_filename, lineno, code.co_name))
5227
nframes += 1
5328
tb = tb.tb_next
5429
return frames, nframes
@@ -92,7 +67,7 @@ cpdef pyframe_to_frames(frame, max_nframes):
9267
return [], 0
9368

9469
lineno = 0 if frame.f_lineno is None else frame.f_lineno
95-
frames.append(DDFrame(code.co_filename, lineno, code.co_name, _extract_class_name(frame)))
70+
frames.append(DDFrame(code.co_filename, lineno, code.co_name))
9671
nframes += 1
9772
frame = frame.f_back
9873
return frames, nframes

ddtrace/profiling/collector/stack.pyx

Lines changed: 0 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -317,7 +317,6 @@ cdef stack_collect(ignore_profiler, thread_time, max_nframes, interval, wall_tim
317317
handle.push_threadinfo(thread_id, thread_native_id, thread_name)
318318
handle.push_task_id(task_id)
319319
handle.push_task_name(task_name)
320-
handle.push_class_name(frames[0].class_name)
321320
for frame in frames:
322321
handle.push_frame(frame.function_name, frame.file_name, 0, frame.lineno)
323322
handle.flush_sample()
@@ -330,7 +329,6 @@ cdef stack_collect(ignore_profiler, thread_time, max_nframes, interval, wall_tim
330329
handle.push_cputime( cpu_time, 1)
331330
handle.push_walltime( wall_time, 1)
332331
handle.push_threadinfo(thread_id, thread_native_id, thread_name)
333-
handle.push_class_name(frames[0].class_name)
334332
for frame in frames:
335333
handle.push_frame(frame.function_name, frame.file_name, 0, frame.lineno)
336334
handle.push_span(span)
@@ -346,7 +344,6 @@ cdef stack_collect(ignore_profiler, thread_time, max_nframes, interval, wall_tim
346344
handle.push_monotonic_ns(now_ns)
347345
handle.push_threadinfo(thread_id, thread_native_id, thread_name)
348346
handle.push_exceptioninfo(exc_type, 1)
349-
handle.push_class_name(frames[0].class_name)
350347
for frame in frames:
351348
handle.push_frame(frame.function_name, frame.file_name, 0, frame.lineno)
352349
handle.push_span(span)

ddtrace/profiling/event.py

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -2,5 +2,5 @@
22
import typing
33

44

5-
DDFrame = namedtuple("DDFrame", ["file_name", "lineno", "function_name", "class_name"])
5+
DDFrame = namedtuple("DDFrame", ["file_name", "lineno", "function_name"])
66
StackTraceType = typing.List[DDFrame]
Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,5 @@
1+
---
2+
fixes:
3+
- |
4+
profiling: Fixes a segmentation fault caused by accessing ``frame.f_locals``
5+
while trying to retrieve class name of a ``PyFrameObject``.

tests/profiling/collector/test_memalloc.py

Lines changed: 2 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -90,11 +90,10 @@ def test_iter_events():
9090
__file__,
9191
_ALLOC_LINE_NUMBER,
9292
"<listcomp>" if sys.version_info < (3, 12) else "_allocate_1k",
93-
"",
9493
):
9594
assert thread_id == threading.main_thread().ident
9695
if sys.version_info < (3, 12) and len(stack) > 1:
97-
assert stack[1] == DDFrame(__file__, _ALLOC_LINE_NUMBER, "_allocate_1k", "")
96+
assert stack[1] == DDFrame(__file__, _ALLOC_LINE_NUMBER, "_allocate_1k")
9897
object_count += sample.count
9998

10099
assert object_count >= 1000
@@ -156,12 +155,11 @@ def test_iter_events_multi_thread():
156155
__file__,
157156
_ALLOC_LINE_NUMBER,
158157
"<listcomp>" if sys.version_info < (3, 12) else "_allocate_1k",
159-
"",
160158
):
161159
if thread_id == threading.main_thread().ident:
162160
count_object += sample.count
163161
if sys.version_info < (3, 12) and len(stack) > 1:
164-
assert stack[1] == DDFrame(__file__, _ALLOC_LINE_NUMBER, "_allocate_1k", "")
162+
assert stack[1] == DDFrame(__file__, _ALLOC_LINE_NUMBER, "_allocate_1k")
165163
elif thread_id == t.ident:
166164
count_thread += sample.count
167165
entry = 2 if sys.version_info < (3, 12) else 1
@@ -205,7 +203,6 @@ def _test_heap_impl(collector, max_nframe):
205203
__file__,
206204
_ALLOC_LINE_NUMBER,
207205
"<listcomp>" if sys.version_info < (3, 12) else "_allocate_1k",
208-
"",
209206
):
210207
break
211208
else:
@@ -229,7 +226,6 @@ def _test_heap_impl(collector, max_nframe):
229226
__file__,
230227
_ALLOC_LINE_NUMBER,
231228
"<listcomp>" if sys.version_info < (3, 12) else "_allocate_1k",
232-
"",
233229
):
234230
break
235231
else:
@@ -258,7 +254,6 @@ def _test_heap_impl(collector, max_nframe):
258254
__file__,
259255
_ALLOC_LINE_NUMBER,
260256
"<listcomp>" if sys.version_info < (3, 12) else "_allocate_1k",
261-
"",
262257
)
263258
and stack[entry].function_name == "_test_heap_impl"
264259
):

tests/profiling/collector/test_stack.py

Lines changed: 2 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -114,11 +114,9 @@ def _find_sleep_event(events, class_name):
114114

115115
for e in events:
116116
for frame in e.frames:
117-
if frame[0] == __file__.replace(".pyc", ".py") and frame[2] == "sleep_class" and frame[3] == class_name:
117+
if frame[0] == __file__.replace(".pyc", ".py") and frame[2] == "sleep_class":
118118
class_method_found = True
119-
elif (
120-
frame[0] == __file__.replace(".pyc", ".py") and frame[2] == "sleep_instance" and frame[3] == class_name
121-
):
119+
elif frame[0] == __file__.replace(".pyc", ".py") and frame[2] == "sleep_instance":
122120
class_classmethod_found = True
123121

124122
if class_method_found and class_classmethod_found:

tests/profiling/collector/test_traceback.py

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -17,6 +17,6 @@ def test_check_traceback_to_frames():
1717

1818
this_file = __file__.replace(".pyc", ".py")
1919
assert frames == [
20-
(this_file, 7, "_x", ""),
21-
(this_file, 15, "test_check_traceback_to_frames", ""),
20+
(this_file, 7, "_x"),
21+
(this_file, 15, "test_check_traceback_to_frames"),
2222
]

0 commit comments

Comments
 (0)