Fix for hang when MSBuild process fails to start or logger doesn't co…

…nnect (#78)
phmonte · Sep 27, 2018 · 050dcd9 · 050dcd9
1 parent 7ff7972
commit 050dcd9
Show file tree

Hide file tree

Showing 9 changed files with 39 additions and 19 deletions.
diff --git a/ReleaseNotes.md b/ReleaseNotes.md
@@ -1,3 +1,7 @@
+# 2.0.1
+
+- [Fix] Fix for hang when the MSBuild process fails to start or logger doesn't connect (#78)
+
 # 2.0.0
 
 - **[Breaking Change]** [Refactoring] Entire API...again. Consider this the "if at first you don't succeed" release.

diff --git a/src/Buildalyzer.Logger/Buildalyzer.Logger.csproj b/src/Buildalyzer.Logger/Buildalyzer.Logger.csproj
@@ -19,7 +19,7 @@
   <ItemGroup>
     <PackageReference Include="Microsoft.Build" Version="14.3.0" PrivateAssets="All" />
     <PackageReference Include="Microsoft.Build.Utilities.Core" Version="14.3.0" PrivateAssets="All" />
-    <PackageReference Include="MsBuildPipeLogger.Logger" Version="1.0.3" PrivateAssets="All" IsLogger="true" />
+    <PackageReference Include="MsBuildPipeLogger.Logger" Version="1.1.0" PrivateAssets="All" IsLogger="true" />
   </ItemGroup>
 
   <!-- Workaround to pack package reference directly -->

diff --git a/src/Buildalyzer/Buildalyzer.csproj b/src/Buildalyzer/Buildalyzer.csproj
@@ -21,7 +21,7 @@
     <PackageReference Include="Microsoft.Extensions.DependencyModel" Version="2.1.0" />
     <PackageReference Include="Microsoft.Extensions.Logging" Version="2.1.1" />
     <PackageReference Include="Microsoft.Extensions.Logging.Abstractions" Version="2.1.1" />
-    <PackageReference Include="MsBuildPipeLogger.Server" Version="1.0.3" />
+    <PackageReference Include="MsBuildPipeLogger.Server" Version="1.1.0" />
   </ItemGroup>
   <ItemGroup>
     <AssemblyAttribute Include="System.Runtime.CompilerServices.InternalsVisibleTo">

diff --git a/src/Buildalyzer/Environment/ProcessRunner.cs b/src/Buildalyzer/Environment/ProcessRunner.cs
@@ -14,6 +14,8 @@ internal class ProcessRunner : IDisposable
 
         public Process Process { get; }
 
+        public Action Exited { get; set; }
+
         public ProcessRunner(
             string fileName,
             string arguments,
@@ -50,7 +52,8 @@ public ProcessRunner(
                 Process.OutputDataReceived += DataReceived;
             }
 
-            Process.Exited += Exited;
+            Process.EnableRaisingEvents = true;  // Raises Process.Exited immediatly instead of when checked via .WaitForExit() or .HasExited
+            Process.Exited += ProcessExited;
         }
 
         public ProcessRunner Start()
@@ -64,14 +67,15 @@ public ProcessRunner Start()
             return this;
         }
 
