Attempt to simplify the buggy bits port checking

Author: bouwkast

profiler/src/Demos/Samples.BuggyBits/Program.cs

@@ -3,16 +3,14 @@
 // This product includes software developed at Datadog (https://www.datadoghq.com/). Copyright 2022 Datadog, Inc.
 // </copyright>
 using System;
-using System.Collections.Generic;
 using System.Diagnostics;
-using System.Net.Sockets;
+using System.Linq;
 using System.Threading;
 using System.Threading.Tasks;
 using Datadog.Demos.Util;
 using Microsoft.AspNetCore.Hosting;
 using Microsoft.AspNetCore.Hosting.Server;
 using Microsoft.AspNetCore.Hosting.Server.Features;
-using Microsoft.Extensions.Configuration;
 using Microsoft.Extensions.Hosting;
 using Microsoft.Extensions.Logging;
 
@@ -44,8 +42,6 @@ public enum Scenario
 
     public class Program
     {
-        private const int HostBindRetries = 5;
-
         private static bool _disableLogs = false;
 
         public static async Task Main(string[] args)
@@ -58,8 +54,10 @@ public static async Task Main(string[] args)
 
             ParseCommandLine(args, out _disableLogs, out var timeout, out var iterations, out var scenario, out var nbIdleThreads);
 
-            using (var host = await StartHostWithPortResilience(args))
+            using (var host = CreateHostBuilder(args).Build())
             {
+                await host.StartAsync();
+
                 var rootUrl = GetBoundRootUrl(host);
                 WriteLine($"Listening to {rootUrl}");
 
@@ -127,116 +125,8 @@ public static async Task Main(string[] args)
             WriteLine($"The application exited after: {sw.Elapsed} at {DateTime.UtcNow}");
         }
 
-        private static async Task<IHost> StartHostWithPortResilience(string[] args)
-        {
-            var host = CreateHostBuilder(args).Build();
-            try
-            {
-                var rootUrl = GetConfiguredRootUrl(host);
-
-                for (var remainingRetries = HostBindRetries; ; remainingRetries--)
-                {
-                    try
-                    {
-                        WriteLine($"Starting web host at {rootUrl}");
-                        await host.StartAsync();
-                        return host;
-                    }
-                    catch (Exception ex) when (remainingRetries > 0 && IsAddressInUseException(ex))
-                    {
-                        var retryRootUrl = GetUrlWithNewPort(rootUrl);
-                        WriteLine($"Address already in use for {rootUrl}. Retrying on {retryRootUrl}.");
-
-                        host.Dispose();
-                        rootUrl = retryRootUrl;
-                        host = CreateHostBuilder(args, rootUrl).Build();
-                    }
-                }
-            }
-            catch
-            {
-                host.Dispose();
-                throw;
-            }
-        }
-
-        private static string GetConfiguredRootUrl(IHost host)
-        {
-            // ASP.NET Core accepts listening urls from launchSettings.json, ASPNETCORE_URLS,
-            // or --urls. If none are supplied, Kestrel uses this default.
-            var configuration = host.Services.GetService(typeof(IConfiguration)) as IConfiguration;
-            var rootUrl = configuration?["urls"];
-
-            if (string.IsNullOrEmpty(rootUrl))
-            {
-                rootUrl = "http://localhost:5000";
-            }
-
-            return GetFirstUrl(rootUrl);
-        }
-
-        private static string GetBoundRootUrl(IHost host)
-        {
-            var server = host.Services.GetService(typeof(IServer)) as IServer;
-            var addresses = server?.Features.Get<IServerAddressesFeature>()?.Addresses;
-
-            if (addresses != null)
-            {
-                foreach (var address in addresses)
-                {
-                    if (!string.IsNullOrEmpty(address))
-                    {
-                        return address.TrimEnd('/');
-                    }
-                }
-            }
-
-            return GetConfiguredRootUrl(host);
-        }
-
-        private static string GetFirstUrl(string urls)
-        {
-            var separatorIndex = urls.IndexOf(';');
-            if (separatorIndex >= 0)
-            {
-                urls = urls.Substring(0, separatorIndex);
-            }
-
-            return urls.TrimEnd('/');
-        }
-
-        private static string GetUrlWithNewPort(string rootUrl)
-        {
-            // Avoid UriBuilder here: ASP.NET Core wildcard hosts (http://*:5000, http://+:5000)
-            // are not valid Uri hosts and would throw UriFormatException. Since the retry always
-            // rebinds to 127.0.0.1 with an OS-assigned port, only the scheme needs to be preserved.
-            var schemeSeparator = rootUrl.IndexOf("://", StringComparison.Ordinal);
-            var scheme = schemeSeparator > 0 ? rootUrl.Substring(0, schemeSeparator) : "http";
-            return $"{scheme}://127.0.0.1:0";
-        }
-
-        private static bool IsAddressInUseException(Exception exception)
-        {
-            while (exception != null)
-            {
-                if (exception is SocketException socketException && socketException.SocketErrorCode == SocketError.AddressAlreadyInUse)
-                {
-                    return true;
-                }
-
-                if (exception.Message.IndexOf("address already in use", StringComparison.OrdinalIgnoreCase) >= 0)
-                {
-                    return true;
-                }
-
-                exception = exception.InnerException;
-            }
-
-            return false;
-        }
-
-        public static IHostBuilder CreateHostBuilder(string[] args, string rootUrl = null) =>
-            Host.CreateDefaultBuilder(GetArgsWithUrlsOverride(args, rootUrl))
+        public static IHostBuilder CreateHostBuilder(string[] args) =>
+            Host.CreateDefaultBuilder(args)
                 .ConfigureWebHostDefaults(webBuilder =>
                 {
                     webBuilder.UseStartup<Startup>();
@@ -249,34 +139,13 @@ public static IHostBuilder CreateHostBuilder(string[] args, string rootUrl = nul
                     }
                 });
 
-        private static string[] GetArgsWithUrlsOverride(string[] args, string rootUrl)
+        private static string GetBoundRootUrl(IHost host)
         {
-            if (string.IsNullOrEmpty(rootUrl))
-            {
-                return args;
-            }
-
-            var effectiveArgs = new List<string>(args.Length + 2);
-            for (var i = 0; i < args.Length; i++)
-            {
-                var arg = args[i];
-                if (string.Equals(arg, "--urls", StringComparison.OrdinalIgnoreCase))
-                {
-                    i++;
-                    continue;
-                }
-
-                if (arg.StartsWith("--urls=", StringComparison.OrdinalIgnoreCase))
-                {
-                    continue;
-                }
-
-                effectiveArgs.Add(arg);
-            }
-
-            effectiveArgs.Add("--urls");
-            effectiveArgs.Add(rootUrl);
-            return effectiveArgs.ToArray();
+            // Read the address Kestrel actually bound. Avoids race conditions where the caller
+            // passes a pre-picked port that another process grabs before we bind.
+            var server = (IServer)host.Services.GetService(typeof(IServer));
+            var address = server?.Features.Get<IServerAddressesFeature>()?.Addresses.FirstOrDefault();
+            return string.IsNullOrEmpty(address) ? "http://localhost:5000" : address.TrimEnd('/');
         }
 
         private static void ParseCommandLine(string[] args, out bool disableLogs, out TimeSpan timeout, out int iterations, out Scenario scenario, out int nbIdleThreads)

profiler/test/Datadog.Profiler.IntegrationTests/Helpers/TestApplicationRunner.cs

@@ -7,6 +7,7 @@
 using System.Collections.Specialized;
 using System.Diagnostics;
 using System.IO;
+using System.Text.RegularExpressions;
 using Xunit;
 using Xunit.Abstractions;
 using Xunit.Sdk;
@@ -151,10 +152,9 @@ private string GetApplicationPath()
                 throw new Exception($"Unable to find executing assembly at {applicationPath}");
             }
 
