[vs17.14] Add check for node shut down event and reorder the sequence of shutdown in OutOfProcNode (#12235)

Author: YuliiaKovalova

eng/Versions.props

@@ -2,7 +2,7 @@
 <!-- Copyright (c) .NET Foundation and contributors. All rights reserved. Licensed under the MIT license. See License.txt in the project root for full license information. -->
 <Project>
   <PropertyGroup>
-    <VersionPrefix>17.14.18</VersionPrefix><DotNetFinalVersionKind>release</DotNetFinalVersionKind>
+    <VersionPrefix>17.14.19</VersionPrefix><DotNetFinalVersionKind>release</DotNetFinalVersionKind>
     <PackageValidationBaselineVersion>17.13.9</PackageValidationBaselineVersion>
     <AssemblyVersion>15.1.0.0</AssemblyVersion>
     <PreReleaseVersionLabel>servicing</PreReleaseVersionLabel>

src/Build.UnitTests/BackEnd/MockSdkResolverService.cs

@@ -16,6 +16,8 @@ internal sealed class MockSdkResolverService : IBuildComponent, ISdkResolverServ
     {
         public Action<INodePacket> SendPacket { get; }
 
+        public bool IsNodeShutDown { get; set; }
+
         public void ClearCache(int submissionId)
         {
         }
@@ -33,8 +35,6 @@ public void InitializeComponent(IBuildComponentHost host)
         {
         }
 
-        public void ShutdownComponent()
-        {
-        }
+        public void ShutdownComponent() => IsNodeShutDown = true;
     }
 }

src/Build/BackEnd/BuildManager/BuildManager.cs

@@ -779,6 +779,7 @@ private static void AttachDebugger()
         /// </summary>
         public void CancelAllSubmissions()
         {
+            MSBuildEventSource.Log.CancelSubmissionsStart();
             CancelAllSubmissions(true);
         }
 

src/Build/BackEnd/Components/Communications/SerializationContractInitializer.cs

@@ -25,6 +25,7 @@ private static void RegisterExceptions()
             BuildExceptionSerializationHelper.InitializeSerializationContract(
                 new(typeof(GenericBuildTransferredException), (msg, inner) => new GenericBuildTransferredException(msg, inner)),
                 new(typeof(SdkResolverException), (msg, inner) => new SdkResolverException(msg, inner)),
+                new(typeof(SdkResolverServiceException), (msg, inner) => new SdkResolverServiceException(msg, inner)),
                 new(typeof(BuildAbortedException), BuildAbortedException.CreateFromRemote),
                 new(typeof(CircularDependencyException), (msg, inner) => new CircularDependencyException(msg, inner)),
                 new(typeof(InternalLoggerException), (msg, inner) => new InternalLoggerException(msg, inner)),

src/Build/BackEnd/Components/SdkResolution/HostedSdkResolverServiceBase.cs

@@ -5,6 +5,7 @@
 using System.Threading;
 using Microsoft.Build.BackEnd.Logging;
 using Microsoft.Build.Construction;
+using Microsoft.Build.Eventing;
 using Microsoft.Build.Framework;
 
 #nullable disable
@@ -29,6 +30,9 @@ internal abstract class HostedSdkResolverServiceBase : IBuildComponent, INodePac
         /// <inheritdoc cref="ISdkResolverService.SendPacket"/>
         public Action<INodePacket> SendPacket { get; set; }
 
+        /// <inheritdoc cref="ISdkResolverService.IsNodeShutDown"/>
+        public bool IsNodeShutDown { get; set; }
+
         /// <inheritdoc cref="ISdkResolverService.ClearCache"/>
         public virtual void ClearCache(int submissionId)
         {
@@ -55,6 +59,8 @@ public virtual void InitializeComponent(IBuildComponentHost host)
         public virtual void ShutdownComponent()
         {
             ShutdownEvent.Set();
+            IsNodeShutDown = true;
+            MSBuildEventSource.Log.SdkResolverServiceNodeShutDownSet();
         }
     }
 }

src/Build/BackEnd/Components/SdkResolution/ISdkResolverService.cs

@@ -20,6 +20,11 @@ internal interface ISdkResolverService
         /// </summary>
         Action<INodePacket> SendPacket { get; }
 
+        /// <summary>
+        /// Gets or sets if shutdown event was already triggered.
+        /// </summary>
+        bool IsNodeShutDown { get; set; }
+
         /// <summary>
         /// Clears the cache for the specified build submission ID.
         /// </summary>

src/Build/BackEnd/Components/SdkResolution/OutOfProcNodeSdkResolverService.cs

