Skip to content

Commit 650f8fd

Browse files
fix(profiling): thread lock segfault with 3.12 [backport 2.3] (#7847)
Backport 096445a from #7822 to 2.3. We use the new _PyThread_CurrentFrames and _PyThread_CurrentExceptions to retrieve the current top frames and exceptions for each thread to avoid the use of a lock that has been removed in CPython 3.12. ## Checklist - [x] Change(s) are motivated and described in the PR description. - [x] Testing strategy is described if automated tests are not included in the PR. - [x] Risk is outlined (performance impact, potential for breakage, maintainability, etc). - [x] Change is maintainable (easy to change, telemetry, documentation). - [x] [Library release note guidelines](https://ddtrace.readthedocs.io/en/stable/releasenotes.html) are followed. If no release note is required, add label `changelog/no-changelog`. - [x] Documentation is included (in-code, generated user docs, [public corp docs](https://github.com/DataDog/documentation/)). - [x] Backport labels are set (if [applicable](https://ddtrace.readthedocs.io/en/latest/contributing.html#backporting)) ## Reviewer Checklist - [x] Title is accurate. - [x] No unnecessary changes are introduced. - [x] Description motivates each change. - [x] Avoids breaking [API](https://ddtrace.readthedocs.io/en/stable/versioning.html#interfaces) changes unless absolutely necessary. - [x] Testing strategy adequately addresses listed risk(s). - [x] Change is maintainable (easy to change, telemetry, documentation). - [x] Release note makes sense to a user of the library. - [x] Reviewer has explicitly acknowledged and discussed the performance implications of this PR as reported in the benchmarks PR comment. - [x] Backport labels are set in a manner that is consistent with the [release branch maintenance policy](https://ddtrace.readthedocs.io/en/latest/contributing.html#backporting) - [x] If this PR touches code that signs or publishes builds or packages, or handles credentials of any kind, I've requested a review from `@DataDog/security-design-and-guidance`. - [x] This PR doesn't touch any of that. Co-authored-by: Gabriele N. Tornetta <[email protected]>
1 parent c893809 commit 650f8fd

File tree

2 files changed

+40
-52
lines changed

2 files changed

+40
-52
lines changed

ddtrace/profiling/collector/stack.pyx

Lines changed: 35 additions & 52 deletions
Original file line numberDiff line numberDiff line change
@@ -25,7 +25,7 @@ from ddtrace.settings.profiling import config
2525
# These are special features that might not be available depending on your Python version and platform
2626
FEATURES = {
2727
"cpu-time": False,
28-
"stack-exceptions": False,
28+
"stack-exceptions": True,
2929
"transparent_events": False,
3030
}
3131

@@ -150,12 +150,16 @@ ELSE:
150150

151151

152152
from cpython.object cimport PyObject
153+
from cpython.ref cimport Py_DECREF
153154

155+
cdef extern from "<pystate.h>":
156+
PyObject* _PyThread_CurrentFrames()
154157

155-
# The head lock (the interpreter mutex) is only exposed in a data structure in Python ≥ 3.7
156-
IF UNAME_SYSNAME != "Windows" and PY_VERSION_HEX >= 0x03070000:
157-
FEATURES['stack-exceptions'] = True
158+
IF PY_VERSION_HEX >= 0x030b0000:
159+
cdef extern from "<pystate.h>":
160+
PyObject* _PyThread_CurrentExceptions()
158161

162+
ELIF UNAME_SYSNAME != "Windows":
159163
from cpython cimport PyInterpreterState
160164
from cpython cimport PyInterpreterState_Head
161165
from cpython cimport PyInterpreterState_Next
@@ -167,9 +171,6 @@ IF UNAME_SYSNAME != "Windows" and PY_VERSION_HEX >= 0x03070000:
167171
from cpython.pythread cimport PyThread_type_lock
168172
from cpython.pythread cimport WAIT_LOCK
169173

170-
IF PY_VERSION_HEX >= 0x030b0000:
171-
from cpython.ref cimport Py_XDECREF
172-
173174
cdef extern from "<Python.h>":
174175
# This one is provided as an opaque struct from Cython's cpython/pystate.pxd,
175176
# but we need to access some of its fields so we redefine it here.
@@ -179,14 +180,10 @@ IF UNAME_SYSNAME != "Windows" and PY_VERSION_HEX >= 0x03070000:
179180

180181
_PyErr_StackItem * _PyErr_GetTopmostException(PyThreadState *tstate)
181182

182-
IF PY_VERSION_HEX >= 0x030b0000:
183-
ctypedef struct _PyErr_StackItem:
184-
PyObject* exc_value
185-
ELSE:
186-
ctypedef struct _PyErr_StackItem:
187-
PyObject* exc_type
188-
PyObject* exc_value
189-
PyObject* exc_traceback
183+
ctypedef struct _PyErr_StackItem:
184+
PyObject* exc_type
185+
PyObject* exc_value
186+
PyObject* exc_traceback
190187

191188
PyObject* PyException_GetTraceback(PyObject* exc)
192189
PyObject* Py_TYPE(PyObject* ob)
@@ -222,24 +219,35 @@ IF UNAME_SYSNAME != "Windows" and PY_VERSION_HEX >= 0x03070000:
222219
cdef extern from "<Python.h>":
223220
PyObject* PyThreadState_GetFrame(PyThreadState* tstate)
224221
ELSE:
225-
from cpython.ref cimport Py_DECREF
226-
227-
cdef extern from "<pystate.h>":
228-
PyObject* _PyThread_CurrentFrames()
229-
222+
FEATURES['stack-exceptions'] = False
230223

231224

232225
cdef collect_threads(thread_id_ignore_list, thread_time, thread_span_links) with gil:
233-
cdef dict current_exceptions = {}
226+
cdef dict running_threads = <dict>_PyThread_CurrentFrames()
227+
Py_DECREF(running_threads)
234228

235-
IF UNAME_SYSNAME != "Windows" and PY_VERSION_HEX >= 0x03070000:
229+
IF PY_VERSION_HEX >= 0x030b0000:
230+
cdef dict current_exceptions = <dict>_PyThread_CurrentExceptions()
231+
Py_DECREF(current_exceptions)
232+
233+
for thread_id, exc_info in current_exceptions.items():
234+
if exc_info is None:
235+
continue
236+
IF PY_VERSION_HEX >= 0x030c0000:
237+
exc_type = type(exc_info)
238+
exc_traceback = getattr(exc_info, "__traceback__", None)
239+
ELSE:
240+
exc_type, exc_value, exc_traceback = exc_info
241+
current_exceptions[thread_id] = exc_type, exc_traceback
242+
243+
ELIF UNAME_SYSNAME != "Windows":
236244
cdef PyInterpreterState* interp
237245
cdef PyThreadState* tstate
238246
cdef _PyErr_StackItem* exc_info
239247
cdef PyThread_type_lock lmutex = _PyRuntime.interpreters.mutex
240248
cdef PyObject* exc_type
241249
cdef PyObject* exc_tb
242-
cdef dict running_threads = {}
250+
cdef dict current_exceptions = {}
243251

244252
# This is an internal lock but we do need it.
245253
# See https://bugs.python.org/issue1021318
@@ -253,40 +261,16 @@ cdef collect_threads(thread_id_ignore_list, thread_time, thread_span_links) with
253261
while interp:
254262
tstate = PyInterpreterState_ThreadHead(interp)
255263
while tstate:
256-
# The frame can be NULL
257-
# Python 3.11 moved PyFrameObject to internal C API and cannot be directly accessed from tstate
258-
IF PY_VERSION_HEX >= 0x030b0000:
259-
frame = PyThreadState_GetFrame(tstate)
260-
if frame:
261-
running_threads[tstate.thread_id] = <object>frame
262-
exc_info = _PyErr_GetTopmostException(tstate)
263-
if exc_info and exc_info.exc_value and <object> exc_info.exc_value is not None:
264-
# Python 3.11 removed exc_type, exc_traceback from exception representations,
265-
# can instead derive exc_type and exc_traceback from remaining exc_value field
266-
exc_type = Py_TYPE(exc_info.exc_value)
267-
exc_tb = PyException_GetTraceback(exc_info.exc_value)
268-
if exc_tb:
269-
current_exceptions[tstate.thread_id] = (<object>exc_type, <object>exc_tb)
270-
Py_XDECREF(exc_tb)
271-
Py_XDECREF(frame)
272-
ELSE:
273-
frame = tstate.frame
274-
if frame:
275-
running_threads[tstate.thread_id] = <object>frame
276-
exc_info = _PyErr_GetTopmostException(tstate)
277-
if exc_info and exc_info.exc_type and exc_info.exc_traceback:
278-
current_exceptions[tstate.thread_id] = (<object>exc_info.exc_type, <object>exc_info.exc_traceback)
264+
exc_info = _PyErr_GetTopmostException(tstate)
265+
if exc_info and exc_info.exc_type and exc_info.exc_traceback:
266+
current_exceptions[tstate.thread_id] = (<object>exc_info.exc_type, <object>exc_info.exc_traceback)
279267
tstate = PyThreadState_Next(tstate)
280268

281269
interp = PyInterpreterState_Next(interp)
282270
finally:
283271
PyThread_release_lock(lmutex)
284272
ELSE:
285-
cdef dict running_threads = <dict>_PyThread_CurrentFrames()
286-
287-
# Now that we own the ref via <dict> casting, we can safely decrease the default refcount
288-
# so we don't leak the object
289-
Py_DECREF(running_threads)
273+
cdef dict current_exceptions = {}
290274

291275
cdef dict cpu_times = thread_time(running_threads.keys())
292276

@@ -305,7 +289,6 @@ cdef collect_threads(thread_id_ignore_list, thread_time, thread_span_links) with
305289
)
306290

307291

308-
309292
cdef stack_collect(ignore_profiler, thread_time, max_nframes, interval, wall_time, thread_span_links, collect_endpoint):
310293
# Do not use `threading.enumerate` to not mess with locking (gevent!)
311294
thread_id_ignore_list = {
Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,5 @@
1+
---
2+
fixes:
3+
- |
4+
profiling: Fix a segmentation fault with CPython 3.12 when sampling thread
5+
stacks.

0 commit comments

Comments
 (0)