fix(server): prevent devenv.exe from hanging on shutdown

.github/workflows/build.yml

@@ -19,4 +19,5 @@ jobs:
     uses: CodingWithCalvin/.github/.github/workflows/vsix-build.yml@main
     with:
       extension-name: MCPServer
+      test-project: src/CodingWithCalvin.MCPServer.Tests/CodingWithCalvin.MCPServer.Tests.csproj
     secrets: inherit

src/CodingWithCalvin.MCPServer.Tests/CodingWithCalvin.MCPServer.Tests.csproj

@@ -0,0 +1,29 @@
+<Project Sdk="Microsoft.NET.Sdk">
+
+  <PropertyGroup>
+    <TargetFramework>net48</TargetFramework>
+    <LangVersion>latest</LangVersion>
+    <Nullable>enable</Nullable>
+    <IsPackable>false</IsPackable>
+    <RootNamespace>CodingWithCalvin.MCPServer.Tests</RootNamespace>
+    <!--
+      These tests deliberately block on tasks: reproducing the issue #97 deadlock requires
+      driving the shutdown path exactly the way package disposal does.
+    -->
+    <NoWarn>$(NoWarn);VSTHRD002;VSTHRD103</NoWarn>
+  </PropertyGroup>
+
+  <ItemGroup>
+    <PackageReference Include="Microsoft.NET.Test.Sdk" Version="17.11.1" />
+    <PackageReference Include="xunit" Version="2.9.2" />
+    <PackageReference Include="xunit.runner.visualstudio" Version="2.8.2">
+      <IncludeAssets>runtime; build; native; contentfiles; analyzers; buildtransitive</IncludeAssets>
+      <PrivateAssets>all</PrivateAssets>
+    </PackageReference>
+  </ItemGroup>
+
+  <ItemGroup>
+    <ProjectReference Include="..\CodingWithCalvin.MCPServer\CodingWithCalvin.MCPServer.csproj" />
+  </ItemGroup>
+
+</Project>

src/CodingWithCalvin.MCPServer.Tests/ProcessJobObjectTests.cs

@@ -0,0 +1,122 @@
+using System;
+using System.Diagnostics;
+using CodingWithCalvin.MCPServer.Services;
+using Xunit;
+
+namespace CodingWithCalvin.MCPServer.Tests;
+
+/// <summary>
+/// Covers the kill-on-close backstop that stops the MCP server process from outliving Visual
+/// Studio when devenv.exe terminates without running its normal shutdown path.
+/// </summary>
+public class ProcessJobObjectTests
+{
+    private const int ExitWaitMs = 5000;
+
+    [Fact]
+    public void Create_ReturnsJobObject()
+    {
+        using var job = ProcessJobObject.Create();
+
+        Assert.NotNull(job);
+    }
+
+    [Fact]
+    public void Dispose_TerminatesAssignedProcess()
+    {
+        var process = StartLongRunningProcess();
+
+        try
+        {
+            var job = ProcessJobObject.Create();
+            Assert.NotNull(job);
+
+            Assert.True(job!.TryAssign(process), "Failed to assign the process to the job object.");
+            Assert.False(process.HasExited, "The child process exited before the job was closed.");
+
+            job.Dispose();
+
+            Assert.True(
+                process.WaitForExit(ExitWaitMs),
+                "Closing the job object did not terminate the assigned process.");
+        }
+        finally
+        {
+            KillIfRunning(process);
+            process.Dispose();
+        }
+    }
+
+    [Fact]
+    public void TryAssign_ReturnsFalse_AfterDispose()
+    {
+        var job = ProcessJobObject.Create();
+        Assert.NotNull(job);
+        job!.Dispose();
+
+        var process = StartLongRunningProcess();
+
+        try
+        {
+            Assert.False(job.TryAssign(process));
+        }
+        finally
+        {
+            KillIfRunning(process);
+            process.Dispose();
+        }
+    }
+
+    [Fact]
+    public void Dispose_IsIdempotent()
+    {
+        var job = ProcessJobObject.Create();
+        Assert.NotNull(job);
+
+        job!.Dispose();
+        job.Dispose();
+    }
+
+    [Fact]
+    public void TryAssign_Throws_ForNullProcess()
+    {
+        using var job = ProcessJobObject.Create();
+
+        Assert.Throws<ArgumentNullException>(() => job!.TryAssign(null!));
+    }
+
+    /// <summary>
+    /// Starts a process that runs for long enough that any exit observed during a test is
+    /// attributable to the job object rather than to natural termination.
+    /// </summary>
+    private static Process StartLongRunningProcess()
+    {
+        var startInfo = new ProcessStartInfo("ping.exe", "-n 120 127.0.0.1")
+        {
+            UseShellExecute = false,
+            CreateNoWindow = true,
+            RedirectStandardOutput = true,
+        };
+
+        var process = Process.Start(startInfo);
+        Assert.NotNull(process);
+
+        return process!;
+    }
+
+    private static void KillIfRunning(Process process)
+    {
+        try
+        {
+            if (!process.HasExited)
+            {
+                process.Kill();
+                process.WaitForExit(ExitWaitMs);
+            }
+        }
+        catch (InvalidOperationException)
+        {
+            // Already gone.
+        }
+    }
+}

