Skip to content

Commit bde2e3e

Browse files
committed
refactor(server): generalize lifetime registrations
Signed-off-by: King Star <mcxin.y@gmail.com>
1 parent 456f064 commit bde2e3e

5 files changed

Lines changed: 202 additions & 54 deletions

File tree

src/ModelContextProtocol.Core/Server/DestinationBoundMcpServer.cs

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -73,11 +73,11 @@ public override Implementation? ClientInfo
7373

7474
public override bool IsMrtrSupported => server.IsMrtrSupported;
7575

76-
CancellationToken IMcpServerLifetimeFeature.BackgroundTaskCancellationToken =>
77-
((IMcpServerLifetimeFeature)server).BackgroundTaskCancellationToken;
76+
CancellationToken IMcpServerLifetimeFeature.ServerCancellationToken =>
77+
((IMcpServerLifetimeFeature)server).ServerCancellationToken;
7878

79-
void IMcpServerLifetimeFeature.RegisterBackgroundTask(Task backgroundTask) =>
80-
((IMcpServerLifetimeFeature)server).RegisterBackgroundTask(backgroundTask);
79+
IDisposable IMcpServerLifetimeFeature.RegisterForDisposeAsync(IAsyncDisposable disposable) =>
80+
((IMcpServerLifetimeFeature)server).RegisterForDisposeAsync(disposable);
8181

8282
public override ValueTask DisposeAsync() => server.DisposeAsync();
8383

src/ModelContextProtocol.Core/Server/IMcpServerLifetimeFeature.cs

Lines changed: 12 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -8,15 +8,19 @@ namespace ModelContextProtocol.Server;
88
[EditorBrowsable(EditorBrowsableState.Never)]
99
public interface IMcpServerLifetimeFeature
1010
{
11-
/// <summary>Gets the token that should cancel background work owned by this server.</summary>
11+
/// <summary>Gets the token that is cancelled when this server starts disposing.</summary>
1212
/// <remarks>
13-
/// The token is <see cref="CancellationToken.None"/> when background work intentionally outlives
14-
/// the server instance, as it does for per-request servers in stateless HTTP mode.
13+
/// The token is <see cref="CancellationToken.None"/> when work intentionally outlives the server,
14+
/// as it does for per-request servers in stateless HTTP mode.
1515
/// </remarks>
16-
CancellationToken BackgroundTaskCancellationToken { get; }
16+
CancellationToken ServerCancellationToken { get; }
1717

18-
/// <summary>Registers background work that server disposal must await.</summary>
19-
/// <param name="backgroundTask">The background work to track.</param>
20-
/// <remarks>This is a no-op when background work intentionally outlives the server instance.</remarks>
21-
void RegisterBackgroundTask(Task backgroundTask);
18+
/// <summary>Registers an asynchronously disposable resource that server disposal must await.</summary>
19+
/// <param name="disposable">The resource to dispose when this server is disposed.</param>
20+
/// <returns>A handle that unregisters the resource without disposing it.</returns>
21+
/// <remarks>
22+
/// Dispose the returned handle when the resource completes independently so the server does not
23+
/// retain it until shutdown. Registration is a no-op when the server does not own the resource.
24+
/// </remarks>
25+
IDisposable RegisterForDisposeAsync(IAsyncDisposable disposable);
2226
}

src/ModelContextProtocol.Core/Server/McpServerImpl.cs

