Fix shutdown cancellation and process waits under parallel load

.editorconfig

@@ -18,6 +18,9 @@ csharp_style_expression_bodied_accessors = true:silent
 csharp_style_expression_bodied_lambdas = true:silent
 csharp_style_expression_bodied_local_functions = false:silent
 
+# Tests must exercise the public API rather than friend assemblies.
+dotnet_diagnostic.RS0030.severity = error
+
 [*.{cs,vb}]
 dotnet_style_operator_placement_when_wrapping = beginning_of_line
 tab_width = 4

BannedSymbols.txt

@@ -0,0 +1 @@
+T:System.Runtime.CompilerServices.InternalsVisibleToAttribute;Do not expose internals to other assemblies. Tests must use the public API.

Directory.Build.props

@@ -29,6 +29,11 @@
     <SymbolPackageFormat>snupkg</SymbolPackageFormat>
   </PropertyGroup>
 
+  <ItemGroup>
+    <PackageReference Include="Microsoft.CodeAnalysis.BannedApiAnalyzers" PrivateAssets="all" />
+    <AdditionalFiles Include="$(MSBuildThisFileDirectory)BannedSymbols.txt" />
+  </ItemGroup>
+
   <ItemGroup>
     <Using Include="System"/>
     <Using Include="System.Collections.Generic"/>

Directory.Packages.props

@@ -10,6 +10,7 @@
     <PackageVersion Include="GrpcDotNetNamedPipes" Version="3.1.0" />
     <PackageVersion Include="MaterialDesignThemes" Version="5.3.2" />
     <PackageVersion Include="Microsoft.CodeAnalysis.Analyzers" Version="5.9.0" PrivateAssets="all" />
+    <PackageVersion Include="Microsoft.CodeAnalysis.BannedApiAnalyzers" Version="5.6.0" />
     <PackageVersion Include="Microsoft.CodeAnalysis.CSharp" Version="5.9.0" PrivateAssets="all" />
     <PackageVersion Include="Microsoft.CodeAnalysis.Common" Version="5.9.0" />
     <PackageVersion Include="Microsoft.Extensions.Hosting" Version="10.0.12" />
@@ -18,7 +19,7 @@
     <PackageVersion Include="Microsoft.VisualStudio.Interop" Version="17.14.40260" />
     <PackageVersion Include="Microsoft.Windows.CsWin32" Version="0.3.360" />
     <PackageVersion Include="ModelContextProtocol" Version="2.2.0" />
-    <PackageVersion Include="MSTest.TestAdapter" Version="4.4.1" />
+    <PackageVersion Include="MSTest.TestAdapter" Version="4.5.1" />
     <PackageVersion Include="MSTest.TestFramework" Version="4.5.1" />
     <PackageVersion Include="System.CommandLine" Version="2.0.12" />
     <PackageVersion Include="System.Drawing.Common" Version="10.0.7" />

README.md

