Cleanup Dispose implementations (#6730)

Author: Youssef1313

src/Platform/Microsoft.Testing.Extensions.HangDump/HangDumpActivityIndicator.cs

@@ -18,10 +18,9 @@ namespace Microsoft.Testing.Extensions.Diagnostics;
 
 internal sealed class HangDumpActivityIndicator : IDataConsumer, ITestSessionLifetimeHandler,
 #if NETCOREAPP
-    IAsyncDisposable
-#else
-    IDisposable
+    IAsyncDisposable,
 #endif
+    IDisposable
 {
     private readonly ICommandLineOptions _commandLineOptions;
     private readonly IEnvironment _environment;
@@ -267,7 +266,7 @@ private async Task ExitSignalActivityIndicatorTaskAsync()
 #if NETCOREAPP
     public async ValueTask DisposeAsync()
     {
-        await DisposeHelper.DisposeAsync(_namedPipeClient).ConfigureAwait(false);
+        _namedPipeClient?.Dispose();
 
         // If the OnTestSessionFinishingAsync is not called means that something unhandled happened
         // and we didn't correctly coordinate the shutdown with the HangDumpProcessLifetimeHandler.
@@ -277,12 +276,12 @@ public async ValueTask DisposeAsync()
             await DisposeHelper.DisposeAsync(_singleConnectionNamedPipeServer).ConfigureAwait(false);
         }
 
-        _pipeNameDescription?.Dispose();
         _mutexCreated.Dispose();
         _signalActivity.Dispose();
         _activityIndicatorMutex?.Dispose();
     }
-#else
+#endif
+
     public void Dispose()
     {
         _namedPipeClient?.Dispose();
@@ -295,10 +294,8 @@ public void Dispose()
             _singleConnectionNamedPipeServer?.Dispose();
         }
 
-        _pipeNameDescription?.Dispose();
         _mutexCreated.Dispose();
         _signalActivity.Dispose();
         _activityIndicatorMutex?.Dispose();
     }
-#endif
 }

src/Platform/Microsoft.Testing.Extensions.HangDump/HangDumpProcessLifetimeHandler.cs

@@ -25,10 +25,9 @@ namespace Microsoft.Testing.Extensions.Diagnostics;
 
 internal sealed class HangDumpProcessLifetimeHandler : ITestHostProcessLifetimeHandler, IOutputDeviceDataProducer, IDataProducer,
 #if NETCOREAPP
-    IAsyncDisposable
-#else
-    IDisposable
+    IAsyncDisposable,
 #endif
+    IDisposable
 {
     private readonly IMessageBus _messageBus;
     private readonly OutputDeviceWriter _outputDisplay;
@@ -140,14 +139,7 @@ private async Task<IResponse> CallbackAsync(IRequest request)
         {
             await _logger.LogDebugAsync("Session end received by the test host").ConfigureAwait(false);
             _exitActivityIndicatorTask = true;
-#if NET
-            if (_namedPipeClient is not null)
-            {
-                await _namedPipeClient.DisposeAsync().ConfigureAwait(false);
-            }
-#else
             _namedPipeClient?.Dispose();
-#endif
             return VoidResponse.CachedInstance;
         }
         else if (request is ConsumerPipeNameRequest consumerPipeNameRequest)
@@ -519,7 +511,6 @@ public void Dispose()
         _waitConsumerPipeName.Dispose();
         _mutexNameReceived.Dispose();
         _singleConnectionNamedPipeServer?.Dispose();
-        _pipeNameDescription.Dispose();
     }
 
 #if NETCOREAPP
@@ -534,7 +525,6 @@ public async ValueTask DisposeAsync()
         _waitConsumerPipeName.Dispose();
         _mutexNameReceived.Dispose();
         _singleConnectionNamedPipeServer?.Dispose();
-        _pipeNameDescription.Dispose();
     }
 #endif
 }