Lines changed: 51 additions & 22 deletions
Original file line numberDiff line numberDiff line change
@@ -32,8 +32,8 @@ internal sealed partial class McpServerImpl : McpServer, IMcpServerLifetimeFeatu
3232
private readonly string[] _perRequestMetadataProtocolVersions;
3333
private readonly SemaphoreSlim _disposeLock = new(1, 1);
3434
private readonly CancellationTokenSource _serverLifetimeCts = new();
35-
private readonly object _backgroundTasksLock = new();
36-
private readonly ConcurrentDictionary<Task, byte> _backgroundTasks = new();
35+
private readonly object _serverLifetimeRegistrationsLock = new();
36+
private readonly HashSet<ServerLifetimeRegistration> _serverLifetimeRegistrations = [];
3737
private readonly ConcurrentDictionary<string, MrtrContinuation> _mrtrContinuations = new();
3838
private readonly ConcurrentDictionary<RequestId, MrtrContext> _mrtrContextsByRequestId = new();
3939
private static readonly string[] s_perRequestMetadataKeys =
@@ -58,7 +58,7 @@ internal sealed partial class McpServerImpl : McpServer, IMcpServerLifetimeFeatu
5858
private int _started;
5959

6060
private bool _disposed;
61-
private bool _backgroundTaskRegistrationClosed;
61+
private bool _serverLifetimeRegistrationClosed;
6262

6363
/// <summary>Holds a boxed <see cref="LoggingLevel"/> value for the server.</summary>
6464
/// <remarks>
@@ -508,36 +508,40 @@ public override Task SendMessageAsync(JsonRpcMessage message, CancellationToken
508508
public override IAsyncDisposable RegisterNotificationHandler(string method, Func<JsonRpcNotification, CancellationToken, ValueTask> handler)
509509
=> _sessionHandler.RegisterNotificationHandler(method, handler);
510510

511-
CancellationToken IMcpServerLifetimeFeature.BackgroundTaskCancellationToken =>
511+
CancellationToken IMcpServerLifetimeFeature.ServerCancellationToken =>
512512
HasStatefulTransport() ? _serverLifetimeCts.Token : CancellationToken.None;
513513

514-
void IMcpServerLifetimeFeature.RegisterBackgroundTask(Task backgroundTask)
514+
IDisposable IMcpServerLifetimeFeature.RegisterForDisposeAsync(IAsyncDisposable disposable)
515515
{
516-
Throw.IfNull(backgroundTask);
516+
Throw.IfNull(disposable);
517517

518518
// Stateless HTTP servers are request-scoped, while Tasks runners intentionally outlive
519519
// the originating request and are governed by tasks/cancel and task-store retention.
520520
if (!HasStatefulTransport())
521521
{
522-
return;
522+
return NoopRegistration.Instance;
523523
}
524524

525-
lock (_backgroundTasksLock)
525+
var registration = new ServerLifetimeRegistration(this, disposable);
526+
lock (_serverLifetimeRegistrationsLock)
526527
{
527-
if (_backgroundTaskRegistrationClosed)
528+
if (_serverLifetimeRegistrationClosed)
528529
{
529530
throw new ObjectDisposedException(nameof(McpServer));
530531
}
531532

532-
_backgroundTasks.TryAdd(backgroundTask, 0);
533+
_serverLifetimeRegistrations.Add(registration);
533534
}
534535

535-
_ = backgroundTask.ContinueWith(
536-
static (task, state) => ((ConcurrentDictionary<Task, byte>)state!).TryRemove(task, out _),
537-
_backgroundTasks,
538-
CancellationToken.None,
539-
TaskContinuationOptions.ExecuteSynchronously,
540-
TaskScheduler.Default);
536+
return registration;
537+
}
538+
539+
private void UnregisterServerLifetime(ServerLifetimeRegistration registration)
540+
{
541+
lock (_serverLifetimeRegistrationsLock)
542+
{
543+
_serverLifetimeRegistrations.Remove(registration);
544+
}
541545
}
542546

543547
/// <inheritdoc/>
@@ -560,11 +564,11 @@ public override async ValueTask DisposeAsync()
560564
_disposables.ForEach(d => d());
561565
await _sessionHandler.DisposeAsync().ConfigureAwait(false);
562566

563-
Task[] backgroundTasks;
564-
lock (_backgroundTasksLock)
567+
ServerLifetimeRegistration[] serverLifetimeRegistrations;
568+
lock (_serverLifetimeRegistrationsLock)
565569
{
566-
_backgroundTaskRegistrationClosed = true;
567-
backgroundTasks = [.. _backgroundTasks.Keys];
570+
_serverLifetimeRegistrationClosed = true;
571+
serverLifetimeRegistrations = [.. _serverLifetimeRegistrations];
568572
}
569573

570574
// Cancel all orphaned MRTR handlers still suspended in continuations (waiting for
@@ -589,9 +593,34 @@ public override async ValueTask DisposeAsync()
589593
await _allMrtrHandlersCompleted.Task.ConfigureAwait(false);
590594
}
591595

