Skip to content

Commit df923a2

Browse files
committed
Fix macOS CI hang and flaky idle session pruning test
On macOS, the createdump tool hangs indefinitely due to ptrace/SIP restrictions when invoked by grandchild .NET processes that inherit DOTNET_DbgEnableMiniDump=1 from dotnet test --blame-crash. - Suppress mini-dump generation on macOS only for conformance child processes (the runtime still prints crash diagnostics to stderr) - Add 5-minute process timeout to conformance test runners as a safety net against any future hangs - Fix race in IdleSessions_ArePruned_AfterIdleTimeout where the background pruning task hadn't completed before the assertion
1 parent bbc6c3d commit df923a2

File tree

5 files changed

+86
-7
lines changed

5 files changed

+86
-7
lines changed

src/ModelContextProtocol.Core/Client/StreamableHttpClientSessionTransport.cs

Lines changed: 27 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,6 @@
11
using Microsoft.Extensions.Logging;
22
using Microsoft.Extensions.Logging.Abstractions;
3+
using System.Diagnostics;
34
using System.Net.Http.Headers;
45
using System.Net.ServerSentEvents;
56
using System.Text.Json;
@@ -239,7 +240,19 @@ await SendGetSseRequestWithRetriesAsync(
239240
if (shouldDelay)
240241
{
241242
var delay = state.RetryInterval ?? _options.DefaultReconnectionInterval;
242-
await Task.Delay(delay, cancellationToken).ConfigureAwait(false);
243+
244+
// Subtract time already elapsed since the SSE stream ended to more accurately
245+
// honor the retry interval. Without this, processing overhead (HTTP response
246+
// disposal, condition checks, etc.) inflates the observed reconnection delay.
247+
if (state.StreamEndedTimestamp != 0)
248+
{
249+
delay -= ElapsedSince(state.StreamEndedTimestamp);
250+
}
251+
252+
if (delay > TimeSpan.Zero)
253+
{
254+
await Task.Delay(delay, cancellationToken).ConfigureAwait(false);
255+
}
243256
}
244257
shouldDelay = true;
245258

@@ -336,9 +349,11 @@ private async Task<SseResponse> ProcessSseResponseAsync(
336349
}
337350
catch (Exception ex) when (ex is IOException or HttpRequestException)
338351
{
352+
state.StreamEndedTimestamp = Stopwatch.GetTimestamp();
339353
return new() { IsNetworkError = true };
340354
}
341355

356+
state.StreamEndedTimestamp = Stopwatch.GetTimestamp();
342357
return default;
343358
}
344359

@@ -435,6 +450,8 @@ private sealed class SseStreamState
435450
{
436451
public string? LastEventId { get; set; }
437452
public TimeSpan? RetryInterval { get; set; }
453+
/// <summary>Timestamp (via Stopwatch.GetTimestamp()) when the last SSE stream ended, used to discount processing overhead from the retry delay.</summary>
454+
public long StreamEndedTimestamp { get; set; }
438455
}
439456

440457
/// <summary>
@@ -445,4 +462,13 @@ private readonly struct SseResponse
445462
public JsonRpcMessageWithId? Response { get; init; }
446463
public bool IsNetworkError { get; init; }
447464
}
465+
466+
private static TimeSpan ElapsedSince(long stopwatchTimestamp)
467+
{
468+
#if NET
469+
return Stopwatch.GetElapsedTime(stopwatchTimestamp);
470+
#else
471+
return TimeSpan.FromSeconds((double)(Stopwatch.GetTimestamp() - stopwatchTimestamp) / Stopwatch.Frequency);
472+
#endif
473+
}
448474
}

tests/Common/Utils/NodeHelpers.cs

Lines changed: 18 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -83,10 +83,11 @@ public static ProcessStartInfo ConformanceTestStartInfo(string arguments)
8383
var repoRoot = FindRepoRoot();
8484
var binPath = Path.Combine(repoRoot, "node_modules", ".bin", "conformance");
8585