src/CodingWithCalvin.MCPServer.Tests/ServerShutdownTests.cs

@@ -0,0 +1,114 @@
+using System;
+using System.Collections.Generic;
+using System.Threading;
+using System.Threading.Tasks;
+using CodingWithCalvin.MCPServer.Services;
+using CodingWithCalvin.MCPServer.Shared.Models;
+using Xunit;
+
+namespace CodingWithCalvin.MCPServer.Tests;
+
+/// <summary>
+/// Regression tests for issue #97: package disposal blocked the Visual Studio UI thread waiting
+/// on <c>StopAsync</c>, whose continuations were posted straight back to that same blocked
+/// thread. devenv.exe never finished exiting and stayed resident in Task Manager.
+/// </summary>
+public class ServerShutdownTests
+{
+    private static readonly TimeSpan CompletionTimeout = TimeSpan.FromSeconds(5);
+    private static readonly TimeSpan ThreadJoinTimeout = TimeSpan.FromSeconds(30);
+
+    [Fact]
+    public void ServerProcessManager_StopAsync_CompletesWhenCallerBlocksItsSynchronizationContext()
+    {
+        var completed = RunWithBlockedSynchronizationContext(() =>
+        {
+            var manager = new ServerProcessManager(new StubRpcServer());
+            return manager.StopAsync();
+        });
+
+        Assert.True(
+            completed,
+            "ServerProcessManager.StopAsync did not complete. A continuation is being posted back to "
+                + "the caller's synchronization context, which is the deadlock from issue #97. "
+                + "Every await in the shutdown path needs ConfigureAwait(false).");
+    }
+
+    /// <summary>
+    /// Mirrors the shape of package disposal itself: a synchronous, blocking wait on the shutdown
+    /// path from a thread that cannot pump its own message queue.
+    /// </summary>
+    [Fact]
+    public void BlockingOnStopAsync_Returns_WhenCallerBlocksItsSynchronizationContext()
+    {
+        var completed = RunWithBlockedSynchronizationContext(() =>
+        {
+            var manager = new ServerProcessManager(new StubRpcServer());
+            manager.StopAsync().GetAwaiter().GetResult();
+            return Task.CompletedTask;
+        });
+
+        Assert.True(completed, "Blocking on the shutdown path deadlocked the calling thread.");
+    }
+
+    /// <summary>
+    /// Runs <paramref name="operation"/> on a thread whose synchronization context silently drops
+    /// everything posted to it — the observable behaviour of a UI thread blocked inside
+    /// <c>Dispose</c>. Any continuation that tries to resume on the caller's context will never
+    /// run, so the returned task never completes and this reports <see langword="false"/>.
+    /// </summary>
+    private static bool RunWithBlockedSynchronizationContext(Func<Task> operation)
+    {
+        var completed = false;
+
+        var thread = new Thread(() =>
+        {
+            SynchronizationContext.SetSynchronizationContext(new BlockedSynchronizationContext());
+            completed = operation().Wait(CompletionTimeout);
+        })
+        {
+            IsBackground = true,
+        };
+
+        thread.Start();
+        thread.Join(ThreadJoinTimeout);
+
+        return completed;
+    }
+
+    private sealed class BlockedSynchronizationContext : SynchronizationContext
+    {
+        public override void Post(SendOrPostCallback d, object? state)
+        {
+            // Deliberately dropped: the thread that would pump this is blocked.
+        }
+
+        public override void Send(SendOrPostCallback d, object? state)
+            => throw new NotSupportedException("The blocked thread cannot run work synchronously.");
+    }
+
+    /// <summary>
+    /// Minimal <see cref="IRpcServer"/> whose async members complete on the thread pool, which is
+    /// what makes the captured-context bug observable.
+    /// </summary>
+    private sealed class StubRpcServer : IRpcServer
+    {
+        public string PipeName => string.Empty;
+
+        public bool IsListening => true;
+
+        public bool IsConnected => false;
+
+        public Task StartAsync(string pipeName) => Task.CompletedTask;
+
+        public async Task StopAsync() => await Task.Delay(25).ConfigureAwait(false);
+
+        public Task<List<ToolInfo>> GetAvailableToolsAsync() => Task.FromResult(new List<ToolInfo>());
+
+        public async Task RequestShutdownAsync() => await Task.Delay(25).ConfigureAwait(false);
+
+        public void Dispose()
+        {
+        }
+    }
+}