592-
if (backgroundTasks.Length > 0)
596+
if (serverLifetimeRegistrations.Length > 0)
597+
{
598+
await Task.WhenAll(
599+
serverLifetimeRegistrations.Select(static registration => registration.DisposeResourceAsync().AsTask())
600+
).ConfigureAwait(false);
601+
}
602+
}
603+
604+
private sealed class ServerLifetimeRegistration(
605+
McpServerImpl server,
606+
IAsyncDisposable resource) : IDisposable
607+
{
608+
private McpServerImpl? _server = server;
609+
610+
public ValueTask DisposeResourceAsync() => resource.DisposeAsync();
611+
612+
public void Dispose()
613+
{
614+
Interlocked.Exchange(ref _server, null)?.UnregisterServerLifetime(this);
615+
}
616+
}
617+
618+
private sealed class NoopRegistration : IDisposable
619+
{
620+
public static NoopRegistration Instance { get; } = new();
621+
622+
public void Dispose()
593623
{
594-
await Task.WhenAll(backgroundTasks).ConfigureAwait(false);
595624
}
596625
}
597626

src/ModelContextProtocol.Extensions.Tasks/Server/McpTasksBuilderExtensions.cs

Lines changed: 52 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -148,14 +148,25 @@ private async ValueTask<ResultOrAlternate<CallToolResult>> RunAsTaskAsync(
148148
executionRequest.Server = request.Server.WithMcpTaskOutgoingRequestInterceptor(taskId, _store);
149149
var serverLifetime = request.Server as IMcpServerLifetimeFeature;
150150
var cancellationState = new TaskCancellationState(
151-
serverLifetime?.BackgroundTaskCancellationToken ?? CancellationToken.None);
151+
serverLifetime?.ServerCancellationToken ?? CancellationToken.None);
152152
_cancellationStates[taskId] = cancellationState;
153153

154154
var taskCancellationToken = cancellationState.Token;
155155
var backgroundTask = Task.Run(
156156
() => ExecuteTaskAsync(next, executionRequest, taskId, taskCancellationToken, executionScope),
157157
CancellationToken.None);
158-
serverLifetime?.RegisterBackgroundTask(backgroundTask);
158+
cancellationState.SetBackgroundTask(backgroundTask);
159+
try
160+
{
161+
cancellationState.SetServerLifetimeRegistration(
162+
serverLifetime?.RegisterForDisposeAsync(cancellationState));
163+
}
164+
catch
165+
{
166+
cancellationState.Cancel();
167+
await backgroundTask.ConfigureAwait(false);
168+
throw;
169+
}
159170

