Skip to content

Commit 9a03f2b

Browse files
committed
Address more PR feedback
- Fix flaky Cancellation_ThrowsCancellationException test
1 parent 27c0028 commit 9a03f2b

3 files changed

Lines changed: 9 additions & 18 deletions

File tree

src/ModelContextProtocol.AspNetCore/IdleTrackingBackgroundService.cs

Lines changed: 4 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -31,7 +31,8 @@ protected override async Task ExecuteAsync(CancellationToken stoppingToken)
3131
var idleTimeoutTicks = options.Value.IdleTimeout.Ticks;
3232
var maxIdleSessionCount = options.Value.MaxIdleSessionCount;
3333

34-
var idleSessions = new SortedSet<(string SessionId, long Timestamp)>(SessionTimestampComparer.Instance);
34+
// The default ValueTuple Comparer will check the first item then the second which preserves both order and uniqueness.
35+
var idleSessions = new SortedSet<(long Timestamp, string SessionId)>();
3536

3637
while (!stoppingToken.IsCancellationRequested && await timer.WaitForNextTickAsync(stoppingToken))
3738
{
@@ -55,7 +56,7 @@ protected override async Task ExecuteAsync(CancellationToken stoppingToken)
5556
continue;
5657
}
5758

58-
idleSessions.Add((session.Id, session.LastActivityTicks));
59+
idleSessions.Add((session.LastActivityTicks, session.Id));
5960

6061
// Emit critical log at most once every 5 seconds the idle count it exceeded,
6162
// since the IdleTimeout will no longer be respected.
@@ -68,7 +69,7 @@ protected override async Task ExecuteAsync(CancellationToken stoppingToken)
6869
if (idleSessions.Count > maxIdleSessionCount)
6970
{
7071
var sessionsToPrune = idleSessions.ToArray()[..^maxIdleSessionCount];
71-
foreach (var (id, _) in sessionsToPrune)
72+
foreach (var (_, id) in sessionsToPrune)
7273
{
7374
RemoveAndCloseSession(id);
7475
}
@@ -133,19 +134,6 @@ private async Task DisposeSessionAsync(HttpMcpSession<StreamableHttpServerTransp
133134
}
134135
}
135136

136-
private sealed class SessionTimestampComparer : IComparer<(string SessionId, long Timestamp)>
137-
{
138-
public static SessionTimestampComparer Instance { get; } = new();
139-
140-
public int Compare((string SessionId, long Timestamp) x, (string SessionId, long Timestamp) y) =>
141-
x.Timestamp.CompareTo(y.Timestamp) switch
142-
{
143-
// Use a SessionId comparison as tiebreaker to ensure uniqueness in the SortedSet.
144-
0 => string.CompareOrdinal(x.SessionId, y.SessionId),
145-
var timestampComparison => timestampComparison,
146-
};
147-
}
148-
149137
[LoggerMessage(Level = LogLevel.Information, Message = "Closing idle session {sessionId}.")]
150138
private partial void LogSessionIdle(string sessionId);
151139

tests/ModelContextProtocol.Tests/Client/McpClientFactoryTests.cs

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -45,7 +45,10 @@ public async Task Cancellation_ThrowsCancellationException(bool preCanceled)
4545
Task t = McpClientFactory.CreateAsync(
4646
new StreamClientTransport(new Pipe().Writer.AsStream(), new Pipe().Reader.AsStream()),
4747
cancellationToken: cts.Token);
48-
Assert.False(t.IsCompleted);
48+
if (!preCanceled)
49+
{
50+
Assert.False(t.IsCompleted);
51+
}
4952

5053
if (!preCanceled)
5154
{

tests/ModelContextProtocol.Tests/ClientServerTestBase.cs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -62,7 +62,7 @@ public async ValueTask DisposeAsync()
6262
Dispose();
6363
}
6464

65-
protected async Task<IMcpClient> CreateMcpClientForServer(McpClientOptions? options = null)
65+
protected async Task<IMcpClient> CreateMcpClientForServer()
6666
{
6767
return await McpClientFactory.CreateAsync(
6868
new StreamClientTransport(

0 commit comments

Comments
 (0)