Make ReversedDiagnosticsServer.Start method throw if address is in use

src/Microsoft.Diagnostics.NETCore.Client/ReversedServer/ReversedDiagnosticsServer.cs

@@ -29,8 +29,9 @@ internal sealed class ReversedDiagnosticsServer : IAsyncDisposable
         private readonly string _address;
 
         private bool _disposed = false;
-        private Task _listenTask;
+        private Task _acceptTransportTask;
         private bool _enableTcpIpProtocol = false;
+        private IpcServerTransport _transport;
 
         /// <summary>
         /// Constructs the <see cref="ReversedDiagnosticsServer"/> instance with an endpoint bound
@@ -72,13 +73,24 @@ public async ValueTask DisposeAsync()
         {
             if (!_disposed)
             {
+                // Dispose the server transport before signaling cancellation in order to prevent the
+                // AcceptAsync call on the server transport from recreating the server stream.
+                try
+                {
+                    _transport?.Dispose();
+                }
+                catch (Exception ex)
+                {
+                    Debug.Fail(ex.Message);
+                }
+
                 _disposalSource.Cancel();
 
-                if (null != _listenTask)
+                if (null != _acceptTransportTask)
                 {
                     try
                     {
-                        await _listenTask.ConfigureAwait(false);
+                        await _acceptTransportTask.ConfigureAwait(false);
                     }
                     catch (Exception ex)
                     {
@@ -122,9 +134,12 @@ public void Start(int maxConnections)
                 throw new InvalidOperationException(nameof(ReversedDiagnosticsServer.Start) + " method can only be called once.");
             }
 
-            _listenTask = ListenAsync(maxConnections, _disposalSource.Token);
-            if (_listenTask.IsFaulted)
-                _listenTask.Wait(); // Rethrow aggregated exception.
+            _transport = IpcServerTransport.Create(_address, maxConnections, _enableTcpIpProtocol, TransportCallback);
+
+            _acceptTransportTask = AcceptTransportAsync(_transport, _disposalSource.Token);
+
+            if (_acceptTransportTask.IsFaulted)
+                _acceptTransportTask.Wait(); // Rethrow aggregated exception.
         }
 
         /// <summary>
@@ -188,19 +203,13 @@ private void VerifyIsStarted()
         }
 
         /// <summary>
-        /// Listens at the address for new connections.
+        /// Accept connections from the transport.
         /// </summary>
-        /// <param name="maxConnections">The maximum number of connections the server will support.</param>
+        /// <param name="transport">The server transport from which connections are accepted.</param>
         /// <param name="token">The token to monitor for cancellation requests.</param>
         /// <returns>A task that completes when the server is no longer listening at the address.</returns>
-        private async Task ListenAsync(int maxConnections, CancellationToken token)
+        private async Task AcceptTransportAsync(IpcServerTransport transport, CancellationToken token)
         {
-            // This disposal shuts down the transport in case an exception is thrown.
-            using var transport = IpcServerTransport.Create(_address, maxConnections, _enableTcpIpProtocol, TransportCallback);
-            // This disposal shuts down the transport in case of cancellation; causes the transport
-            // to not recreate the server stream before the AcceptAsync call observes the cancellation.
-            using var _ = token.Register(() => transport.Dispose());
-
             while (!token.IsCancellationRequested)
             {
                 Stream stream = null;
@@ -366,7 +375,7 @@ private static bool TestStream(Stream stream)
             return false;
         }
 
-        private bool IsStarted => null != _listenTask;
+        private bool IsStarted => null != _transport;
 
         public static int MaxAllowedConnections = IpcServerTransport.MaxAllowedConnections;
 

src/tests/Microsoft.Diagnostics.NETCore.Client/ReversedServerTests.cs

@@ -10,6 +10,8 @@
 using System.Diagnostics.Tracing;
 using System.IO;
 using System.Net;
+using System.Net.Sockets;
+using System.Runtime.InteropServices;
 using System.Threading;
 using System.Threading.Tasks;
 using Xunit;
@@ -98,6 +100,40 @@ await Assert.ThrowsAsync<ObjectDisposedException>(
                 () => server.RemoveConnection(Guid.Empty));
         }
 
+        [SkippableFact]
+        public async Task ReversedServerAddressInUseTest()
+        {
+            if (RuntimeInformation.IsOSPlatform(OSPlatform.Windows))
+            {
+                throw new SkipTestException("Not applicable on Windows due to named pipe usage.");
+            }
+
+            await using var server = CreateReversedServer(out string transportName);
+
+            Assert.False(File.Exists(transportName), "Unix Domain Socket should not exist yet.");
+
+            try
+            {
+                // Create file to simulate that the socket is already created.
+                File.Create(transportName).Dispose();
+
+                SocketException ex = Assert.Throws<SocketException>(() => server.Start());
+
+                int expectedErrorCode = RuntimeInformation.IsOSPlatform(OSPlatform.OSX) ? 48 : 98; // Address already in use
+                Assert.Equal(expectedErrorCode, ex.ErrorCode);
+            }
+            finally
+            {
+                try
+                {
+                    File.Delete(transportName);
+                }
+                catch (Exception)
+                {
+                }
+            }
+        }
+
         /// <summary>
         /// Tests that <see cref="ReversedDiagnosticsServer.AcceptAsync(CancellationToken)"/> does not complete
         /// when no connections are available and that cancellation will move the returned task to the cancelled state.