160171
return ResultOrAlternate<CallToolResult>.FromAlternate(
161172
ToCreateTaskResult(taskInfo),
@@ -326,27 +337,63 @@ private async Task ExecuteToolPipelineAsync(
326337
return JsonSerializer.SerializeToNode(new CancelTaskResult(), McpTasksJsonContext.Default.CancelTaskResult);
327338
}
328339

329-
private sealed class TaskCancellationState
340+
private sealed class TaskCancellationState : IAsyncDisposable
330341
{
331342
private readonly CancellationTokenSource _source = new();
332343
private readonly CancellationTokenRegistration _serverLifetimeRegistration;
344+
private Task? _backgroundTask;
345+
private IDisposable? _serverLifetimeUnregistration;
346+
private int _completed;
333347

334348
public TaskCancellationState(CancellationToken serverLifetimeToken)
335349
{
336350
_serverLifetimeRegistration = serverLifetimeToken.Register(
337-
static state => ((CancellationTokenSource)state!).Cancel(),
338-
_source);
351+
static state => ((TaskCancellationState)state!).Cancel(),
352+
this);
339353
}
340354

341355
public CancellationToken Token => _source.Token;
342356

343357
public void Cancel() => _source.Cancel();
344358

359+
public void SetBackgroundTask(Task backgroundTask) => _backgroundTask = backgroundTask;
360+
361+
public void SetServerLifetimeRegistration(IDisposable? registration)
362+
{
363+
if (registration is null)
364+
{
365+
return;
366+
}
367+
368+
if (Volatile.Read(ref _completed) != 0)
369+
{
370+
registration.Dispose();
371+
return;
372+
}
373+
374+
Interlocked.CompareExchange(ref _serverLifetimeUnregistration, registration, null);
375+
if (Volatile.Read(ref _completed) != 0)
376+
{
377+
Interlocked.Exchange(ref _serverLifetimeUnregistration, null)?.Dispose();
378+
}
379+
}
380+
345381
public void UnregisterServerLifetime()
346382
{
347383
// Cancellation can arrive concurrently from tasks/cancel and server disposal.
348384
// Once the dictionary entry and server registration are gone, the CTS is collectible.
385+
Interlocked.Exchange(ref _completed, 1);
349386
_serverLifetimeRegistration.Dispose();
387+
Interlocked.Exchange(ref _serverLifetimeUnregistration, null)?.Dispose();
388+
}
389+
390+
public async ValueTask DisposeAsync()
391+
{
392+
Cancel();
393+
if (_backgroundTask is { } backgroundTask)
394+
{
395+
await backgroundTask.ConfigureAwait(false);
396+
}
350397
}
351398
}
352399

tests/ModelContextProtocol.Tests/Server/TaskCancellationIntegrationTests.cs

Lines changed: 83 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -179,6 +179,45 @@ protected override void ConfigureServices(ServiceCollection services, IMcpServer
179179
})]);
180180
}
181181