src/CodingWithCalvin.MCPServer.slnx

@@ -2,4 +2,5 @@
   <Project Path="CodingWithCalvin.MCPServer/CodingWithCalvin.MCPServer.csproj" />
   <Project Path="CodingWithCalvin.MCPServer.Server/CodingWithCalvin.MCPServer.Server.csproj" />
   <Project Path="CodingWithCalvin.MCPServer.Shared/CodingWithCalvin.MCPServer.Shared.csproj" />
+  <Project Path="CodingWithCalvin.MCPServer.Tests/CodingWithCalvin.MCPServer.Tests.csproj" />
 </Solution>

src/CodingWithCalvin.MCPServer/CodingWithCalvin.MCPServer.csproj

@@ -5,10 +5,19 @@
     <LangVersion>latest</LangVersion>
     <Nullable>enable</Nullable>
     <RootNamespace>CodingWithCalvin.MCPServer</RootNamespace>
-    <!-- Suppress VS-Threading analyzer warnings that are architectural decisions or false positives in VSIX context -->
-    <NoWarn>$(NoWarn);VSTHRD002;VSTHRD003;VSTHRD010;VSTHRD110;VSSDK007</NoWarn>
+    <!--
+      Suppress VS-Threading analyzer warnings that are architectural decisions or false positives
+      in VSIX context. VSTHRD002 (synchronous waits) is deliberately NOT suppressed here: it flags
+      exactly the deadlock that left devenv.exe resident in issue #97. Suppress it at the specific
+      call site, with a justification, if a blocking wait is genuinely required.
+    -->
+    <NoWarn>$(NoWarn);VSTHRD003;VSTHRD010;VSTHRD110;VSSDK007</NoWarn>
   </PropertyGroup>
 
+  <ItemGroup>
+    <InternalsVisibleTo Include="CodingWithCalvin.MCPServer.Tests" />
+  </ItemGroup>
+
   <ItemGroup>
     <PackageReference Include="CodingWithCalvin.Otel4Vsix" Version="0.2.2" />
     <PackageReference Include="Microsoft.VisualStudio.Editor" Version="17.14.249" />

src/CodingWithCalvin.MCPServer/MCPServerPackage.cs

@@ -102,13 +102,68 @@ public void InitializeServices()
         }
     }
 
+    /// <summary>
+    /// Total time package disposal will spend shutting the server down before giving up and
+    /// letting Visual Studio finish exiting.
+    /// </summary>
+    private static readonly TimeSpan ShutdownTimeout = TimeSpan.FromSeconds(5);
+
+    /// <remarks>
+    /// Visual Studio calls this on the UI thread. Shutdown work is therefore pushed onto the
+    /// thread pool and waited on with a timeout: <see cref="Task.Run(Func{Task})"/> starts with
+    /// no synchronization context, so no continuation can need the UI thread back, and the
+    /// timeout bounds the damage if one ever does. Blocking the UI thread directly on
+    /// <c>StopAsync</c> deadlocked and left devenv.exe resident after the main window closed
+    /// (issue #97).
+    /// </remarks>
     protected override void Dispose(bool disposing)
     {
         if (disposing)
         {
-            ServerManager?.StopAsync().GetAwaiter().GetResult();
-            RpcServer?.Dispose();
+            var serverManager = ServerManager;
+
+            if (serverManager != null)
+            {
+                try
+                {
+                    var stopTask = Task.Run(() => serverManager.StopAsync());
+
+                    // VSTHRD002: Dispose cannot be async, so a blocking wait is unavoidable. It is
+                    // safe here because Task.Run starts the work without a synchronization context
+                    // and StopAsync uses ConfigureAwait(false) throughout, so no continuation can
+                    // require this thread. The timeout guarantees VS exits regardless.
+#pragma warning disable VSTHRD002
+                    if (!stopTask.Wait(ShutdownTimeout))
+#pragma warning restore VSTHRD002
+                    {
+                        // The job object assigned at start-up still guarantees the server
+                        // process dies when devenv.exe does, so exiting is safe here.
+                        System.Diagnostics.Debug.WriteLine("MCPServer: server shutdown timed out during package disposal.");
+                    }
+                }
+                catch (Exception ex)
+                {
+                    // A failure to stop the server must never prevent Visual Studio from exiting.
+                    System.Diagnostics.Debug.WriteLine($"MCPServer: error stopping server during package disposal: {ex}");
+                }
+            }
+
+            try
+            {
+                RpcServer?.Dispose();
+            }
+            catch (Exception ex)
+            {
+                System.Diagnostics.Debug.WriteLine($"MCPServer: error disposing RPC server: {ex}");
+            }
+
             VsixTelemetry.Shutdown();
+
+            ServerManager = null;
+            RpcServer = null;
+            VsService = null;
+            OutputPaneService = null;
+            Settings = null;
             Instance = null;
         }