diff --git a/src/Microsoft.Diagnostics.NETCore.Client/ReversedServer/ReversedDiagnosticsServer.cs b/src/Microsoft.Diagnostics.NETCore.Client/ReversedServer/ReversedDiagnosticsServer.cs index b4e26bd207..f4a9d4fae1 100644 --- a/src/Microsoft.Diagnostics.NETCore.Client/ReversedServer/ReversedDiagnosticsServer.cs +++ b/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; /// /// Constructs the 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. } /// @@ -188,19 +203,13 @@ private void VerifyIsStarted() } /// - /// Listens at the address for new connections. + /// Accept connections from the transport. /// - /// The maximum number of connections the server will support. + /// The server transport from which connections are accepted. /// The token to monitor for cancellation requests. /// A task that completes when the server is no longer listening at the address. - 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; diff --git a/src/tests/Microsoft.Diagnostics.NETCore.Client/ReversedServerTests.cs b/src/tests/Microsoft.Diagnostics.NETCore.Client/ReversedServerTests.cs index e7a1867f1c..4576994396 100644 --- a/src/tests/Microsoft.Diagnostics.NETCore.Client/ReversedServerTests.cs +++ b/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( () => 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(() => 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) + { + } + } + } + /// /// Tests that does not complete /// when no connections are available and that cancellation will move the returned task to the cancelled state.