182+
[Fact]
183+
public async Task DisposeAsync_DisposesAndWaitsForRegisteredLifetimeResource()
184+
{
185+
await using var client = await CreateMcpClientForServer();
186+
var ct = TestContext.Current.CancellationToken;
187+
var serverLifetime = Assert.IsAssignableFrom<IMcpServerLifetimeFeature>(Server);
188+
var resource = new BlockingAsyncDisposable();
189+
using var registration = serverLifetime.RegisterForDisposeAsync(resource);
190+
191+
Task disposeTask = Server.DisposeAsync().AsTask();
192+
193+
try
194+
{
195+
await resource.DisposeStarted.WaitAsync(TestConstants.DefaultTimeout, ct);
196+
Assert.False(disposeTask.IsCompleted, "DisposeAsync should await the registered resource.");
197+
198+
resource.Release();
199+
await disposeTask.WaitAsync(TestConstants.DefaultTimeout, ct);
200+
}
201+
finally
202+
{
203+
resource.Release();
204+
}
205+
}
206+
207+
[Fact]
208+
public async Task LifetimeRegistration_DisposeUnregistersResource()
209+
{
210+
await using var client = await CreateMcpClientForServer();
211+
var serverLifetime = Assert.IsAssignableFrom<IMcpServerLifetimeFeature>(Server);
212+
var resource = new RecordingAsyncDisposable();
213+
using var registration = serverLifetime.RegisterForDisposeAsync(resource);
214+
215+
registration.Dispose();
216+
await Server.DisposeAsync();
217+
218+
Assert.False(resource.IsDisposed);
219+
}
220+
182221
[Fact]
183222
public async Task DisposeAsync_CancelsAndWaitsForTaskStoreRunner()
184223
{
@@ -230,7 +269,7 @@ public async Task DisposeAsync_CancelsAndWaitsForRunnerRegisteredDuringDisposal(
230269

231270
var serverLifetime = Assert.IsAssignableFrom<IMcpServerLifetimeFeature>(Server);
232271
var serverCancellationFired = new TaskCompletionSource<bool>(TaskCreationOptions.RunContinuationsAsynchronously);
233-
using var registration = serverLifetime.BackgroundTaskCancellationToken.Register(
272+
using var registration = serverLifetime.ServerCancellationToken.Register(
234273
static state => ((TaskCompletionSource<bool>)state!).TrySetResult(true), serverCancellationFired);
235274

236275
Task disposeTask = Server.DisposeAsync().AsTask();
@@ -283,6 +322,33 @@ async Task<bool> IMcpTaskStore.SetCancelledAsync(string taskId, CancellationToke
283322
return await base.SetCancelledAsync(taskId, cancellationToken);
284323
}
285324
}
325+
326+
private sealed class BlockingAsyncDisposable : IAsyncDisposable
327+
{
328+
private readonly TaskCompletionSource<bool> _disposeStarted = new(TaskCreationOptions.RunContinuationsAsynchronously);
329+
private readonly TaskCompletionSource<bool> _release = new(TaskCreationOptions.RunContinuationsAsynchronously);
330+
331+
public Task DisposeStarted => _disposeStarted.Task;
332+
333+
public void Release() => _release.TrySetResult(true);
334+
335+
public async ValueTask DisposeAsync()
336+
{
337+
_disposeStarted.TrySetResult(true);
338+
await _release.Task;
339+
}
340+
}
341+
342+
private sealed class RecordingAsyncDisposable : IAsyncDisposable
343+
{
344+
public bool IsDisposed { get; private set; }
345+
346+
public ValueTask DisposeAsync()
347+
{
348+
IsDisposed = true;
349+
return default;
350+
}
351+
}
286352
}
287353

288354
public class McpServerLifetimeFeatureTests(ITestOutputHelper testOutputHelper) : LoggedTest(testOutputHelper)
@@ -299,24 +365,26 @@ public async Task DisposeAsync_DoesNotCancelOrWaitForStatelessBackgroundTask()
299365
},
300366
LoggerFactory);
301367
var serverLifetime = Assert.IsAssignableFrom<IMcpServerLifetimeFeature>(statelessServer);
302-
var releaseBackgroundTask = new TaskCompletionSource<bool>(TaskCreationOptions.RunContinuationsAsynchronously);
303-
Task backgroundTask = releaseBackgroundTask.Task;
368+
var backgroundResource = new RecordingAsyncDisposable();
304369

305-
serverLifetime.RegisterBackgroundTask(backgroundTask);
370+
using var registration = serverLifetime.RegisterForDisposeAsync(backgroundResource);
306371

307-
try
308-
{
309-
await statelessServer.DisposeAsync().AsTask()
310-
.WaitAsync(TestConstants.DefaultTimeout, TestContext.Current.CancellationToken);
372+
await statelessServer.DisposeAsync().AsTask()
373+
.WaitAsync(TestConstants.DefaultTimeout, TestContext.Current.CancellationToken);
311374

312-
Assert.False(serverLifetime.BackgroundTaskCancellationToken.CanBeCanceled);
313-
Assert.False(backgroundTask.IsCompleted,
314-
"A stateless per-request server should not own background work that outlives the request.");
315-
}
316-
finally
375+
Assert.False(serverLifetime.ServerCancellationToken.CanBeCanceled);
376+
Assert.False(backgroundResource.IsDisposed,
377+
"A stateless per-request server should not own background work that outlives the request.");
378+
}
379+
380+
private sealed class RecordingAsyncDisposable : IAsyncDisposable
381+
{
382+
public bool IsDisposed { get; private set; }
383+
384+
public ValueTask DisposeAsync()
317385
{
318-
releaseBackgroundTask.TrySetResult(true);
319-
await backgroundTask;
386+
IsDisposed = true;
387+
return default;
320388
}
321389
}
322390
}

0 commit comments

Comments
 (0)