@@ -10,3 +10,17 @@ After the 1.0.0 release, this library will follow [Semantic Versioning](https://
 
 ## Samples?
 See the tests (XAMLTests.Tests) for samples of how tests can be setup. [Material Design In XAML Toolkit](https://github.com/MaterialDesignInXAML/MaterialDesignInXamlToolkit) is also leveraging this library for the purpose of testing its various styles and templates. You can see samples of its tests [here](https://github.com/MaterialDesignInXAML/MaterialDesignInXamlToolkit/tree/master/MaterialDesignThemes.UITests).
+
+## Application cleanup
+Dispose each `IApp` after use, preferably with `await using`. Disposal allows up to
+10 seconds for the shutdown RPC, then waits up to 10 seconds for process exit.
+If the host does not exit, it is killed and given another 10 seconds to terminate.
+Asynchronous disposal waits without blocking a thread or polling process state.
+Local shutdown timeouts are logged and process cleanup still runs; unexpected
+RPC errors remain visible to the caller.
+
+## Testing policy
+Tests must exercise public APIs, not internal implementation details.
+`Microsoft.CodeAnalysis.BannedApiAnalyzers` runs in every project, using the
+repository-wide `BannedSymbols.txt`. `InternalsVisibleToAttribute` is banned;
+violations fail the build with RS0030.

XAMLTest.Tests/AppDisposalTests.cs

@@ -0,0 +1,170 @@
+using System.Collections.Concurrent;
+
+namespace XamlTest.Tests;
+
+[TestClass]
+public class AppDisposalTests
+{
+    [TestMethod]
+    [DataRow(false)]
+    [DataRow(true)]
+    public async Task AlreadyExitedProcess_DisposesSuccessfully(bool asynchronous)
+    {
+        IApp app = await StartApp();
+        using Process process = await GetHostProcess(app);
+        try
+        {
+            process.Kill();
+            await process.WaitForExitAsync();
+
+            await Dispose(app, asynchronous);
+
+            Assert.IsTrue(process.HasExited);
+        }
+        finally
+        {
+            await EnsureProcessExited(process);
+        }
+    }
+
+    [TestMethod]
+    [DataRow(false)]
+    [DataRow(true)]
+    public async Task TenConcurrentApps_CanStartAndDisposeRepeatedly(bool asynchronous)
+    {
+        for (int batch = 0; batch < 3; batch++)
+        {
+            await Task.WhenAll(Enumerable.Range(0, 10).Select(_ => Task.Run(async () =>
+            {
+                IApp app = await StartApp();
+                using Process process = await GetHostProcess(app);
+                try
+                {
+                    await Dispose(app, asynchronous);
+
+                    Assert.IsTrue(process.HasExited);
+                }
+                finally
+                {
+                    await EnsureProcessExited(process);
+                }
+            })));
+        }
+    }
+
+    [TestMethod]
+    [DataRow(false)]
+    [DataRow(true)]
+    public async Task SlowExit_DisposalWaitsForProcess(bool asynchronous)
+    {
+        ConcurrentQueue<string> messages = new();
+        IApp app = await StartApp(messages.Enqueue);
+        using Process process = await GetHostProcess(app);
+        try
+        {
+            static void DelayExit(Application application)
+            {
+                application.Exit += (_, _) => Thread.Sleep(TimeSpan.FromSeconds(1.5));
+            }
+            await app.RemoteExecute<Application>(DelayExit);
+
+            Stopwatch stopwatch = Stopwatch.StartNew();
+            await Dispose(app, asynchronous);
+
+            Assert.IsTrue(process.HasExited);
+            Assert.IsTrue(stopwatch.Elapsed >= TimeSpan.FromSeconds(1.5));
+            Assert.IsFalse(messages.Any(x => x.Contains("Invoking kill")));
+        }
+        finally
+        {
+            await EnsureProcessExited(process);
+        }
+    }
+
+    [TestMethod]
+    [DataRow(false)]
+    [DataRow(true)]
+    public async Task UnresponsiveExit_IsKilledAndReaped(bool asynchronous)
+    {
+        ConcurrentQueue<string> messages = new();
+        IApp app = await StartApp(messages.Enqueue);
+        using Process process = await GetHostProcess(app);
+        try
+        {
+            static void BlockExit(Application application)
+            {
+                application.Exit += (_, _) => Thread.Sleep(Timeout.Infinite);
+            }
+            await app.RemoteExecute<Application>(BlockExit);
+
+            Task disposal = Dispose(app, asynchronous);
+            if (asynchronous)
+            {
+                Assert.IsFalse(disposal.IsCompleted, "Async disposal must yield while waiting for the host.");
+            }
+            await disposal;
+
+            Assert.IsTrue(process.HasExited);
+            Assert.IsTrue(messages.Any(x => x.Contains("Invoking kill")));
+        }
+        finally
+        {
+            await EnsureProcessExited(process);
+        }
+    }
+
+    private static async Task<IApp> StartApp(Action<string>? logMessage = null)
+    {
+        IApp app = await App.StartRemote(new AppOptions
+        {
+            AllowVisualStudioDebuggerAttach = false,
+            LogMessage = logMessage
+        });
+        try
+        {
+            await app.InitializeWithDefaults(Assembly.GetExecutingAssembly().Location);
+            return app;
+        }
+        catch
+        {
+            await app.DisposeAsync();
+            throw;
+        }
+    }
+
+    private static async Task<Process> GetHostProcess(IApp app)
+    {
+        static int GetProcessId(Application _) => Environment.ProcessId;
+        try
+        {
+            int processId = await app.RemoteExecute<Application, int>(GetProcessId);
+            return Process.GetProcessById(processId);
+        }
+        catch
+        {
+            await app.DisposeAsync();
+            throw;
+        }
+    }
+
+    private static async Task EnsureProcessExited(Process process)
+    {
+        if (!process.HasExited)
+        {
+            process.Kill();
+            await process.WaitForExitAsync().WaitAsync(TimeSpan.FromSeconds(10));
+        }
+    }
+
+    private static async Task Dispose(IApp app, bool asynchronous)
+    {
+        if (asynchronous)
+        {
+            await app.DisposeAsync();
+        }
+        else
+        {
+            app.Dispose();
+        }
+    }
+}

XAMLTest.Tests/NativeMethods.txt

@@ -1 +1 @@
-GetWindowPlacement
+GetWindowPlacement

XAMLTest/Internal/App.cs

@@ -8,6 +8,9 @@ internal sealed class App(
     Protocol.ProtocolClient client,
     AppOptions appOptions) : IApp
 {
+    private static readonly TimeSpan ShutdownTimeout = TimeSpan.FromSeconds(10);
+    private static readonly TimeSpan ProcessExitTimeout = TimeSpan.FromSeconds(10);
+
     public Process Process { get; } = process ?? throw new ArgumentNullException(nameof(process));
 
     private Protocol.ProtocolClient Client { get; } = client ?? throw new ArgumentNullException(nameof(client));
@@ -27,10 +30,9 @@ public void Dispose()
             ExitCode = 0
         };
         LogMessage?.Invoke($"{nameof(IApp)}.{nameof(Dispose)}()");
+        using CancellationTokenSource cts = new(ShutdownTimeout);
         try
         {
-            using CancellationTokenSource cts = new();
-            cts.CancelAfter(TimeSpan.FromSeconds(1));
             if (Client.Shutdown(request, cancellationToken: cts.Token) is { } reply)
             {
                 if (reply.ErrorMessages.Any())
@@ -41,14 +43,26 @@ public void Dispose()
             }
             throw new XamlTestException("Failed to get a reply");
         }
-        catch (OperationCanceledException)
-        { }
+        catch (OperationCanceledException) when (cts.IsCancellationRequested)
+        {
+            LogMessage?.Invoke("Shutdown request timed out; waiting for process exit.");
+        }
+        catch (RpcException rpcException) when (rpcException.StatusCode == StatusCode.Cancelled && cts.IsCancellationRequested)
+        {
+            LogMessage?.Invoke("Shutdown request timed out; waiting for process exit.");
+        }
         catch (RpcException rpcException) when (rpcException.StatusCode == StatusCode.Unavailable)
         { }
         finally
         {
-            KillProcess();
-            CleanupLogFiles();
+            try
+            {
+                KillProcess();
+            }
+            finally
+            {
+                CleanupLogFiles();
+            }
         }
     }
 
@@ -59,10 +73,9 @@ public async ValueTask DisposeAsync()
             ExitCode = 0
         };
         LogMessage?.Invoke($"{nameof(IApp)}.{nameof(DisposeAsync)}()");
