fix Metric livelock by replacing potential infinate loop in MetricVal…

…uesBuffer.GetAndResetValue (#2612) * initial commit * less invasive change * rename * review comment and changelog * Update CHANGELOG.md * update changelog * log warning * sort usings * fxcop
microsoft · Jun 17, 2022 · e9d4974 · e9d4974
1 parent f1ebc65
commit e9d4974
Show file tree

Hide file tree

Showing 3 changed files with 15 additions and 2 deletions.
diff --git a/...src/Microsoft.ApplicationInsights/Extensibility/Implementation/Tracing/CoreEventSource.cs b/...src/Microsoft.ApplicationInsights/Extensibility/Implementation/Tracing/CoreEventSource.cs
@@ -672,6 +672,9 @@ public void IngestionResponseTimeEventCounter(float responseDurationInMs)
         [Event(75, Message = "Ingestion Service responded with redirect. {0}", Level = EventLevel.Error)]
         public void IngestionRedirectError(string message, string appDomainName = "Incorrect") => this.WriteEvent(75, message, this.nameProvider.Name);
 
+        [Event(76, Message = "MetricValueBuffer exceeded spin count.", Level = EventLevel.Warning)]
+        public void MetricValueBufferExceededSpinCount(string appDomainName = "Incorrect") => this.WriteEvent(76, this.nameProvider.Name);
+
         [NonEvent]
         public void TransmissionStatusEventFailed(Exception ex)
         {

diff --git a/BASE/src/Microsoft.ApplicationInsights/Metrics/Extensibility/MetricValuesBuffer.cs b/BASE/src/Microsoft.ApplicationInsights/Metrics/Extensibility/MetricValuesBuffer.cs
@@ -5,6 +5,8 @@
     using System.Threading;
     using System.Threading.Tasks;
 
+    using Microsoft.ApplicationInsights.Extensibility.Implementation.Tracing;
+
 #pragma warning disable SA1649 // File name must match first type name
 #pragma warning disable SA1402 // File may only contain a single class
 
@@ -91,13 +93,19 @@ public TValue GetAndResetValue(int index)
                 while (this.IsInvalidValue(value))
                 {
                     spinWait.SpinOnce();
-                    
+
                     if (spinWait.Count % 100 == 0)
                     {
-                        // In tests (including stress tests) we always finished wating before 100 cycles.
+                        // In tests (including stress tests) we always finished waiting before 100 cycles.
                         // However, this is a protection against en extreme case on a slow machine. 
                         Task.Delay(10).ConfigureAwait(continueOnCapturedContext: false).GetAwaiter().GetResult();
                     }
+                    else if (spinWait.Count > 10000)
+                    {
+                        // Exceeded maximum spin count. Break out to avoid infinite loop.
+                        CoreEventSource.Log.MetricValueBufferExceededSpinCount();
+                        break;
+                    }
 
                     value = this.GetAndResetValueOnce(this.values, index);
                 }

diff --git a/CHANGELOG.md b/CHANGELOG.md
@@ -9,6 +9,8 @@
 - [Extension methods to retrive specific operation details.](https://github.com/microsoft/ApplicationInsights-dotnet/issues/1350)
 - [Mark Instrumentation Key based APIs as Obsolete](https://github.com/microsoft/ApplicationInsights-dotnet/issues/2560).
   - See also: https://docs.microsoft.com/azure/azure-monitor/app/migrate-from-instrumentation-keys-to-connection-strings
+- [Fix: Livelock in MetricValuesBuffer.](https://github.com/microsoft/ApplicationInsights-dotnet/pull/2612)
+  Mitigation for TelemetryClient.Flush deadlocks ([#1186](https://github.com/microsoft/ApplicationInsights-dotnet/issues/1186))
 
 ## Version 2.21.0-beta1
 - [Support IPv6 in request headers](https://github.com/microsoft/ApplicationInsights-dotnet/issues/2521)