@@ -66,6 +66,11 @@ public override void PacketReceived(int node, INodePacket packet)
         /// <inheritdoc cref="ISdkResolverService.ResolveSdk"/>
         public override SdkResult ResolveSdk(int submissionId, SdkReference sdk, LoggingContext loggingContext, ElementLocation sdkReferenceLocation, string solutionPath, string projectPath, bool interactive, bool isRunningInVisualStudio, bool failOnUnresolvedSdk)
         {
+            if (IsNodeShutDown)
+            {
+                throw new SdkResolverServiceException("SDK could not be resolved by the SDK resolver because the worker node was shut down.");
+            }
+
             bool wasResultCached = true;
 
             MSBuildEventSource.Log.OutOfProcSdkResolverServiceRequestSdkPathFromMainNodeStart(submissionId, sdk.Name, solutionPath, projectPath);
@@ -128,6 +133,11 @@ private SdkResult RequestSdkPathFromMainNode(int submissionId, SdkReference sdk,
             // Wait for either the response or a shutdown event.  Either event means this thread should return
             WaitHandle.WaitAny([_responseReceivedEvent, ShutdownEvent]);
 
+            if (_lastResponse == null)
+            {
+                throw new SdkResolverServiceException("SDK could not be resolved by the SDK resolver.");
+            }
+
             // Keep track of the element location of the reference
             _lastResponse.ElementLocation = sdkReferenceLocation;
 

src/Build/BackEnd/Components/SdkResolution/SdkResolverService.cs

@@ -78,6 +78,9 @@ public SdkResolverService()
         /// <inheritdoc cref="ISdkResolverService.SendPacket"/>
         public Action<INodePacket> SendPacket { get; }
 
+        /// <inheritdoc cref="ISdkResolverService.IsNodeShutDown"/>
+        public bool IsNodeShutDown { get; set; }
+
         /// <summary>
         /// Determines if the <see cref="SdkReference"/> is the same as the specified version.  If the <paramref name="sdk"/> object has <code>null</code> for the version,
         /// this method will always return true since <code>null</code> can match any version.

src/Build/BackEnd/Components/SdkResolution/SdkResolverServiceException.cs

@@ -0,0 +1,26 @@
+// Licensed to the .NET Foundation under one or more agreements.
+// The .NET Foundation licenses this file to you under the MIT license.
+
+using System;
+using Microsoft.Build.Framework.BuildException;
+
+#nullable disable
+
+namespace Microsoft.Build.BackEnd.SdkResolution
+{
+    /// <summary>
+    /// Represents an exception that occurs when an SdkResolverService throws an unhandled exception.
+    /// </summary>
+    public class SdkResolverServiceException : BuildExceptionBase
+    {
+        public SdkResolverServiceException(string message, params string[] args)
+            : base(string.Format(message, args))
+        {
+        }
+
+        // Do not remove - used by BuildExceptionSerializationHelper
+        internal SdkResolverServiceException(string message, Exception inner)
+            : base(message, inner)
+        { }
+    }
+}

src/Build/BackEnd/Node/OutOfProcNode.cs

@@ -14,6 +14,7 @@
 using Microsoft.Build.BackEnd.Logging;
 using Microsoft.Build.BackEnd.SdkResolution;
 using Microsoft.Build.Evaluation;
+using Microsoft.Build.Eventing;
 using Microsoft.Build.FileAccesses;
 using Microsoft.Build.Framework;
 using Microsoft.Build.Internal;
@@ -444,6 +445,14 @@ private NodeEngineShutdownReason HandleShutdown(out Exception exception)
         {
             CommunicationsUtilities.Trace("Shutting down with reason: {0}, and exception: {1}.", _shutdownReason, _shutdownException);
 
+            MSBuildEventSource.Log.OutOfProcNodeShutDownStart();
+
+            // Signal the SDK resolver service to shutdown
+            // It should be shut down first so all the requests for SDK resolution are discarded.
+            // Otherwise worker node might stuck in a situation where _buildRequestEngine.CleanupForBuild() waiting for the SDK resolver service response from the main node
+            // and it never comes since we don't listen to _packetReceivedEvent in the middle of the _shutdownEvent.
+            ((IBuildComponent)_sdkResolverService).ShutdownComponent();
+
             // Clean up the engine
             if (_buildRequestEngine != null && _buildRequestEngine.Status != BuildRequestEngineStatus.Uninitialized)
             {
@@ -455,9 +464,6 @@ private NodeEngineShutdownReason HandleShutdown(out Exception exception)
                 }
             }
 
-            // Signal the SDK resolver service to shutdown
-            ((IBuildComponent)_sdkResolverService).ShutdownComponent();
-
             // Dispose of any build registered objects
             IRegisteredTaskObjectCache objectCache = (IRegisteredTaskObjectCache)(_componentFactories.GetComponent(BuildComponentType.RegisteredTaskObjectCache));
             objectCache.DisposeCacheObjects(RegisteredTaskObjectLifetime.Build);