+        using CancellationTokenSource cts = new(ShutdownTimeout);
         try
         {
-            using CancellationTokenSource cts = new();
-            cts.CancelAfter(TimeSpan.FromSeconds(1));
             if (await Client.ShutdownAsync(request, cancellationToken: cts.Token) is { } reply)
             {
                 if (reply.ErrorMessages.Any())
@@ -73,14 +86,26 @@ public async ValueTask DisposeAsync()
             }
             throw new XamlTestException("Failed to get a reply");
         }
-        catch (OperationCanceledException)
-        { }
+        catch (OperationCanceledException) when (cts.IsCancellationRequested)
+        {
+            LogMessage?.Invoke("Shutdown request timed out; waiting for process exit.");
+        }
+        catch (RpcException rpcException) when (rpcException.StatusCode == StatusCode.Cancelled && cts.IsCancellationRequested)
+        {
+            LogMessage?.Invoke("Shutdown request timed out; waiting for process exit.");
+        }
         catch (RpcException rpcException) when (rpcException.StatusCode == StatusCode.Unavailable)
         { }
         finally
         {
-            KillProcess();
-            await CleanupLogFilesAsync();
+            try
+            {
+                await KillProcessAsync();
+            }
+            finally
+            {
+                await CleanupLogFilesAsync();
+            }
         }
     }
 
@@ -133,29 +158,41 @@ private void CleanupLogFiles()
     private void KillProcess()
     {
         LogMessage?.Invoke("Waiting for process exit");
-        using CancellationTokenSource cts = new();
-        cts.CancelAfter(TimeSpan.FromSeconds(10));
-        Process? process = null;
-        do
+        if (!Process.WaitForExit((int)ProcessExitTimeout.TotalMilliseconds))
         {
-            try
-            {
-                process = Process.GetProcessById(Process.Id);
-            }
-            catch (ArgumentException)
+            LogMessage?.Invoke($"Invoking kill");
+            Process.Kill();
+            if (!Process.WaitForExit((int)ProcessExitTimeout.TotalMilliseconds))
             {
-                //Thrown when the process specified by the processId parameter is not running.
+                throw new XamlTestException("Host process did not exit after being killed.");
             }
         }
-        while (process?.HasExited == false && !cts.IsCancellationRequested);
+        LogMessage?.Invoke($"Process Exited? {Process.HasExited}");
+    }
 
-        LogMessage?.Invoke($"Process Exited? {process?.HasExited}");
-        if (process?.HasExited == false)
+    private async Task KillProcessAsync()
+    {
+        LogMessage?.Invoke("Waiting for process exit");
+        using CancellationTokenSource cts = new(ProcessExitTimeout);
+        try
         {
-            LogMessage?.Invoke($"Invoking kill");
-            process.Kill();
-            process.WaitForExit(1_000);
+            await Process.WaitForExitAsync(cts.Token);
+        }
+        catch (OperationCanceledException) when (cts.IsCancellationRequested)
+        {
+            LogMessage?.Invoke("Invoking kill");
+            Process.Kill();
+            using CancellationTokenSource killCts = new(ProcessExitTimeout);
+            try
+            {
+                await Process.WaitForExitAsync(killCts.Token);
+            }
+            catch (OperationCanceledException e) when (killCts.IsCancellationRequested)
+            {
+                throw new XamlTestException("Host process did not exit after being killed.", e);
+            }
         }
+        LogMessage?.Invoke($"Process Exited? {Process.HasExited}");
     }
 
     public async Task Initialize(string applicationResourceXaml, params string[] assemblies)