Skip to content

Commit 75241fe

Browse files
authored
fix(core): use unpatched lock from forksafe module (#15151)
## Description <!-- Provide an overview of the change and motivation for the change --> From a recent profiling escalation, we've seen a flamegraph where forksafe lock is calling into patched profiling lock. <img width="918" height="329" alt=" " src="https://github.com/user-attachments/assets/3bff0b94-4ad7-49b1-847f-61812e2f43eb" /> The library used in above flamegraph is version 3.17.0. From the leaf frame 1. [_release (_lock.py:#155)](https://github.com/DataDog/dd-trace-py/blob/7451e8468236787abefb164407a8bd75dd005501/ddtrace/profiling/collector/_lock.py#L155) 2. [\_\_exit__ (_lock.py:L#206)](https://github.com/DataDog/dd-trace-py/blob/7451e8468236787abefb164407a8bd75dd005501/ddtrace/profiling/collector/_lock.py#L206) 3. [start (service.py:L#57)](https://github.com/DataDog/dd-trace-py/blob/7451e8468236787abefb164407a8bd75dd005501/ddtrace/internal/service.py#L57) https://github.com/DataDog/dd-trace-py/blob/7451e8468236787abefb164407a8bd75dd005501/ddtrace/internal/service.py#L57-L61 `self._service_lock` is declared as a `forksafe.Lock()` https://github.com/DataDog/dd-trace-py/blob/7451e8468236787abefb164407a8bd75dd005501/ddtrace/internal/service.py#L33 ## Testing <!-- Describe your testing strategy or note what tests are included --> ## Risks <!-- Note any risks associated with this change, or "None" if no risks --> ## Additional Notes <!-- Any other information that would be helpful for reviewers -->
1 parent 83a032e commit 75241fe

File tree

4 files changed

+49
-8
lines changed

4 files changed

+49
-8
lines changed

ddtrace/internal/_unpatched.py

Lines changed: 5 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -10,9 +10,13 @@
1010
# to get a reference to the right threading module.
1111
import threading as _threading # noqa
1212
import gc as _gc # noqa
13-
1413
import sys
1514

15+
threading_Lock = _threading.Lock
16+
threading_RLock = _threading.RLock
17+
threading_Event = _threading.Event
18+
19+
1620
previous_loaded_modules = frozenset(sys.modules.keys())
1721
from subprocess import Popen as unpatched_Popen # noqa # nosec B404
1822
from os import close as unpatched_close # noqa: F401, E402

ddtrace/internal/forksafe.py

Lines changed: 8 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -5,12 +5,13 @@
55
import functools
66
import logging
77
import os
8-
import threading
98
import typing
109
import weakref
1110

1211
import wrapt
1312

13+
from ddtrace.internal import _unpatched
14+
1415

1516
log = logging.getLogger(__name__)
1617

@@ -138,13 +139,13 @@ def _reset_object(self):
138139
self.__wrapped__ = self._self_wrapped_class()
139140

140141

141-
def Lock() -> threading.Lock:
142-
return ResetObject(threading.Lock) # type: ignore
142+
def Lock() -> _unpatched.threading_Lock:
143+
return ResetObject(_unpatched.threading_Lock) # type: ignore
143144

144145

145-
def RLock() -> threading.RLock:
146-
return ResetObject(threading.RLock) # type: ignore
146+
def RLock() -> _unpatched.threading_RLock:
147+
return ResetObject(_unpatched.threading_RLock) # type: ignore
147148

148149

149-
def Event() -> threading.Event:
150-
return ResetObject(threading.Event) # type: ignore
150+
def Event() -> _unpatched.threading_Event:
151+
return ResetObject(_unpatched.threading_Event) # type: ignore
Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,4 @@
1+
---
2+
fixes:
3+
- |
4+
core: This fix resolves an issue where forksafe locks used patched threading primitives from the profiling module, causing performance issues. The forksafe module now uses unpatched threading primitives (``Lock``, ``RLock``, ``Event``).

tests/internal/test_forksafe.py

Lines changed: 32 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,6 @@
11
from collections import Counter
22
import os
3+
import sys
34

45
import pytest
56

@@ -204,6 +205,37 @@ def test_lock_fork():
204205
assert exit_code == 12
205206

206207

208+
@pytest.mark.skipif(sys.version_info >= (3, 14), reason="Profiling is not supported on Python 3.14 yet")
209+
@pytest.mark.subprocess(
210+
env=dict(DD_PROFILING_ENABLED="1"),
211+
ddtrace_run=True,
212+
)
213+
def test_lock_unpatched():
214+
"""Check that a forksafe.Lock is not patched when profiling is enabled."""
215+
216+
from ddtrace.internal import forksafe
217+
from ddtrace.profiling import bootstrap
218+
from ddtrace.profiling.collector.threading import ThreadingLockCollector
219+
220+
# When Profiler is started, bootstrap.profiler is set to the Profiler
221+
# instance. We explicitly access the Profiler instance and the collector list
222+
# to verify that the forksafe.Lock is not using the same class that is
223+
# patched by ThreadingLockCollector that's running.
224+
profiler = bootstrap.profiler._profiler
225+
lock_collector = None
226+
for c in profiler._collectors:
227+
if isinstance(c, ThreadingLockCollector):
228+
lock_collector = c
229+
break
230+
231+
assert lock_collector is not None, "ThreadingLockCollector not found in profiler collectors"
232+
233+
lock = forksafe.Lock()
234+
assert (
235+
lock_collector._get_patch_target() is not lock._self_wrapped_class
236+
), "forksafe.Lock is using the same class that is patched by ThreadingLockCollector"
237+
238+
207239
def test_rlock_basic():
208240
# type: (...) -> None
209241
"""Check that a forksafe.RLock implements the correct threading.RLock interface"""

0 commit comments

Comments
 (0)