Skip to content

Commit 12e5e54

Browse files
Fix silent suppression of AssertionError exceptions in _ProfiledLock::_acquire
1 parent d5872b9 commit 12e5e54

File tree

3 files changed

+34
-2
lines changed

3 files changed

+34
-2
lines changed

ddtrace/profiling/collector/_lock.py

Lines changed: 4 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -127,8 +127,10 @@ def _acquire(self, inner_func: Callable[..., Any], *args: Any, **kwargs: Any) ->
127127
for ddframe in frames:
128128
handle.push_frame(ddframe.function_name, ddframe.file_name, 0, ddframe.lineno)
129129
handle.flush_sample()
130-
except Exception:
131-
pass # nosec
130+
except Exception as e:
131+
# _maybe_update_self_name throws AssertionError exceptions which need to propagate
132+
if type(e) is AssertionError:
133+
raise e
132134

133135
def acquire(self, *args: Any, **kwargs: Any) -> Any:
134136
return self._acquire(self.__wrapped__.acquire, *args, **kwargs)
Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,4 @@
1+
---
2+
fixes:
3+
- |
4+
profiling: This fix resolves an issue where AssertionError exceptions were silently suppressed in the `_acquire` method of the Lock profiler (note: this only occurs when assertions are enabled.)

tests/profiling_v2/collector/test_threading.py

Lines changed: 26 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -979,6 +979,32 @@ def test_upload_resets_profile(self) -> None:
979979
with pytest.raises(AssertionError):
980980
pprof_utils.parse_newest_profile(self.output_filename)
981981

982+
def test_assertion_error_suppressed_in_acquire(self) -> None:
983+
"""Ensure that AssertionError exceptions is propagated in the `_acquire` method."""
984+
985+
with self.collector_class(capture_pct=100):
986+
lock = self.lock_class()
987+
988+
# Patch _maybe_update_self_name to raise AssertionError
989+
lock._maybe_update_self_name = mock.Mock(side_effect=AssertionError("test: unexpected frame in stack"))
990+
991+
with pytest.raises(AssertionError):
992+
# acquire() will propagate AssertionError if _maybe_update_self_name raises it
993+
lock.acquire()
994+
995+
def test_other_exceptions_are_suppressed(self) -> None:
996+
"""Ensure that non - AssertionError exceptions are silently suppressed in the `_acquire` method."""
997+
998+
with self.collector_class(capture_pct=100):
999+
lock = self.lock_class()
1000+
1001+
# Patch _maybe_update_self_name to raise RuntimeError
1002+
lock._maybe_update_self_name = mock.Mock(side_effect=RuntimeError("Some profiling error"))
1003+
1004+
# RuntimeError should be caught and suppressed - lock operations should succeed
1005+
lock.acquire()
1006+
lock.release()
1007+
9821008

9831009
class TestThreadingLockCollector(BaseThreadingLockCollectorTest):
9841010
"""Test Lock profiling"""

0 commit comments

Comments
 (0)