[dotnet watch] Agent improvements (#45997)

Author: tmat

src/BuiltInTools/DotNetDeltaApplier/Microsoft.Extensions.DotNetDeltaApplier.csproj

@@ -13,10 +13,6 @@
     <Nullable>enable</Nullable>
   </PropertyGroup>
 
-  <ItemGroup>
-    <Compile Include="..\dotnet-watch\EnvironmentVariables_StartupHook.cs" Link="EnvironmentVariables_StartupHook.cs" />
-  </ItemGroup>
-
   <ItemGroup>
     <InternalsVisibleTo Include="Microsoft.Extensions.DotNetDeltaApplier.Tests" />
   </ItemGroup>

src/BuiltInTools/DotNetDeltaApplier/StartupHook.cs

@@ -1,17 +1,19 @@
 // Licensed to the .NET Foundation under one or more agreements.
 // The .NET Foundation licenses this file to you under the MIT license.
 
+using System.Diagnostics;
 using System.IO.Pipes;
-using Microsoft.DotNet.Watch;
 using Microsoft.DotNet.HotReload;
 
 /// <summary>
 /// The runtime startup hook looks for top-level type named "StartupHook".
 /// </summary>
 internal sealed class StartupHook
 {
-    private static readonly bool s_logToStandardOutput = Environment.GetEnvironmentVariable(EnvironmentVariables.Names.HotReloadDeltaClientLogMessages) == "1";
-    private static readonly string s_namedPipeName = Environment.GetEnvironmentVariable(EnvironmentVariables.Names.DotnetWatchHotReloadNamedPipeName);
+    private const int ConnectionTimeoutMS = 5000;
+
+    private static readonly bool s_logToStandardOutput = Environment.GetEnvironmentVariable(AgentEnvironmentVariables.HotReloadDeltaClientLogMessages) == "1";
+    private static readonly string s_namedPipeName = Environment.GetEnvironmentVariable(AgentEnvironmentVariables.DotNetWatchHotReloadNamedPipeName);
 
     /// <summary>
     /// Invoked by the runtime when the containing assembly is listed in DOTNET_STARTUP_HOOKS.
@@ -22,65 +24,148 @@ public static void Initialize()
 
         Log($"Loaded into process: {processPath}");
 
-        ClearHotReloadEnvironmentVariables();
+        HotReloadAgent.ClearHotReloadEnvironmentVariables(typeof(StartupHook));
+
+        Log($"Connecting to hot-reload server");
 
-        _ = Task.Run(async () =>
+        // Connect to the pipe synchronously.
+        //
+        // If a debugger is attached and there is a breakpoint in the startup code connecting asynchronously would
+        // set up a race between this code connecting to the server, and the breakpoint being hit. If the breakpoint
+        // hits first, applying changes will throw an error that the client is not connected.
+        //
+        // Updates made before the process is launched need to be applied before loading the affected modules. 
+
+        var pipeClient = new NamedPipeClientStream(".", s_namedPipeName, PipeDirection.InOut, PipeOptions.CurrentUserOnly | PipeOptions.Asynchronous);
+        try
+        {
+            pipeClient.Connect(ConnectionTimeoutMS);
+            Log("Connected.");
+        }
+        catch (TimeoutException)
         {
-            Log($"Connecting to hot-reload server");
+            Log($"Failed to connect in {ConnectionTimeoutMS}ms.");
+            return;
+        }
 
-            const int TimeOutMS = 5000;
+        var agent = new HotReloadAgent();
+        try
+        {
+            // block until initialization completes:
+            InitializeAsync(pipeClient, agent, CancellationToken.None).GetAwaiter().GetResult();
 
-            using var pipeClient = new NamedPipeClientStream(".", s_namedPipeName, PipeDirection.InOut, PipeOptions.CurrentUserOnly | PipeOptions.Asynchronous);
-            try
-            {
-                await pipeClient.ConnectAsync(TimeOutMS);
-                Log("Connected.");
-            }
-            catch (TimeoutException)
-            {
-                Log($"Failed to connect in {TimeOutMS}ms.");
-                return;
-            }
+            // fire and forget:
+            _ = ReceiveAndApplyUpdatesAsync(pipeClient, agent, initialUpdates: false, CancellationToken.None);
+        }
+        catch (Exception ex)
+        {
+            Log(ex.Message);
+            pipeClient.Dispose();
+        }
+    }
 
-            using var agent = new HotReloadAgent();
-            try
-            {
-                agent.Reporter.Report("Writing capabilities: " + agent.Capabilities, AgentMessageSeverity.Verbose);
+    private static async ValueTask InitializeAsync(NamedPipeClientStream pipeClient, HotReloadAgent agent, CancellationToken cancellationToken)
+    {
+        agent.Reporter.Report("Writing capabilities: " + agent.Capabilities, AgentMessageSeverity.Verbose);
 
-                var initPayload = new ClientInitializationRequest(agent.Capabilities);
-                await initPayload.WriteAsync(pipeClient, CancellationToken.None);
+        var initPayload = new ClientInitializationResponse(agent.Capabilities);
+        await initPayload.WriteAsync(pipeClient, cancellationToken);
+
+        // Apply updates made before this process was launched to avoid executing unupdated versions of the affected modules.
+        await ReceiveAndApplyUpdatesAsync(pipeClient, agent, initialUpdates: true, cancellationToken);
+    }
 
-                while (pipeClient.IsConnected)
+    private static async Task ReceiveAndApplyUpdatesAsync(NamedPipeClientStream pipeClient, HotReloadAgent agent, bool initialUpdates, CancellationToken cancellationToken)
+    {
+        try
+        {
+            while (pipeClient.IsConnected)
+            {
+                var payloadType = (RequestType)await pipeClient.ReadByteAsync(cancellationToken);
+                switch (payloadType)
                 {
-                    var update = await ManagedCodeUpdateRequest.ReadAsync(pipeClient, CancellationToken.None);
-                    Log($"ResponseLoggingLevel = {update.ResponseLoggingLevel}");
-
-                    bool success;
-                    try
-                    {
-                        agent.ApplyDeltas(update.Deltas);
-                        success = true;
-                    }
-                    catch (Exception e)
-                    {
-                        agent.Reporter.Report($"The runtime failed to applying the change: {e.Message}", AgentMessageSeverity.Error);
-                        agent.Reporter.Report("Further changes won't be applied to this process.", AgentMessageSeverity.Warning);
-                        success = false;
-                    }
-
-                    var logEntries = agent.GetAndClearLogEntries(update.ResponseLoggingLevel);
-
-                    var response = new UpdateResponse(logEntries, success);
-                    await response.WriteAsync(pipeClient, CancellationToken.None);
+                    case RequestType.ManagedCodeUpdate:
+                        // Shouldn't get initial managed code updates when the debugger is attached.
+                        // The debugger itself applies these updates when launching process with the debugger attached.
+                        Debug.Assert(!Debugger.IsAttached);
+                        await ReadAndApplyManagedCodeUpdateAsync(pipeClient, agent, cancellationToken);
+                        break;
+
+                    case RequestType.StaticAssetUpdate:
+                        await ReadAndApplyStaticAssetUpdateAsync(pipeClient, agent, cancellationToken);
+                        break;
+
+                    case RequestType.InitialUpdatesCompleted when initialUpdates:
+                        return;
+
+                    default:
+                        // can't continue, the pipe content is in an unknown state
+                        Log($"Unexpected payload type: {payloadType}. Terminating agent.");
+                        return;
                 }
             }
-            catch (Exception e)
+        }
+        catch (Exception ex)
+        {
+            Log(ex.Message);
+        }
+        finally
+        {
+            if (!pipeClient.IsConnected)
+            {
+                await pipeClient.DisposeAsync();
+            }
+
+            if (!initialUpdates)
             {
-                Log(e.ToString());
+                agent.Dispose();
             }
+        }
+    }
+
+    private static async ValueTask ReadAndApplyManagedCodeUpdateAsync(
+        NamedPipeClientStream pipeClient,
+        HotReloadAgent agent,
+        CancellationToken cancellationToken)
+    {
+        var request = await ManagedCodeUpdateRequest.ReadAsync(pipeClient, cancellationToken);
 
-            Log("Stopped received delta updates. Server is no longer connected.");
-        });
+        bool success;
+        try
+        {
+            agent.ApplyDeltas(request.Deltas);
+            success = true;
+        }
+        catch (Exception e)
+        {
+            agent.Reporter.Report($"The runtime failed to applying the change: {e.Message}", AgentMessageSeverity.Error);
+            agent.Reporter.Report("Further changes won't be applied to this process.", AgentMessageSeverity.Warning);
+            success = false;
+        }
+
+        var logEntries = agent.GetAndClearLogEntries(request.ResponseLoggingLevel);
+
+        var response = new UpdateResponse(logEntries, success);
+        await response.WriteAsync(pipeClient, cancellationToken);
+    }
+
+    private static async ValueTask ReadAndApplyStaticAssetUpdateAsync(
+        NamedPipeClientStream pipeClient,
+        HotReloadAgent agent,
+        CancellationToken cancellationToken)
+    {
+        var request = await StaticAssetUpdateRequest.ReadAsync(pipeClient, cancellationToken);
+
+        agent.ApplyStaticAssetUpdate(new StaticAssetUpdate(request.AssemblyName, request.RelativePath, request.Contents, request.IsApplicationProject));
+
+        var logEntries = agent.GetAndClearLogEntries(request.ResponseLoggingLevel);
+
+        // Updating static asset only invokes ContentUpdate metadata update handlers.
+        // Failures of these handlers are reported to the log and ignored.
+        // Therefore, this request always succeeds.
+        var response = new UpdateResponse(logEntries, success: true);
+
+        await response.WriteAsync(pipeClient, cancellationToken);
     }
 
     public static bool IsMatchingProcess(string processPath, string targetProcessPath)
@@ -101,32 +186,6 @@ public static bool IsMatchingProcess(string processPath, string targetProcessPat
                string.Equals(processPath[..^4], targetProcessPath[..^4], comparison);
     }
 
-    internal static void ClearHotReloadEnvironmentVariables()
-    {
-        // Clear any hot-reload specific environment variables. This prevents child processes from being
-        // affected by the current app's hot reload settings. See https://github.com/dotnet/runtime/issues/58000
-
-        Environment.SetEnvironmentVariable(EnvironmentVariables.Names.DotnetStartupHooks,
-            RemoveCurrentAssembly(Environment.GetEnvironmentVariable(EnvironmentVariables.Names.DotnetStartupHooks)));
-
-        Environment.SetEnvironmentVariable(EnvironmentVariables.Names.DotnetWatchHotReloadNamedPipeName, "");
-        Environment.SetEnvironmentVariable(EnvironmentVariables.Names.HotReloadDeltaClientLogMessages, "");
-    }
-
-    internal static string RemoveCurrentAssembly(string environment)
-    {
-        if (environment is "")
-        {
-            return environment;
-        }
-
-        var assemblyLocation = typeof(StartupHook).Assembly.Location;
-        var updatedValues = environment.Split(Path.PathSeparator, StringSplitOptions.RemoveEmptyEntries)
-            .Where(e => !string.Equals(e, assemblyLocation, StringComparison.OrdinalIgnoreCase));
-
-        return string.Join(Path.PathSeparator, updatedValues);
-    }
-
     private static void Log(string message)
     {
         if (s_logToStandardOutput)

src/BuiltInTools/HotReloadAgent.PipeRpc/NamedPipeContract.cs

@@ -12,26 +12,29 @@ namespace Microsoft.DotNet.HotReload;
 
 internal interface IRequest
 {
+    RequestType Type { get; }
     ValueTask WriteAsync(Stream stream, CancellationToken cancellationToken);
 }
 
+internal interface IUpdateRequest : IRequest
+{
+}
+
 internal enum RequestType
 {
     ManagedCodeUpdate = 1,
     StaticAssetUpdate = 2,
     InitialUpdatesCompleted = 3,
 }
 
-internal readonly struct ManagedCodeUpdateRequest(IReadOnlyList<UpdateDelta> deltas, ResponseLoggingLevel responseLoggingLevel) : IRequest
+internal readonly struct ManagedCodeUpdateRequest(IReadOnlyList<UpdateDelta> deltas, ResponseLoggingLevel responseLoggingLevel) : IUpdateRequest
 {
     private const byte Version = 4;
 
     public IReadOnlyList<UpdateDelta> Deltas { get; } = deltas;
     public ResponseLoggingLevel ResponseLoggingLevel { get; } = responseLoggingLevel;
+    public RequestType Type => RequestType.ManagedCodeUpdate;
 
-    /// <summary>
-    /// Called by the dotnet-watch.
-    /// </summary>
     public async ValueTask WriteAsync(Stream stream, CancellationToken cancellationToken)
     {
         await stream.WriteAsync(Version, cancellationToken);
@@ -49,9 +52,6 @@ public async ValueTask WriteAsync(Stream stream, CancellationToken cancellationT
         await stream.WriteAsync((byte)ResponseLoggingLevel, cancellationToken);
     }
 
-    /// <summary>
-    /// Called by delta applier.
-    /// </summary>
     public static async ValueTask<ManagedCodeUpdateRequest> ReadAsync(Stream stream, CancellationToken cancellationToken)
     {
         var version = await stream.ReadByteAsync(cancellationToken);
@@ -114,25 +114,19 @@ public async ValueTask WriteAsync(Stream stream, CancellationToken cancellationT
     }
 }
 
-internal readonly struct ClientInitializationRequest(string capabilities) : IRequest
+internal readonly struct ClientInitializationResponse(string capabilities) 
 {
     private const byte Version = 0;
 
     public string Capabilities { get; } = capabilities;
 
-    /// <summary>
-    /// Called by delta applier.
-    /// </summary>
     public async ValueTask WriteAsync(Stream stream, CancellationToken cancellationToken)
     {
         await stream.WriteAsync(Version, cancellationToken);
         await stream.WriteAsync(Capabilities, cancellationToken);
     }
 
-    /// <summary>
-    /// Called by dotnet-watch.
-    /// </summary>
-    public static async ValueTask<ClientInitializationRequest> ReadAsync(Stream stream, CancellationToken cancellationToken)
+    public static async ValueTask<ClientInitializationResponse> ReadAsync(Stream stream, CancellationToken cancellationToken)
     {
         var version = await stream.ReadByteAsync(cancellationToken);
         if (version != Version)
@@ -141,22 +135,26 @@ public static async ValueTask<ClientInitializationRequest> ReadAsync(Stream stre
         }
 
         var capabilities = await stream.ReadStringAsync(cancellationToken);
-        return new ClientInitializationRequest(capabilities);
+        return new ClientInitializationResponse(capabilities);
     }
 }
 
 internal readonly struct StaticAssetUpdateRequest(
     string assemblyName,
     string relativePath,
     byte[] contents,
-    bool isApplicationProject) : IRequest
+    bool isApplicationProject,
+    ResponseLoggingLevel responseLoggingLevel) : IUpdateRequest
 {
-    private const byte Version = 1;
+    private const byte Version = 2;
 
     public string AssemblyName { get; } = assemblyName;
     public bool IsApplicationProject { get; } = isApplicationProject;
     public string RelativePath { get; } = relativePath;
     public byte[] Contents { get; } = contents;
+    public ResponseLoggingLevel ResponseLoggingLevel { get; } = responseLoggingLevel;
+
+    public RequestType Type => RequestType.StaticAssetUpdate;
 
     public async ValueTask WriteAsync(Stream stream, CancellationToken cancellationToken)
     {
@@ -165,6 +163,7 @@ public async ValueTask WriteAsync(Stream stream, CancellationToken cancellationT
         await stream.WriteAsync(IsApplicationProject, cancellationToken);
         await stream.WriteAsync(RelativePath, cancellationToken);
         await stream.WriteByteArrayAsync(Contents, cancellationToken);
+        await stream.WriteAsync((byte)ResponseLoggingLevel, cancellationToken);
     }
 
     public static async ValueTask<StaticAssetUpdateRequest> ReadAsync(Stream stream, CancellationToken cancellationToken)
@@ -176,14 +175,16 @@ public static async ValueTask<StaticAssetUpdateRequest> ReadAsync(Stream stream,
         }
 
         var assemblyName = await stream.ReadStringAsync(cancellationToken);
-        var isAppProject = await stream.ReadBooleanAsync(cancellationToken);
+        var isApplicationProject = await stream.ReadBooleanAsync(cancellationToken);
         var relativePath = await stream.ReadStringAsync(cancellationToken);
         var contents = await stream.ReadByteArrayAsync(cancellationToken);
+        var responseLoggingLevel = (ResponseLoggingLevel)await stream.ReadByteAsync(cancellationToken);
 
         return new StaticAssetUpdateRequest(
             assemblyName: assemblyName,
             relativePath: relativePath,
             contents: contents,
-            isApplicationProject: isAppProject);
+            isApplicationProject,
+            responseLoggingLevel);
     }
 }