From 3f7ee338d4cdd1c49bb965338ad7b118fa070a83 Mon Sep 17 00:00:00 2001 From: Mike Harder Date: Tue, 17 Apr 2018 14:01:33 -0700 Subject: [PATCH] Change SelfHostDeployer to use dynamic ports by default (#1383) * Should significantly reduce flaky test failures due to AddressInUse exceptions * Addresses https://github.com/aspnet/Hosting/issues/1296 --- .../Common/TestUriHelper.cs | 47 ++++++++++++++++--- .../Deployers/SelfHostDeployer.cs | 5 +- 2 files changed, 44 insertions(+), 8 deletions(-) diff --git a/src/Microsoft.AspNetCore.Server.IntegrationTesting/Common/TestUriHelper.cs b/src/Microsoft.AspNetCore.Server.IntegrationTesting/Common/TestUriHelper.cs index 9fff9f1c59a4..7ebcb3036774 100644 --- a/src/Microsoft.AspNetCore.Server.IntegrationTesting/Common/TestUriHelper.cs +++ b/src/Microsoft.AspNetCore.Server.IntegrationTesting/Common/TestUriHelper.cs @@ -11,39 +11,72 @@ public static class TestUriHelper { public static Uri BuildTestUri() { - return new UriBuilder("http", "localhost", GetNextPort()).Uri; + return BuildTestUri(null); } public static Uri BuildTestUri(string hint) + { + // If this method is called directly, there is no way to know the server type or whether status messages + // are enabled. It's safest to assume the server is WebListener (which doesn't support binding to dynamic + // port "0") and status messages are not enabled (so the assigned port cannot be scraped from console output). + return BuildTestUri(hint, serverType: ServerType.WebListener, statusMessagesEnabled: false); + } + + internal static Uri BuildTestUri(string hint, ServerType serverType, bool statusMessagesEnabled) { if (string.IsNullOrEmpty(hint)) { - return BuildTestUri(); + if (serverType == ServerType.Kestrel && statusMessagesEnabled) + { + // Most functional tests use this codepath and should directly bind to dynamic port "0" and scrape + // the assigned port from the status message, which should be 100% reliable since the port is bound + // once and never released. Binding to dynamic port "0" on "localhost" (both IPv4 and IPv6) is not + // supported, so the port is only bound on "127.0.0.1" (IPv4). If a test explicitly requires IPv6, + // it should provide a hint URL with "localhost" (IPv4 and IPv6) or "[::1]" (IPv6-only). + return new UriBuilder("http", "127.0.0.1", 0).Uri; + } + else + { + // If the server type is not Kestrel, or status messages are disabled, there is no status message + // from which to scrape the assigned port, so the less reliable GetNextPort() must be used. The + // port is bound on "localhost" (both IPv4 and IPv6), since this is supported when using a specific + // (non-zero) port. + return new UriBuilder("http", "localhost", GetNextPort()).Uri; + } } else { var uriHint = new Uri(hint); if (uriHint.Port == 0) { + // Only a few tests use this codepath, so it's fine to use the less reliable GetNextPort() for simplicity. + // The tests using this codepath will be reviewed to see if they can be changed to directly bind to dynamic + // port "0" on "127.0.0.1" and scrape the assigned port from the status message (the default codepath). return new UriBuilder(uriHint) { Port = GetNextPort() }.Uri; } else { + // If the hint contains a specific port, return it unchanged. return uriHint; } } } // Copied from https://github.com/aspnet/KestrelHttpServer/blob/47f1db20e063c2da75d9d89653fad4eafe24446c/test/Microsoft.AspNetCore.Server.Kestrel.FunctionalTests/AddressRegistrationTests.cs#L508 + // + // This method is an attempt to safely get a free port from the OS. Most of the time, + // when binding to dynamic port "0" the OS increments the assigned port, so it's safe + // to re-use the assigned port in another process. However, occasionally the OS will reuse + // a recently assigned port instead of incrementing, which causes flaky tests with AddressInUse + // exceptions. This method should only be used when the application itself cannot use + // dynamic port "0" (e.g. IISExpress). Most functional tests using raw Kestrel + // (with status messages enabled) should directly bind to dynamic port "0" and scrape + // the assigned port from the status message, which should be 100% reliable since the port + // is bound once and never released. public static int GetNextPort() { using (var socket = new Socket(AddressFamily.InterNetwork, SocketType.Stream, ProtocolType.Tcp)) { - // Let the OS assign the next available port. Unless we cycle through all ports - // on a test run, the OS will always increment the port number when making these calls. - // This prevents races in parallel test runs where a test is already bound to - // a given port, and a new test is able to bind to the same port due to port - // reuse being enabled by default by the OS. socket.Bind(new IPEndPoint(IPAddress.Loopback, 0)); return ((IPEndPoint)socket.LocalEndPoint).Port; } diff --git a/src/Microsoft.AspNetCore.Server.IntegrationTesting/Deployers/SelfHostDeployer.cs b/src/Microsoft.AspNetCore.Server.IntegrationTesting/Deployers/SelfHostDeployer.cs index 99fb9b61d659..12f2b83de1ff 100644 --- a/src/Microsoft.AspNetCore.Server.IntegrationTesting/Deployers/SelfHostDeployer.cs +++ b/src/Microsoft.AspNetCore.Server.IntegrationTesting/Deployers/SelfHostDeployer.cs @@ -41,7 +41,10 @@ public override async Task DeployAsync() DotnetPublish(); } - var hintUrl = TestUriHelper.BuildTestUri(DeploymentParameters.ApplicationBaseUriHint); + var hintUrl = TestUriHelper.BuildTestUri( + DeploymentParameters.ApplicationBaseUriHint, + DeploymentParameters.ServerType, + DeploymentParameters.StatusMessagesEnabled); // Launch the host process. var (actualUrl, hostExitToken) = await StartSelfHostAsync(hintUrl);