-        private void Exited(object sender, EventArgs e)
+        private void ProcessExited(object sender, EventArgs e)
         {
+            Exited?.Invoke();
             _logger?.LogDebug($"Process {Process.Id} exited with code {Process.ExitCode}{System.Environment.NewLine}{System.Environment.NewLine}");
         }
 
         public void Dispose()
         {
-            Process.Exited -= Exited;
+            Process.Exited -= ProcessExited;
             if (_logger != null || _output != null)
             {
                 Process.OutputDataReceived -= DataReceived;

diff --git a/src/Buildalyzer/ProjectAnalyzer.cs b/src/Buildalyzer/ProjectAnalyzer.cs
@@ -3,6 +3,7 @@
 using System.Collections.Generic;
 using System.IO;
 using System.Linq;
+using System.Threading;
 using Buildalyzer.Construction;
 using Buildalyzer.Environment;
 using Buildalyzer.Logger;
@@ -230,25 +231,27 @@ private void Restore(BuildEnvironment buildEnvironment, ref string[] targetsToBu
         // This is where the magic happens - returns one result per result target framework
         private AnalyzerResults BuildTargets(BuildEnvironment buildEnvironment, string targetFramework, string[] targetsToBuild, AnalyzerResults results)
         {
-            using (AnonymousPipeLoggerServer pipeLogger = new AnonymousPipeLoggerServer())
+            using (CancellationTokenSource cancellation = new CancellationTokenSource())
             {
-                using (EventProcessor eventProcessor = new EventProcessor(this, BuildLoggers, pipeLogger, results != null))
+                using (AnonymousPipeLoggerServer pipeLogger = new AnonymousPipeLoggerServer(cancellation.Token))
                 {
-                    // Run MSBuild
-                    int exitCode;
-                    string fileName = GetCommand(buildEnvironment, targetFramework, targetsToBuild, pipeLogger.GetClientHandle(), out string arguments);
-                    using (ProcessRunner processRunner = new ProcessRunner(fileName, arguments, Path.GetDirectoryName(ProjectFile.Path), GetEffectiveEnvironmentVariables(buildEnvironment), Manager.LoggerFactory))
+                    using (EventProcessor eventProcessor = new EventProcessor(this, BuildLoggers, pipeLogger, results != null))
                     {
-                        processRunner.Start();
-                        while (pipeLogger.Read())
+                        // Run MSBuild
+                        int exitCode;
+                        string fileName = GetCommand(buildEnvironment, targetFramework, targetsToBuild, pipeLogger.GetClientHandle(), out string arguments);
+                        using (ProcessRunner processRunner = new ProcessRunner(fileName, arguments, Path.GetDirectoryName(ProjectFile.Path), GetEffectiveEnvironmentVariables(buildEnvironment), Manager.LoggerFactory))
                         {
+                            processRunner.Exited = () => cancellation.Cancel();
+                            processRunner.Start();
+                            pipeLogger.ReadAll();
+                            processRunner.Process.WaitForExit();
+                            exitCode = processRunner.Process.ExitCode;
                         }
-                        processRunner.Process.WaitForExit();
-                        exitCode = processRunner.Process.ExitCode;
-                    }
 
-                    // Collect the results
-                    results?.Add(eventProcessor.Results, exitCode == 0 && eventProcessor.OverallSuccess);
+                        // Collect the results
+                        results?.Add(eventProcessor.Results, exitCode == 0 && eventProcessor.OverallSuccess);
+                    }
                 }
             }
             return results;

diff --git a/tests/Buildalyzer.Tests/Buildalyzer.Tests.csproj b/tests/Buildalyzer.Tests/Buildalyzer.Tests.csproj
@@ -12,7 +12,7 @@
     <PackageReference Include="NUnit" Version="3.10.1" />
     <PackageReference Include="NUnit3TestAdapter" Version="3.10.0" />
     <PackageReference Include="Shouldly" Version="3.0.0" />
-    <PackageReference Include="MsBuildPipeLogger.Logger" Version="1.0.3" />
+    <PackageReference Include="MsBuildPipeLogger.Logger" Version="1.1.0" />
   </ItemGroup>
 
   <ItemGroup>

diff --git a/tests/Buildalyzer.Tests/Integration/OpenSourceProjectsFixture.cs b/tests/Buildalyzer.Tests/Integration/OpenSourceProjectsFixture.cs
@@ -144,10 +144,14 @@ public void CompilesProject(EnvironmentPreference preference, string solutionPat
             DeleteProjectDirectory(analyzer.ProjectFile.Path, "obj");
             DeleteProjectDirectory(analyzer.ProjectFile.Path, "bin");
             analyzer.IgnoreFaultyImports = false;
+
+#pragma warning disable 0162
             if (BinaryLog)
             {
                 analyzer.AddBinaryLogger($@"E:\Temp\{Path.GetFileNameWithoutExtension(solutionPath)}.{Path.GetFileNameWithoutExtension(analyzer.ProjectFile.Path)}.core.binlog");
             }
+#pragma warning restore 0162
+
             AnalyzerResults results = analyzer.Build(options);
 
             // Then

diff --git a/tests/Buildalyzer.Tests/Integration/SimpleProjectsFixture.cs b/tests/Buildalyzer.Tests/Integration/SimpleProjectsFixture.cs
@@ -376,10 +376,14 @@ private static ProjectAnalyzer GetProjectAnalyzer(string projectFile, StringWrit
                     LogWriter = log
                 })
                 .GetProject(GetProjectPath(projectFile));
+
+#pragma warning disable 0162
             if (BinaryLog)
             {
                 analyzer.AddBinaryLogger(Path.Combine(@"E:\Temp\", Path.ChangeExtension(Path.GetFileName(projectFile), ".core.binlog")));
             }
+#pragma warning restore 0162
+
             return analyzer;
         }
 

diff --git a/tests/Buildalyzer.Workspaces.Tests/Buildalyzer.Workspaces.Tests.csproj b/tests/Buildalyzer.Workspaces.Tests/Buildalyzer.Workspaces.Tests.csproj
@@ -11,6 +11,7 @@
     <PackageReference Include="NUnit" Version="3.10.1" />
     <PackageReference Include="NUnit3TestAdapter" Version="3.10.0" />
     <PackageReference Include="Shouldly" Version="3.0.0" />
+    <PackageReference Include="MsBuildPipeLogger.Logger" Version="1.1.0" />
   </ItemGroup>
 
   <ItemGroup>