Address more PR feedback

Author: halter73

src/ModelContextProtocol/Client/McpClient.cs

@@ -148,13 +148,18 @@ public override async ValueTask DisposeUnsynchronizedAsync()
             await _connectCts.CancelAsync().ConfigureAwait(false);
         }
 
-        await base.DisposeUnsynchronizedAsync().ConfigureAwait(false);
-
-        if (_sessionTransport is not null)
+        try
         {
-            await _sessionTransport.DisposeAsync().ConfigureAwait(false);
+            await base.DisposeUnsynchronizedAsync().ConfigureAwait(false);
         }
+        finally
+        {
+            if (_sessionTransport is not null)
+            {
+                await _sessionTransport.DisposeAsync().ConfigureAwait(false);
+            }
 
-        _connectCts?.Dispose();
+            _connectCts?.Dispose();
+        }
     }
 }

src/ModelContextProtocol/Hosting/McpServerMultiSessionHostedService.cs

@@ -9,7 +9,7 @@ namespace ModelContextProtocol.Hosting;
 /// <summary>
 /// Hosted service for a multi-session (i.e. HTTP) MCP server.
 /// </summary>
-internal class McpServerMultiSessionHostedService : BackgroundService
+internal sealed class McpServerMultiSessionHostedService : BackgroundService
 {
     private readonly IServerTransport _serverTransport;
     private readonly McpServerOptions _serverOptions;

src/ModelContextProtocol/Hosting/McpServerSingleSessionHostedService.cs

@@ -6,7 +6,7 @@ namespace ModelContextProtocol.Hosting;
 /// <summary>
 /// Hosted service for a single-session (i.e stdio) MCP server.
 /// </summary>
-internal class McpServerSingleSessionHostedService(IMcpServer session) : BackgroundService
+internal sealed class McpServerSingleSessionHostedService(IMcpServer session) : BackgroundService
 {
     /// <inheritdoc />
     protected override Task ExecuteAsync(CancellationToken stoppingToken) => session.RunAsync(stoppingToken);

src/ModelContextProtocol/Protocol/Transport/HttpListenerSseServerTransport.cs

@@ -33,7 +33,7 @@ public sealed class HttpListenerSseServerTransport : IServerTransport, IAsyncDis
     /// <param name="port">The port to listen on.</param>
     /// <param name="loggerFactory">A logger factory for creating loggers.</param>
     public HttpListenerSseServerTransport(McpServerOptions serverOptions, int port, ILoggerFactory loggerFactory)
-        : this(serverOptions?.ServerInfo?.Name!, port, loggerFactory)
+        : this(GetServerName(serverOptions), port, loggerFactory)
     {
     }
 
@@ -167,4 +167,14 @@ private async Task<bool> OnMessageAsync(Stream requestStream, CancellationToken
             return false;
         }
     }
+
+    /// <summary>Validates the <paramref name="serverOptions"/> and extracts from it the server name to use.</summary>
+    private static string GetServerName(McpServerOptions serverOptions)
+    {
+        Throw.IfNull(serverOptions);
+        Throw.IfNull(serverOptions.ServerInfo);
+        Throw.IfNull(serverOptions.ServerInfo.Name);
+
+        return serverOptions.ServerInfo.Name;
+    }
 }

src/ModelContextProtocol/Server/McpServer.cs

@@ -160,12 +160,17 @@ public override async ValueTask DisposeUnsynchronizedAsync()
             tools.Changed -= _toolsChangedDelegate;
         }
 
-        await base.DisposeUnsynchronizedAsync().ConfigureAwait(false);
-
-        if (_serverTransport is not null && _sessionTransport is not null)
+        try
         {
-            // We created the _sessionTransport from the _serverTransport, so we own it.
-            await _sessionTransport.DisposeAsync().ConfigureAwait(false);
+            await base.DisposeUnsynchronizedAsync().ConfigureAwait(false);
+        }
+        finally
+        {
+            if (_serverTransport is not null && _sessionTransport is not null)
+            {
+                // We created the _sessionTransport from the _serverTransport, so we own it.
+                await _sessionTransport.DisposeAsync().ConfigureAwait(false);
+            }
         }
     }
 

src/ModelContextProtocol/Shared/McpJsonRpcEndpoint.cs

@@ -3,6 +3,7 @@
 using ModelContextProtocol.Logging;
 using ModelContextProtocol.Protocol.Messages;
 using ModelContextProtocol.Protocol.Transport;
+using ModelContextProtocol.Utils;
 using System.Diagnostics.CodeAnalysis;
 
 namespace ModelContextProtocol.Shared;
@@ -23,7 +24,7 @@ internal abstract class McpJsonRpcEndpoint : IAsyncDisposable
     private CancellationTokenSource? _sessionCts;
     private int _started;
 
-    private readonly SemaphoreSlim _disposeLock = new(1);
+    private readonly SemaphoreSlim _disposeLock = new(1, 1);
     private bool _disposed;
 
     protected readonly ILogger _logger;
@@ -78,21 +79,15 @@ protected void StartSession(CancellationToken fullSessionCancellationToken = def
 
     public async ValueTask DisposeAsync()
     {
-        await _disposeLock.WaitAsync().ConfigureAwait(false);
-        try
-        {
-            if (_disposed)
-            {
-                return;
-            }
-            _disposed = true;
+        using var _ = await _disposeLock.LockAsync().ConfigureAwait(false);
 
-            await DisposeUnsynchronizedAsync().ConfigureAwait(false);
-        }
-        finally
+        if (_disposed)
         {
-            _disposeLock.Release();
+            return;
         }
+        _disposed = true;
+
+        await DisposeUnsynchronizedAsync().ConfigureAwait(false);
     }
 
     /// <summary>
@@ -126,6 +121,6 @@ public virtual async ValueTask DisposeUnsynchronizedAsync()
         _logger.EndpointCleanedUp(EndpointName);
     }
 
-    protected McpSession GetSessionOrThrow() =>
-        _session ?? throw new InvalidOperationException($"This should be unreachable from public API! Call {nameof(InitializeSession)} before sending messages.");
+    protected McpSession GetSessionOrThrow()
+        => _session ?? throw new InvalidOperationException($"This should be unreachable from public API! Call {nameof(InitializeSession)} before sending messages.");
 }