Reject file-access reporting with multithreaded builds

documentation/specs/multithreading/multithreaded-msbuild.md

@@ -2,6 +2,12 @@
 
 This is a description of changes that will enable MSBuild to run multiple projects concurrently within the same process, rather than spawning separate processes for each node. This will allow better resource utilization and potentially faster builds, as fewer processes will need to be created, reducing .NET runtime overhead and inter-process communication--but it's nontrivial to implement without breaking existing builds, which we must not do.
 
+## Known incompatibilities
+
+### File-access reporting
+
+Because `/reportfileaccesses` requires detoured out-of-proc worker processes and attributes accesses to the single project executing on each node, it cannot be combined with multithreaded mode.
+
 ## Current state
 
 Currently, MSBuild supports parallel builds (a critical feature for a build system) by spawning worker processes. This made adoption easier because it didn't impose any requirements on tasks: they continue to own the whole process while they are executing, just like they did when the build was single-threaded and running one task at a time. But it's a pretty strange design decision in the modern age, where we assume things are multithreaded and async.

src/Build.UnitTests/BackEnd/BuildManager_Tests.cs

@@ -956,6 +956,25 @@ public void SimpleBuildWithFailure()
             _logger.AssertLogContains("[errormessage]");
         }
 
+        [Fact]
+        public void DeferredMessagesAreClearedWhenBeginBuildThrows()
+        {
+            const string deferredMessage = "Message from a failed BeginBuild";
+
+            _buildManager.BeginBuild(_parameters);
+            Should.Throw<InvalidOperationException>(() => _buildManager.BeginBuild(
+                _parameters,
+                [new BuildManager.DeferredBuildMessage(deferredMessage, MessageImportance.High)]));
+            _buildManager.EndBuild();
+
+            BuildResult result = _buildManager.Build(
+                _parameters,
+                GetBuildRequestData("<Project><Target Name='test' /></Project>"));
+
+            result.ShouldHaveSucceeded();
+            _logger.AssertLogDoesntContain(deferredMessage);
+        }
+
         [Fact]
         public void DeferredMessageShouldBeLogged()
         {
@@ -4715,6 +4734,27 @@ public void ProjectWithNoTargetsGraph()
             _logger.AssertLogContains("MSB4040");
         }
 
+#if FEATURE_REPORTFILEACCESSES
+        /// <summary>
+        /// Ensures programmatic callers receive an actionable failure for the unsupported combination from
+        /// https://github.com/dotnet/msbuild/issues/14825.
+        /// </summary>
+        [Fact]
+        public void MultiThreadedAndReportFileAccessesAreRejectedBeforeBuildStarts()
+        {
+            BuildParameters parameters = new()
+            {
+                MultiThreaded = true,
+                ReportFileAccesses = true,
+            };
+
+            InvalidOperationException exception = Should.Throw<InvalidOperationException>(
+                () => _buildManager.BeginBuild(parameters));
+
+            exception.Message.ShouldContain("File-access reporting cannot be used with multi-threaded mode.");
+        }
+#endif
+
         /// <summary>
         /// Verifies that MT mode builds with multiple projects referencing the same dependency
         /// with different target sets do not crash with "Results for configuration X were not

src/Build/BackEnd/BuildManager/BuildManager.cs

@@ -493,7 +493,9 @@ public DeferredBuildMessage(BuildEventArgs buildEvent)
         /// </summary>
         /// <param name="parameters">The build parameters.  May be null.</param>
         /// <param name="deferredBuildMessages"> Build messages to be logged before the build begins. </param>
-        /// <exception cref="InvalidOperationException">Thrown if a build is already in progress.</exception>
+        /// <exception cref="InvalidOperationException">
+        /// Thrown if a build is already in progress, or if file-access reporting and multi-threaded mode are both enabled.
+        /// </exception>
         [RequiresUnreferencedCode("Initializes loggers and project cache plugins by reflecting over assemblies discovered at runtime, which is incompatible with trimming.")]
         public void BeginBuild(BuildParameters parameters, IEnumerable<DeferredBuildMessage> deferredBuildMessages)
         {
@@ -507,8 +509,14 @@ public void BeginBuild(BuildParameters parameters, IEnumerable<DeferredBuildMess
 
             // deferredBuildMessages cannot be an optional parameter on a single BeginBuild method because it would break binary compatibility.
             _deferredBuildMessages = deferredBuildMessages;
-            BeginBuild(parameters);
-            _deferredBuildMessages = null;
+            try
+            {
+                BeginBuild(parameters);
+            }
+            finally
+            {
+                _deferredBuildMessages = null;
+            }
         }
 
         private void UpdatePriority(Process p, ProcessPriorityClass priority)
@@ -528,6 +536,12 @@ private void UpdatePriority(Process p, ProcessPriorityClass priority)
         [RequiresUnreferencedCode("Initializes loggers and project cache plugins by reflecting over assemblies discovered at runtime, which is incompatible with trimming.")]
         public void BeginBuild(BuildParameters parameters)
         {
+#if FEATURE_REPORTFILEACCESSES
+            ErrorUtilities.VerifyThrowInvalidOperation(
+                !parameters.ReportFileAccesses || !parameters.MultiThreaded,
+                "ReportFileAccessesIncompatibleWithMultiThreaded");
+#endif
+
 #if NETFRAMEWORK
             // Collect telemetry unless explicitly opted out via environment variable.
             // The decision to send telemetry is made at EndBuild to avoid eager loading of telemetry assemblies.

src/Build/BackEnd/BuildManager/BuildParameters.cs

@@ -577,6 +577,7 @@ public int MaxNodeCount
         /// <summary>
         /// Enables running build in multiple in-proc nodes.
         /// </summary>
+        /// <remarks>Cannot be enabled together with <c>ReportFileAccesses</c>.</remarks>
         public bool MultiThreaded
         {
             get => _multiThreaded;
@@ -892,6 +893,7 @@ public string OutputResultsCacheFile
         /// <summary>
         /// Gets or sets a value indicating whether file accesses should be reported to any configured project cache plugins.
         /// </summary>
+        /// <remarks>Cannot be enabled together with <see cref="MultiThreaded"/>.</remarks>
         public bool ReportFileAccesses
         {
             get => _reportFileAccesses;

src/Build/Resources/Strings.resx

@@ -2109,6 +2109,9 @@ Utilization:          {0} Average Utilization: {1:###.0}</value>
   <data name="ReportFileAccessesX64Only" xml:space="preserve">
     <value>Reporting file accesses is only currently supported using the x64 flavor of MSBuild.</value>
   </data>
+  <data name="ReportFileAccessesIncompatibleWithMultiThreaded" xml:space="preserve">
+    <value>File-access reporting cannot be used with multi-threaded mode. File-access reporting requires process-isolated worker nodes so each access can be attributed to one project. Disable either ReportFileAccesses or MultiThreaded.</value>
+  </data>
   <data name="Binlog_Source_MultiSubscribeError" xml:space="preserve">
     <value>Structured events and raw events cannot be replayed at the same time.</value>
   </data>