Struct handshake

src/Build.UnitTests/Utilities_Tests.cs

@@ -64,26 +64,6 @@ public void GetTextFromTextNodeWithXmlComment7()
             // Should get XML; note space after x added
             Assert.Equal("<!-- bar; baz; --><!-- bar --><x />", xmlContents);
         }
-
-        [Fact]
-        public void HandshakesDiffer()
-        {
-            int numHandshakeOptions = (int)Math.Pow(2, Enum.GetNames(typeof(HandshakeOptions)).Length - 1);
-            Dictionary<long, int> handshakes = new Dictionary<long, int>();
-            for (int i = 0; i < numHandshakeOptions; i++)
-            {
-                long nextKey = CommunicationsUtilities.GetHostHandshake((HandshakeOptions)i);
-                if (handshakes.TryGetValue(nextKey, out int collision))
-                {
-                    _output.WriteLine("There was a collision between {0} and {1}.", collision, i);
-                }
-                else
-                {
-                    handshakes.Add(nextKey, i);
-                }
-            }
-            handshakes.Count.ShouldBe(numHandshakeOptions, "two or more combinations of handshake options hashed to the same value");
-        }
     }
 
     public class UtilitiesTestReadOnlyLoad : UtilitiesTest

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

@@ -1,6 +1,7 @@
 // Copyright (c) Microsoft. All rights reserved.
 // Licensed under the MIT license. See LICENSE file in the project root for full license information.
 
+using Microsoft.Build.Internal;
 using Microsoft.Build.Shared;
 
 namespace Microsoft.Build.BackEnd
