Fixed double recursion counting in the TaskBlockingListener (#3205)

getsentry · Mar 11, 2024 · ce68f79 · ce68f79
1 parent e24b2c3
commit ce68f79
Show file tree

Hide file tree

Showing 6 changed files with 20 additions and 31 deletions.
diff --git a/src/Sentry/Ben.BlockingDetector/IRecursionTracker.cs b/src/Sentry/Ben.BlockingDetector/IRecursionTracker.cs
@@ -4,6 +4,5 @@ internal interface IRecursionTracker
 {
     void Recurse();
     void Backtrack();
-    bool IsRecursive();
     bool IsFirstRecursion();
 }
diff --git a/src/Sentry/Ben.BlockingDetector/ITaskBlockingListenerState.cs b/src/Sentry/Ben.BlockingDetector/ITaskBlockingListenerState.cs
@@ -1,6 +1,6 @@
 namespace Sentry.Ben.BlockingDetector;
 
-internal interface ITaskBlockingListenerState : IRecursionTracker
+internal interface ITaskBlockingListenerState
 {
     void Suppress();
     bool IsSuppressed();

diff --git a/src/Sentry/Ben.BlockingDetector/StaticRecursionTracker.cs b/src/Sentry/Ben.BlockingDetector/StaticRecursionTracker.cs
@@ -5,7 +5,13 @@ internal class StaticRecursionTracker : IRecursionTracker
     [ThreadStatic] private static int RecursionCount;
 
     public void Recurse() => RecursionCount++;
-    public void Backtrack() => RecursionCount--;
+    public void Backtrack()
+    {
+        if (RecursionCount > 0)
+        {
+            RecursionCount--;
+        }
+    }
     public bool IsRecursive() => RecursionCount > 0;
     public bool IsFirstRecursion() => RecursionCount == 1;
 }
diff --git a/src/Sentry/Ben.BlockingDetector/StaticTaskBlockingListenerState.cs b/src/Sentry/Ben.BlockingDetector/StaticTaskBlockingListenerState.cs
@@ -1,6 +1,6 @@
 namespace Sentry.Ben.BlockingDetector;
 
-internal class StaticTaskBlockingListenerState : StaticRecursionTracker, ITaskBlockingListenerState
+internal class StaticTaskBlockingListenerState : ITaskBlockingListenerState
 {
     [ThreadStatic] private static int SuppressionCount;
 

diff --git a/src/Sentry/Ben.BlockingDetector/TaskBlockingListener.cs b/src/Sentry/Ben.BlockingDetector/TaskBlockingListener.cs
@@ -58,13 +58,10 @@ internal void DoHandleEvent(int eventId, ReadOnlyCollection<object?>? payload)
                 payload[3] is int value && // Behavior
                 value == 1) // TaskWaitBehavior.Synchronous
             {
-                _state.Recurse();
                 monitor?.BlockingStart(DetectionSource.EventListener);
             }
-            else if (eventId == 11 // TASKWAITEND_ID
-                     && _state.IsRecursive())
+            else if (eventId == 11) // TASKWAITEND_ID
             {
-                _state.Backtrack();
                 monitor?.BlockingEnd();
             }
         }

diff --git a/test/Sentry.Tests/Ben.BlockingDetector/TaskBlockingListenerTests.cs b/test/Sentry.Tests/Ben.BlockingDetector/TaskBlockingListenerTests.cs
@@ -4,46 +4,33 @@ namespace Sentry.Tests.Ben.BlockingDetector;
 
 public class TaskBlockingListenerTests
 {
-    [Fact]
-    public void DoHandleEvent_OnTaskWaitBeginNotSuppressed_BlockingStart()
-    {
-        var monitor = Substitute.For<IBlockingMonitor>();
-        var state = Substitute.For<ITaskBlockingListenerState>();
-        state.IsSuppressed().Returns(false);
-        var listener = new TaskBlockingListener(monitor, state);
-
-        listener.DoHandleEvent(10, new List<object> { 0, 0, 0, 1 }.AsReadOnly());
-
-        state.Received(1).Recurse();
-        monitor.Received(1).BlockingStart(DetectionSource.EventListener);
-    }
-
-    [Fact]
-    public void DoHandleEvent_SuppressedOnTaskWaitBegin_BlockingSkipped()
+    [Theory]
+    [InlineData(true, 0)]
+    [InlineData(false, 1)]
+    public void DoHandleEvent_OnTaskWaitBegin_BlockingStart(bool isSuppressed, int expectedReceived)
     {
         var monitor = Substitute.For<IBlockingMonitor>();
         var state = Substitute.For<ITaskBlockingListenerState>();
-        state.IsSuppressed().Returns(true);
+        state.IsSuppressed().Returns(isSuppressed);
         var listener = new TaskBlockingListener(monitor, state);
 
         listener.DoHandleEvent(10, new List<object> { 0, 0, 0, 1 }.AsReadOnly());
 
-        monitor.DidNotReceive().BlockingStart(Arg.Any<DetectionSource>());
+        monitor.Received(expectedReceived).BlockingStart(DetectionSource.EventListener);
     }
 
     [Theory]
-    [InlineData(true, 1)]
-    [InlineData(false, 0)]
-    public void DoHandleEvent_IsRecursiveOnTaskWaitEnd_BlockingEnd(bool isRecursive, int expectedReceived)
+    [InlineData(true, 0)]
+    [InlineData(false, 1)]
+    public void DoHandleEvent_OnTaskWaitEnd_BlockingEnd(bool isSuppressed, int expectedReceived)
     {
         var monitor = Substitute.For<IBlockingMonitor>();
         var state = Substitute.For<ITaskBlockingListenerState>();
-        state.IsRecursive().Returns(isRecursive);
+        state.IsSuppressed().Returns(isSuppressed);
         var listener = new TaskBlockingListener(monitor, state);
 
         listener.DoHandleEvent(11, new List<object>().AsReadOnly());
 
-        state.Received(expectedReceived).Backtrack();
         monitor.Received(expectedReceived).BlockingEnd();
     }
 }