-            // Look for a free open port to pass to the ASP.NET Core applications
-            // that accept --urls on their command line
-            _appListenerPort = $"http://localhost:{TcpPortProvider.GetOpenPort()}";
-            var arguments = $"--timeout {TestDurationInSeconds} --urls {_appListenerPort}";
+            // Use port 0 so Kestrel binds an OS-assigned free port and avoids the TOCTOU race
+            // of pre-picking a port. The actual bound URL is parsed from stdout after the run.
+            var arguments = $"--timeout {TestDurationInSeconds} --urls http://localhost:0";
             if (!string.IsNullOrEmpty(_commandLine))
             {
                 arguments += $" {_commandLine}";
@@ -195,6 +195,7 @@ private void RunTest(MockDatadogAgent agent)
             var standardOutput = processHelper.StandardOutput;
             var errorOutput = processHelper.ErrorOutput;
             ProcessOutput = standardOutput;
+            _appListenerPort = ExtractListeningUrl(standardOutput);
 
             if (!ranToCompletion)
             {
@@ -251,5 +252,18 @@ private void SetEnvironmentVariables(StringDictionary environmentVariables, Mock
         {
             Environment.PopulateEnvironmentVariables(environmentVariables, agent, ProfilingExportsIntervalInSeconds, ServiceName);
         }
+
+        // Matches BuggyBits ("Listening to http://...") and ASP.NET Core's default Kestrel
+        // startup log ("Now listening on: http://..."). Returns null if neither is found.
+        private static string ExtractListeningUrl(string output)
+        {
+            if (string.IsNullOrEmpty(output))
+            {
+                return null;
+            }
+
+            var match = Regex.Match(output, @"(?:Listening to|Now listening on:)\s+(http://\S+)");
+            return match.Success ? match.Groups[1].Value.TrimEnd('/') : null;
+        }
     }
 }