@@ -50,17 +51,9 @@ internal NodeEndpointOutOfProc(
         /// <summary>
         /// Returns the host handshake for this node endpoint
         /// </summary>
-        protected override long GetHostHandshake()
+        protected override Handshake GetHandshake()
         {
-            return NodeProviderOutOfProc.GetHostHandshake(_enableReuse, _lowPriority);
-        }
-
-        /// <summary>
-        /// Returns the client handshake for this node endpoint
-        /// </summary>
-        protected override long GetClientHandshake()
-        {
-            return NodeProviderOutOfProc.GetClientHandshake(_enableReuse, _lowPriority);
+            return new Handshake(CommunicationsUtilities.GetHandshakeOptions(taskHost: false, is64Bit: EnvironmentUtilities.Is64BitProcess, nodeReuse: _enableReuse, lowPriority: _lowPriority));
         }
 
         #region Structs

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

@@ -63,19 +63,10 @@ public int AvailableNodes
         /// </summary>
         /// <param name="enableNodeReuse">Is reuse of build nodes allowed?</param>
         /// <param name="enableLowPriority">Is the build running at low priority?</param>
-        internal static long GetHostHandshake(bool enableNodeReuse, bool enableLowPriority)
+        internal static Handshake GetHandshake(bool enableNodeReuse, bool enableLowPriority)
         {
             CommunicationsUtilities.Trace("MSBUILDNODEHANDSHAKESALT=\"{0}\", msbuildDirectory=\"{1}\", enableNodeReuse={2}, enableLowPriority={3}", Traits.MSBuildNodeHandshakeSalt, BuildEnvironmentHelper.Instance.MSBuildToolsDirectory32, enableNodeReuse, enableLowPriority);
-            return CommunicationsUtilities.GetHostHandshake(CommunicationsUtilities.GetHandshakeOptions(taskHost: false, nodeReuse: enableNodeReuse, lowPriority: enableLowPriority, is64Bit: EnvironmentUtilities.Is64BitProcess));
-        }
-
-        /// <summary>
-        /// Magic number sent by the client to the host during the handshake.
-        /// Munged version of the host handshake.
-        /// </summary>
-        internal static long GetClientHandshake(bool enableNodeReuse, bool enableLowPriority)
-        {
-            return CommunicationsUtilities.GetClientHandshake(CommunicationsUtilities.GetHandshakeOptions(false, nodeReuse: enableNodeReuse, lowPriority: enableLowPriority, is64Bit: EnvironmentUtilities.Is64BitProcess));
+            return new Handshake(CommunicationsUtilities.GetHandshakeOptions(taskHost: false, nodeReuse: enableNodeReuse, lowPriority: enableLowPriority, is64Bit: EnvironmentUtilities.Is64BitProcess));
         }
 
         /// <summary>
@@ -100,8 +91,8 @@ public bool CreateNode(int nodeId, INodePacketFactory factory, NodeConfiguration
             // Make it here.
             CommunicationsUtilities.Trace("Starting to acquire a new or existing node to establish node ID {0}...", nodeId);
 
-            long hostHandShake = NodeProviderOutOfProc.GetHostHandshake(ComponentHost.BuildParameters.EnableNodeReuse, ComponentHost.BuildParameters.LowPriority);
-            NodeContext context = GetNode(null, commandLineArgs, nodeId, factory, hostHandShake, NodeProviderOutOfProc.GetClientHandshake(ComponentHost.BuildParameters.EnableNodeReuse, ComponentHost.BuildParameters.LowPriority), NodeContextTerminated);
+            Handshake hostHandshake = new Handshake(CommunicationsUtilities.GetHandshakeOptions(taskHost: false, nodeReuse: ComponentHost.BuildParameters.EnableNodeReuse, lowPriority: ComponentHost.BuildParameters.LowPriority, is64Bit: EnvironmentUtilities.Is64BitProcess));
+            NodeContext context = GetNode(null, commandLineArgs, nodeId, factory, hostHandshake, NodeContextTerminated);
 
             if (null != context)
             {

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

@@ -125,12 +125,12 @@ protected void ShutdownAllNodes(bool nodeReuse, NodeContextTerminateDelegate ter
                 int timeout = 30;
 
                 // Attempt to connect to the process with the handshake without low priority.
-                Stream nodeStream = TryConnectToProcess(nodeProcess.Id, timeout, NodeProviderOutOfProc.GetHostHandshake(nodeReuse, false), NodeProviderOutOfProc.GetClientHandshake(nodeReuse, false));
+                Stream nodeStream = TryConnectToProcess(nodeProcess.Id, timeout, NodeProviderOutOfProc.GetHandshake(nodeReuse, false));
 
                 if (null == nodeStream)
                 {
                     // If we couldn't connect attempt to connect to the process with the handshake including low priority.
-                    nodeStream = TryConnectToProcess(nodeProcess.Id, timeout, NodeProviderOutOfProc.GetHostHandshake(nodeReuse, true), NodeProviderOutOfProc.GetClientHandshake(nodeReuse, true));
+                    nodeStream = TryConnectToProcess(nodeProcess.Id, timeout, NodeProviderOutOfProc.GetHandshake(nodeReuse, true));
                 }
 
                 if (null != nodeStream)
@@ -148,7 +148,7 @@ protected void ShutdownAllNodes(bool nodeReuse, NodeContextTerminateDelegate ter
         /// Finds or creates a child process which can act as a node.
         /// </summary>
         /// <returns>The pipe stream representing the node.</returns>
-        protected NodeContext GetNode(string msbuildLocation, string commandLineArgs, int nodeId, INodePacketFactory factory, long hostHandshake, long clientHandshake, NodeContextTerminateDelegate terminateNode)
+        protected NodeContext GetNode(string msbuildLocation, string commandLineArgs, int nodeId, INodePacketFactory factory, Handshake hostHandshake, NodeContextTerminateDelegate terminateNode)
         {
 #if DEBUG
             if (Execution.BuildManager.WaitForDebugger)
@@ -188,7 +188,7 @@ protected NodeContext GetNode(string msbuildLocation, string commandLineArgs, in
                     }
 
                     // Get the full context of this inspection so that we can always skip this process when we have the same taskhost context
-                    string nodeLookupKey = GetProcessesToIgnoreKey(hostHandshake, clientHandshake, nodeProcess.Id);
+                    string nodeLookupKey = GetProcessesToIgnoreKey(hostHandshake, nodeProcess.Id);
                     if (_processesToIgnore.Contains(nodeLookupKey))
                     {
                         continue;
@@ -198,7 +198,7 @@ protected NodeContext GetNode(string msbuildLocation, string commandLineArgs, in
                     _processesToIgnore.Add(nodeLookupKey);
 
                     // Attempt to connect to each process in turn.
-                    Stream nodeStream = TryConnectToProcess(nodeProcess.Id, 0 /* poll, don't wait for connections */, hostHandshake, clientHandshake);
+                    Stream nodeStream = TryConnectToProcess(nodeProcess.Id, 0 /* poll, don't wait for connections */, hostHandshake);
                     if (nodeStream != null)
                     {
                         // Connection successful, use this node.
@@ -242,14 +242,14 @@ protected NodeContext GetNode(string msbuildLocation, string commandLineArgs, in
 
                 // Create the node process
                 int msbuildProcessId = LaunchNode(msbuildLocation, commandLineArgs);
-                _processesToIgnore.Add(GetProcessesToIgnoreKey(hostHandshake, clientHandshake, msbuildProcessId));
+                _processesToIgnore.Add(GetProcessesToIgnoreKey(hostHandshake, msbuildProcessId));
 
                 // Note, when running under IMAGEFILEEXECUTIONOPTIONS registry key to debug, the process ID
                 // gotten back from CreateProcess is that of the debugger, which causes this to try to connect
                 // to the debugger process. Instead, use MSBUILDDEBUGONSTART=1
 
                 // Now try to connect to it.
-                Stream nodeStream = TryConnectToProcess(msbuildProcessId, TimeoutForNewNodeCreation, hostHandshake, clientHandshake);
+                Stream nodeStream = TryConnectToProcess(msbuildProcessId, TimeoutForNewNodeCreation, hostHandshake);
                 if (nodeStream != null)
                 {
                     // Connection successful, use this node.
@@ -292,9 +292,9 @@ protected NodeContext GetNode(string msbuildLocation, string commandLineArgs, in
         /// Generate a string from task host context and the remote process to be used as key to lookup processes we have already
         /// attempted to connect to or are already connected to
         /// </summary>
-        private string GetProcessesToIgnoreKey(long hostHandshake, long clientHandshake, int nodeProcessId)
+        private string GetProcessesToIgnoreKey(Handshake hostHandshake, int nodeProcessId)
         {
-            return hostHandshake.ToString(CultureInfo.InvariantCulture) + "|" + clientHandshake.ToString(CultureInfo.InvariantCulture) + "|" + nodeProcessId.ToString(CultureInfo.InvariantCulture);
+            return hostHandshake.ToString() + "|" + nodeProcessId.ToString(CultureInfo.InvariantCulture);
         }
 
 #if !FEATURE_PIPEOPTIONS_CURRENTUSERONLY
@@ -321,7 +321,7 @@ private void ValidateRemotePipeSecurityOnWindows(NamedPipeClientStream nodeStrea
         /// <summary>
         /// Attempts to connect to the specified process.
         /// </summary>
-        private Stream TryConnectToProcess(int nodeProcessId, int timeout, long hostHandshake, long clientHandshake)
+        private Stream TryConnectToProcess(int nodeProcessId, int timeout, Handshake handshake)
         {
             // Try and connect to the process.
             string pipeName = NamedPipeUtil.GetPipeNameOrPath("MSBuild" + nodeProcessId);
@@ -351,22 +351,23 @@ private Stream TryConnectToProcess(int nodeProcessId, int timeout, long hostHand
                 }
 #endif
 
-                CommunicationsUtilities.Trace("Writing handshake to pipe {0}", pipeName);
-                nodeStream.WriteLongForHandshake(hostHandshake);
+                int[] handshakeComponents = handshake.RetrieveHandshakeComponents();
+                for (int i = 0; i < handshakeComponents.Length; i++)
+                {
+                    CommunicationsUtilities.Trace("Writing handshake part {0} to pipe {1}", i, pipeName);
+                    nodeStream.WriteIntForHandshake(handshakeComponents[i]);
+                }
+
+                // This indicates that we have finished all the parts of our handshake; hopefully the endpoint has as well.
+                nodeStream.WriteEndOfHandshakeSignal();
 
                 CommunicationsUtilities.Trace("Reading handshake from pipe {0}", pipeName);
+
 #if NETCOREAPP2_1 || MONO
-                long handshake = nodeStream.ReadLongForHandshake(timeout);
+                nodeStream.ReadEndOfHandshakeSignal(true, timeout);
 #else
-                long handshake = nodeStream.ReadLongForHandshake();
+                nodeStream.ReadEndOfHandshakeSignal(true);
 #endif
-
-                if (handshake != clientHandshake)
-                {
-                    CommunicationsUtilities.Trace("Handshake failed. Received {0} from client not {1}. Probably the client is a different MSBuild build.", handshake, clientHandshake);
-                    throw new InvalidOperationException();
-                }
-
                 // We got a connection.
                 CommunicationsUtilities.Trace("Successfully connected to pipe {0}...!", pipeName);
                 return nodeStream;

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

@@ -200,16 +200,7 @@ public void ShutdownConnectedNodes(bool enableReuse)
         /// </summary>
         public void ShutdownAllNodes()
         {
-            bool nodeReuse = ComponentHost.BuildParameters.EnableNodeReuse;
-
-            // To avoid issues with mismatched priorities not shutting
-            // down all the nodes on exit, we will attempt to shutdown
-            // all matching notes with and without the priroity bit set.
-            // So precompute both versions of the handshake now.
-            long hostHandshake = NodeProviderOutOfProc.GetHostHandshake(nodeReuse, enableLowPriority: false);
-            long hostHandshakeWithLow = NodeProviderOutOfProc.GetHostHandshake(nodeReuse, enableLowPriority: true);
-
-            ShutdownAllNodes(nodeReuse, NodeContextTerminated);
+            ShutdownAllNodes(ComponentHost.BuildParameters.EnableNodeReuse, NodeContextTerminated);
         }
         #endregion
 
@@ -535,8 +526,7 @@ internal bool CreateNode(HandshakeOptions hostContext, INodePacketFactory factor
                                         commandLineArgs,
                                         (int)hostContext,
                                         this,
-                                        CommunicationsUtilities.GetHostHandshake(hostContext),
-                                        CommunicationsUtilities.GetClientHandshake(hostContext),
+                                        new Handshake(hostContext),
                                         NodeContextTerminated
                                     );
 

src/MSBuild/NodeEndpointOutOfProcTaskHost.cs

@@ -27,17 +27,9 @@ internal NodeEndpointOutOfProcTaskHost(string pipeName)
         /// <summary>
         /// Returns the host handshake for this node endpoint
         /// </summary>
-        protected override long GetHostHandshake()
+        protected override Handshake GetHandshake()
         {
-            return CommunicationsUtilities.GetHostHandshake(CommunicationsUtilities.GetHandshakeOptions(taskHost: true));
-        }
-
-        /// <summary>
-        /// Returns the client handshake for this node endpoint
-        /// </summary>
-        protected override long GetClientHandshake()
-        {
-            return CommunicationsUtilities.GetClientHandshake(CommunicationsUtilities.GetHandshakeOptions(taskHost: true));
+            return new Handshake(CommunicationsUtilities.GetHandshakeOptions(taskHost: true));
         }
     }
 }