86+
ProcessStartInfo startInfo;
8687
if (RuntimeInformation.IsOSPlatform(OSPlatform.Windows))
8788
{
8889
// On Windows, node_modules/.bin contains .cmd shims that can be executed directly
89-
return new ProcessStartInfo
90+
startInfo = new ProcessStartInfo
9091
{
9192
FileName = $"{binPath}.cmd",
9293
Arguments = arguments,
@@ -98,7 +99,7 @@ public static ProcessStartInfo ConformanceTestStartInfo(string arguments)
9899
}
99100
else
100101
{
101-
return new ProcessStartInfo
102+
startInfo = new ProcessStartInfo
102103
{
103104
FileName = binPath,
104105
Arguments = arguments,
@@ -108,6 +109,21 @@ public static ProcessStartInfo ConformanceTestStartInfo(string arguments)
108109
CreateNoWindow = true
109110
};
110111
}
112+
113+
// On macOS, disable .NET mini-dump file generation for child processes. When
114+
// dotnet test runs with --blame-crash, it sets DOTNET_DbgEnableMiniDump=1 in the
115+
// environment. This is inherited by grandchild .NET processes (e.g. ConformanceClient
116+
// launched via node). On macOS, the createdump tool can hang indefinitely due to
117+
// ptrace/SIP restrictions, causing the entire test run to hang. Disabling mini-dumps
118+
// only suppresses the dump file creation; the runtime still prints crash diagnostics
119+
// (stack traces, signal info, etc.) to stderr, which the test captures.
120+
if (RuntimeInformation.IsOSPlatform(OSPlatform.OSX))
121+
{
122+
startInfo.Environment["DOTNET_DbgEnableMiniDump"] = "0";
123+
startInfo.Environment["COMPlus_DbgEnableMiniDump"] = "0";
124+
}
125+
126+
return startInfo;
111127
}
112128

113129
/// <summary>

tests/ModelContextProtocol.AspNetCore.Tests/ClientConformanceTests.cs

Lines changed: 14 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -103,7 +103,20 @@ public async Task RunConformanceTest(string scenario)
103103
process.BeginOutputReadLine();
104104
process.BeginErrorReadLine();
105105

106-
await process.WaitForExitAsync();
106+
using var cts = new CancellationTokenSource(TimeSpan.FromMinutes(5));
107+
try
108+
{
109+
await process.WaitForExitAsync(cts.Token);
110+
}
111+
catch (OperationCanceledException)
112+
{
113+
process.Kill(entireProcessTree: true);
114+
return (
115+
Success: false,
116+
Output: outputBuilder.ToString(),
117+
Error: errorBuilder.ToString() + "\nProcess timed out after 5 minutes and was killed."
118+
);
119+
}
107120

108121
return (
109122
Success: process.ExitCode == 0,

tests/ModelContextProtocol.AspNetCore.Tests/ServerConformanceTests.cs

Lines changed: 14 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -167,7 +167,20 @@ private void StartConformanceServer()
167167
process.BeginOutputReadLine();
168168
process.BeginErrorReadLine();
169169

170-
await process.WaitForExitAsync();
170+
using var cts = new CancellationTokenSource(TimeSpan.FromMinutes(5));
171+
try
172+
{
173+
await process.WaitForExitAsync(cts.Token);
174+
}
175+
catch (OperationCanceledException)
176+
{
177+
process.Kill(entireProcessTree: true);
178+
return (
179+
Success: false,
180+
Output: outputBuilder.ToString(),
181+
Error: errorBuilder.ToString() + "\nProcess timed out after 5 minutes and was killed."
182+
);
183+
}
171184

172185
return (
173186
Success: process.ExitCode == 0,

tests/ModelContextProtocol.AspNetCore.Tests/StreamableHttpServerConformanceTests.cs

Lines changed: 13 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -473,8 +473,19 @@ public async Task IdleSessions_ArePruned_AfterIdleTimeout()
473473
// Add 5 seconds to idle timeout to account for the interval of the PeriodicTimer.
474474
fakeTimeProvider.Advance(TimeSpan.FromHours(2) + TimeSpan.FromSeconds(5));
475475

476-
using var response = await HttpClient.PostAsync("", JsonContent(EchoRequest), TestContext.Current.CancellationToken);
477-
Assert.Equal(HttpStatusCode.NotFound, response.StatusCode);
476+
// The background IdleTrackingBackgroundService prunes sessions asynchronously after
477+
// the timer tick fires. Poll until pruning completes and the session returns NotFound.
478+
var deadline = DateTime.UtcNow + TestConstants.DefaultTimeout;
479+
HttpStatusCode statusCode;
480+
do
481+
{
482+
await Task.Delay(100, TestContext.Current.CancellationToken);
483+
using var response = await HttpClient.PostAsync("", JsonContent(EchoRequest), TestContext.Current.CancellationToken);
484+
statusCode = response.StatusCode;
485+
}
486+
while (statusCode != HttpStatusCode.NotFound && DateTime.UtcNow < deadline);
487+
488+
Assert.Equal(HttpStatusCode.NotFound, statusCode);
478489
}
479490

480491
[Fact]

0 commit comments

Comments
 (0)