src/Platform/Microsoft.Testing.Extensions.Retry/RetryFailedTestsPipeServer.cs

@@ -45,10 +45,7 @@ public Task WaitForConnectionAsync(CancellationToken cancellationToken)
         => _singleConnectionNamedPipeServer.WaitConnectionAsync(cancellationToken);
 
     public void Dispose()
-    {
-        _singleConnectionNamedPipeServer.Dispose();
-        _pipeNameDescription.Dispose();
-    }
+        => _singleConnectionNamedPipeServer.Dispose();
 
     private Task<IResponse> CallbackAsync(IRequest request)
     {

src/Platform/Microsoft.Testing.Extensions.Retry/RetryLifecycleCallbacks.cs

@@ -14,12 +14,7 @@
 
 namespace Microsoft.Testing.Extensions.Policy;
 
-internal sealed class RetryLifecycleCallbacks : ITestHostApplicationLifetime,
-#if NETCOREAPP
-    IAsyncDisposable
-#else
-    IDisposable
-#endif
+internal sealed class RetryLifecycleCallbacks : ITestHostApplicationLifetime, IDisposable
 {
     private readonly IServiceProvider _serviceProvider;
     private readonly ICommandLineOptions _commandLineOptions;
@@ -73,15 +68,5 @@ public Task<bool> IsEnabledAsync()
     public Task AfterRunAsync(int exitCode, CancellationToken cancellation)
         => Task.CompletedTask;
 
-#if NETCOREAPP
-    public async ValueTask DisposeAsync()
-    {
-        if (Client is not null)
-        {
-            await Client.DisposeAsync().ConfigureAwait(false);
-        }
-    }
-#else
     public void Dispose() => Client?.Dispose();
-#endif
 }

src/Platform/Microsoft.Testing.Extensions.Telemetry/AppInsightsProvider.cs

@@ -18,14 +18,11 @@ namespace Microsoft.Testing.Extensions.Telemetry;
 /// Allows to log telemetry events via AppInsights.
 /// </summary>
 internal sealed partial class AppInsightsProvider :
-    ITelemetryCollector
-#pragma warning disable SA1001 // Commas should be spaced correctly
+    ITelemetryCollector,
 #if NETCOREAPP
-    , IAsyncDisposable
-#else
-    , IDisposable
+    IAsyncDisposable,
 #endif
-#pragma warning restore SA1001 // Commas should be spaced correctly
+    IDisposable
 {
     // Note: We're currently using the same environment variable as dotnet CLI.
     public static readonly string SessionIdEnvVar = "TESTINGPLATFORM_APPINSIGHTS_SESSIONID";
@@ -282,11 +279,14 @@ Task LogEventAsync(string eventName, IDictionary<string, object> paramsMap, Canc
 #endif
     }
 
-#if !NETCOREAPP
     // Adding dispose on graceful shutdown per https://github.com/microsoft/ApplicationInsights-dotnet/issues/1152#issuecomment-518742922
     public void Dispose()
     {
+#if NETCOREAPP
+        _payloads.Writer.Complete();
+#else
         _payloads.CompleteAdding();
+#endif
         if (!_isDisposed)
         {
             if (_telemetryTask is null)
@@ -304,7 +304,6 @@ public void Dispose()
             _isDisposed = true;
         }
     }
-#endif
 
 #if NETCOREAPP
     public async ValueTask DisposeAsync()

src/Platform/Microsoft.Testing.Extensions.TrxReport/TrxProcessLifetimeHandler.cs

@@ -28,10 +28,9 @@ internal sealed class TrxProcessLifetimeHandler :
     IDataProducer,
     IOutputDeviceDataProducer,
 #if NETCOREAPP
-    IAsyncDisposable
-#else
-    IDisposable
+    IAsyncDisposable,
 #endif
+    IDisposable
 {
     private readonly ICommandLineOptions _commandLineOptions;
     private readonly IEnvironment _environment;
@@ -231,21 +230,11 @@ private Task<IResponse> CallbackAsync(IRequest request)
 
 #if NETCOREAPP
     public async ValueTask DisposeAsync()
-    {
-        await DisposeHelper.DisposeAsync(_singleConnectionNamedPipeServer).ConfigureAwait(false);
+        => await DisposeHelper.DisposeAsync(_singleConnectionNamedPipeServer).ConfigureAwait(false);
+#endif
 
-        // Dispose the pipe descriptor after the server to ensure the pipe is closed.
-        _pipeNameDescription.Dispose();
-    }
-#else
     public void Dispose()
-    {
-        _singleConnectionNamedPipeServer?.Dispose();
-
-        // Dispose the pipe descriptor after the server to ensure the pipe is closed.
-        _pipeNameDescription.Dispose();
-    }
-#endif
+        => _singleConnectionNamedPipeServer?.Dispose();
 
     private sealed class ExtensionInfo : IExtension
     {

src/Platform/Microsoft.Testing.Platform/CommandLine/InformativeCommandLineTestHost.cs

@@ -8,11 +8,6 @@
 namespace Microsoft.Testing.Platform.CommandLine;
 
 internal sealed class InformativeCommandLineHost(int returnValue, IServiceProvider serviceProvider) : IHost, IDisposable
-#if NETCOREAPP
-#pragma warning disable SA1001 // Commas should be spaced correctly
-    , IAsyncDisposable
-#pragma warning restore SA1001 // Commas should be spaced correctly
-#endif
 {
     private readonly int _returnValue = returnValue;
 
@@ -21,14 +16,4 @@ internal sealed class InformativeCommandLineHost(int returnValue, IServiceProvid
     public Task<int> RunAsync() => Task.FromResult(_returnValue);
 
     public void Dispose() => PushOnlyProtocol?.Dispose();
-
-#if NETCOREAPP
-    public async ValueTask DisposeAsync()
-    {
-        if (PushOnlyProtocol is not null)
-        {
-            await PushOnlyProtocol.DisposeAsync().ConfigureAwait(false);
-        }
-    }
-#endif
 }

src/Platform/Microsoft.Testing.Platform/Hosts/TestHostControlledHost.cs

@@ -47,7 +47,7 @@ public void Dispose()
     public async ValueTask DisposeAsync()
     {
         await DisposeHelper.DisposeAsync(_innerHost).ConfigureAwait(false);
-        await _namedPipeClient.DisposeAsync().ConfigureAwait(false);
+        _namedPipeClient.Dispose();
     }
 #endif
 }

src/Platform/Microsoft.Testing.Platform/IPC/NamedPipeClient.cs

@@ -241,6 +241,11 @@ public void Dispose()
     }
 
 #if NETCOREAPP
+    [Obsolete("All owned fields are disposed synchronously. Introduction of DisposeAsync here is unnecessary complexity.")]
+    // NOTE: While NamedPipeClient is internal API, it's breaking to change it as it's consumed via IVT by MTP extensions.
+    // If we removed DisposeAsync in newer MTP version, but an old MTP extension is used with newer MTP version, we will get MissingMethodException.
+    // It might be more safe to obsolete for now, and potentially remove after few versions are released when most users will
+    // already be on those newer versions, and the risk of break is reduced.
     public async ValueTask DisposeAsync()
     {
         if (_disposed)

src/Platform/Microsoft.Testing.Platform/IPC/NamedPipeServer.cs

@@ -320,7 +320,6 @@ public void Dispose()
         }
 
         _namedPipeServerStream.Dispose();
-        PipeName.Dispose();
 
         _disposed = true;
     }
@@ -351,7 +350,6 @@ public async ValueTask DisposeAsync()
         }
 
         _namedPipeServerStream.Dispose();
-        PipeName.Dispose();
 
         _disposed = true;
     }