Fix concurrent modification in MetricsCollection

src/Microsoft.TestPlatform.Common/Telemetry/MetricsCollection.cs

@@ -1,6 +1,7 @@
 // Copyright (c) Microsoft Corporation. All rights reserved.
 // Licensed under the MIT license. See LICENSE file in the project root for full license information.
 
+using System.Collections.Concurrent;
 using System.Collections.Generic;
 
 using Microsoft.VisualStudio.TestPlatform.ObjectModel.Client;
@@ -13,14 +14,14 @@ namespace Microsoft.VisualStudio.TestPlatform.Common.Telemetry;
 /// </summary>
 public class MetricsCollection : IMetricsCollection
 {
-    private readonly Dictionary<string, object> _metricDictionary;
+    private readonly ConcurrentDictionary<string, object> _metricDictionary;
 
     /// <summary>
     /// The Metrics Collection
     /// </summary>
     public MetricsCollection()
     {
-        _metricDictionary = new Dictionary<string, object>();
+        _metricDictionary = new ConcurrentDictionary<string, object>();
     }
 
     /// <summary>

test/Microsoft.TestPlatform.Common.UnitTests/Telemetry/MetricsCollectionTests.cs

@@ -57,4 +57,32 @@ public void MetricsShouldReturnEmptyDictionaryIfMetricsIsEmpty()
     {
         Assert.IsEmpty(_metricsCollection.Metrics);
     }
+
+    [TestMethod]
+    [System.Diagnostics.CodeAnalysis.SuppressMessage("Usage", "MSTEST0049", Justification = "CancellationToken not meaningful for concurrency stress test")]
+    public void AddShouldNotThrowWhenCalledConcurrently()
+    {
+        // Regression test for #15579 — concurrent Add calls on Dictionary
+        // caused InvalidOperationException: "Operations that change
+        // non-concurrent collections must have exclusive access."
+        var tasks = new System.Threading.Tasks.Task[10];
+        for (int t = 0; t < tasks.Length; t++)
+        {
+            var threadId = t;
+            tasks[t] = System.Threading.Tasks.Task.Factory.StartNew(() =>
+            {
+                for (int i = 0; i < 1000; i++)
+                {
+                    _metricsCollection.Add($"Thread{threadId}_Metric{i}", i);
+                }
+            });
+        }
+
+        foreach (var task in tasks)
+        {
+            task.GetAwaiter().GetResult();
+        }
+
+        Assert.IsGreaterThan(0, _metricsCollection.Metrics.Count);
+    }
 }

test/vstest.console.UnitTests/TestPlatformHelpers/TestRequestManagerTests.cs

@@ -2145,7 +2145,7 @@ public void ProcessTestRunAttachmentsShouldSucceedWithTelemetryEnabled()
         _mockTestPlatformEventSource.Verify(es => es.TestRunAttachmentsProcessingRequestStop());
 
         _mockMetricsPublisher.Verify(p => p.PublishMetrics(TelemetryDataConstants.TestAttachmentsProcessingCompleteEvent,
-            It.Is<Dictionary<string, object?>>(m =>
+            It.Is<IDictionary<string, object?>>(m =>
                 m.Count == 2
                 && m.ContainsKey(TelemetryDataConstants.NumberOfAttachmentsSentForProcessing)
                 && (int)m[TelemetryDataConstants.NumberOfAttachmentsSentForProcessing]! == 5
@@ -2216,7 +2216,7 @@ public async Task CancelTestRunAttachmentsProcessingShouldSucceedIfRequestInProg
         _mockTestPlatformEventSource.Verify(es => es.TestRunAttachmentsProcessingRequestStop());
 
         _mockMetricsPublisher.Verify(p => p.PublishMetrics(TelemetryDataConstants.TestAttachmentsProcessingCompleteEvent,
-            It.Is<Dictionary<string, object?>>(m =>
+            It.Is<IDictionary<string, object?>>(m =>
                 m.Count == 1
                 && m.ContainsKey(TelemetryDataConstants.AttachmentsProcessingState)
                 && (string?)m[TelemetryDataConstants.AttachmentsProcessingState] == "Canceled")));