Skip to content

Commit e51f89f

Browse files
Copilotstephentoub
andcommitted
Fix flaky DiagnosticTests.Session_TracksActivities: use thread-safe collection for activities
The InMemoryExporter calls List.Add on server fire-and-forget threads while WaitForAsync polls via enumeration on the test thread. Replace List<Activity> with a synchronized ICollection wrapper that returns snapshot enumerators. Co-authored-by: stephentoub <2642209+stephentoub@users.noreply.github.com>
1 parent 93eec5c commit e51f89f

1 file changed

Lines changed: 28 additions & 3 deletions

File tree

tests/ModelContextProtocol.Tests/DiagnosticTests.cs

Lines changed: 28 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -2,6 +2,7 @@
22
using ModelContextProtocol.Protocol;
33
using ModelContextProtocol.Server;
44
using OpenTelemetry.Trace;
5+
using System.Collections;
56
using System.Diagnostics;
67
using System.IO.Pipelines;
78
using System.Text;
@@ -15,7 +16,10 @@ public class DiagnosticTests
1516
[Fact]
1617
public async Task Session_TracksActivities()
1718
{
18-
var activities = new List<Activity>();
19+
// Use a thread-safe collection to avoid race conditions between
20+
// the WaitForAsync polling (reads) and fire-and-forget server tasks
21+
// exporting activities via the InMemoryExporter (writes).
22+
var activities = new SynchronizedList<Activity>();
1923
var clientToServerLog = new List<string>();
2024

2125
using (var tracerProvider = OpenTelemetry.Sdk.CreateTracerProviderBuilder()
@@ -92,7 +96,7 @@ await WaitForAsync(() => activities.Any(a =>
9296
[Fact]
9397
public async Task Session_FailedToolCall()
9498
{
95-
var activities = new List<Activity>();
99+
var activities = new SynchronizedList<Activity>();
96100

97101
using (var tracerProvider = OpenTelemetry.Sdk.CreateTracerProviderBuilder()
98102
.AddSource("Experimental.ModelContextProtocol")
@@ -156,7 +160,7 @@ public async Task Session_McpAttributesAddedToOuterExecuteToolActivity()
156160
// "execute_tool" activity, and MCP should add its attributes to that activity instead
157161
// of creating a new one.
158162
string outerSourceName = "TestOuterSource";
159-
var activities = new List<Activity>();
163+
var activities = new SynchronizedList<Activity>();
160164

161165
using var outerSource = new ActivitySource(outerSourceName);
162166

@@ -234,6 +238,27 @@ private static async Task RunConnected(Func<McpClient, McpServer, Task> action,
234238
await serverTask;
235239
}
236240

241+
/// <summary>
242+
/// A thread-safe ICollection wrapper. The InMemoryExporter calls Add on server
243+
/// fire-and-forget threads while WaitForAsync polls via enumeration on the test thread.
244+
/// GetEnumerator returns a snapshot to avoid concurrent modification issues.
245+
/// </summary>
246+
private sealed class SynchronizedList<T> : ICollection<T>
247+
{
248+
private readonly List<T> _list = [];
249+
private readonly object _lock = new();
250+
251+
public int Count { get { lock (_lock) return _list.Count; } }
252+
public bool IsReadOnly => false;
253+
public void Add(T item) { lock (_lock) _list.Add(item); }
254+
public void Clear() { lock (_lock) _list.Clear(); }
255+
public bool Contains(T item) { lock (_lock) return _list.Contains(item); }
256+
public void CopyTo(T[] array, int arrayIndex) { lock (_lock) _list.CopyTo(array, arrayIndex); }
257+
public bool Remove(T item) { lock (_lock) return _list.Remove(item); }
258+
public IEnumerator<T> GetEnumerator() { lock (_lock) return _list.ToList().GetEnumerator(); }
259+
IEnumerator IEnumerable.GetEnumerator() => GetEnumerator();
260+
}
261+
237262
private static async Task WaitForAsync(Func<bool> condition, int timeoutMs = 10_000)
238263
{
239264
using var cts = new CancellationTokenSource(timeoutMs);

0 commit comments

Comments
 (0)