Fix silent suppression of AssertionError exceptions in _ProfiledLock::_acquire

Author: vlad-scherbich

ddtrace/profiling/collector/_lock.py

@@ -83,41 +83,36 @@ def _acquire(self, inner_func: Callable[..., Any], *args: Any, **kwargs: Any) ->
         try:
             return inner_func(*args, **kwargs)
         finally:
-            try:
-                end: int = time.monotonic_ns()
-                self._self_acquired_at = end
+            end: int = time.monotonic_ns()
+            # Set acquired_at immediately so release() can match, even if profiling fails below
+            self._self_acquired_at = end
 
-                thread_id: int
-                thread_name: str
+            # All profiling operations below can throw - wrap them tightly
+            try:
                 thread_id, thread_name = _current_thread()
-
-                task_id: Optional[int]
-                task_name: Optional[str]
-                task_frame: Optional[FrameType]
                 task_id, task_name, task_frame = _task.get_task(thread_id)
 
                 self._maybe_update_self_name()
                 lock_name: str = (
                     "%s:%s" % (self._self_init_loc, self._self_name) if self._self_name else self._self_init_loc
                 )
 
-                frame: FrameType
+                # Get the frame for traceback
                 if task_frame is None:
                     # If we can't get the task frame, we use the caller frame. We expect acquire/release or
                     # __enter__/__exit__ to be on the stack, so we go back 2 frames.
                     frame = sys._getframe(2)
                 else:
                     frame = task_frame
 
-                frames: List[DDFrame]
                 frames, _ = _traceback.pyframe_to_frames(frame, self._self_max_nframes)
+                thread_native_id = _threading.get_thread_native_id(thread_id)
 
-                thread_native_id: int = _threading.get_thread_native_id(thread_id)
-
+                # Push sample to profiling backend
                 handle: ddup.SampleHandle = ddup.SampleHandle()
                 handle.push_monotonic_ns(end)
                 handle.push_lock_name(lock_name)
-                handle.push_acquire(end - start, 1)  # AFAICT, capture_pct does not adjust anything here
+                handle.push_acquire(end - start, 1)
                 handle.push_threadinfo(thread_id, thread_native_id, thread_name)
                 handle.push_task_id(task_id)
                 handle.push_task_name(task_name)
@@ -127,8 +122,10 @@ def _acquire(self, inner_func: Callable[..., Any], *args: Any, **kwargs: Any) ->
                 for ddframe in frames:
                     handle.push_frame(ddframe.function_name, ddframe.file_name, 0, ddframe.lineno)
                 handle.flush_sample()
-            except Exception:
-                pass  # nosec
+            except Exception as e:
+                # _maybe_update_self_name throws AssertionError exceptions which need to propagate
+                if type(e) is AssertionError:
+                    raise e
 
     def acquire(self, *args: Any, **kwargs: Any) -> Any:
         return self._acquire(self.__wrapped__.acquire, *args, **kwargs)

tests/profiling_v2/collector/test_threading.py

@@ -979,6 +979,36 @@ def test_upload_resets_profile(self) -> None:
         with pytest.raises(AssertionError):
             pprof_utils.parse_newest_profile(self.output_filename)
 
+    def test_assertion_error_suppressed_in_acquire(self) -> None:
+        """ Ensure that AssertionError exceptions is propagated in the `_acquire` method. """
+
+        with self.collector_class(capture_pct=100):
+            lock = self.lock_class()
+
+            # Patch _maybe_update_self_name to raise AssertionError
+            lock._maybe_update_self_name = mock.Mock(
+                side_effect=AssertionError("test: unexpected frame in stack")
+            )
+
+            with pytest.raises(AssertionError):
+                # acquire() will propagate AssertionError if _maybe_update_self_name raises it
+                lock.acquire()
+
+    def test_other_exceptions_are_suppressed(self) -> None:
+        """ Ensure that non - AssertionError exceptions are silently suppressed in the `_acquire` method. """
+
+        with self.collector_class(capture_pct=100):
+            lock = self.lock_class()
+            
+            # Patch _maybe_update_self_name to raise RuntimeError
+            lock._maybe_update_self_name = mock.Mock(
+                side_effect=RuntimeError("Some profiling error")
+            )
+
+            # RuntimeError should be caught and suppressed - lock operations should succeed
+            lock.acquire()
+            lock.release()
+
 
 class TestThreadingLockCollector(BaseThreadingLockCollectorTest):
     """Test Lock profiling"""