Skip to content
This repository was archived by the owner on Dec 19, 2018. It is now read-only.

Change SelfHostDeployer to use dynamic ports by default #1383

Merged
merged 5 commits into from
Apr 17, 2018
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why default to v4 rather than v6?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If the test doesn't care, I assume IPv4 is safer since some environments don't support IPv6 (parts of Azure, maybe Travis/AppVeyor).

}
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;
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -41,7 +41,10 @@ public override async Task<DeploymentResult> 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);
Expand Down