Fix for a race condition while getting the log buffer (#1)

daveaglick · Jun 23, 2020 · d6ce605 · d6ce605
1 parent 08ab99b
commit d6ce605
Show file tree

Hide file tree

Showing 3 changed files with 35 additions and 3 deletions.
diff --git a/ReleaseNotes.md b/ReleaseNotes.md
@@ -1,5 +1,7 @@
 # 1.1.3
 
+- [Fix] Fixed a race condition when fetching a log buffer (#1, thanks @duncanawoods)
+
 # 1.1.2
 
 - [Fix] No longer catches certain error exceptions that actually indicate a problem

diff --git a/build.cake b/build.cake
@@ -141,6 +141,23 @@ Task("Pack")
         }
     });
 
+Task("Sign")
+    .IsDependentOn("Pack")
+    .WithCriteria(() => isLocal)
+    .Does(() =>
+    {
+        var certPass = EnvironmentVariable("DAVIDGLICK_CERTPASS");
+        if (string.IsNullOrEmpty(certPass))
+        {
+            throw new InvalidOperationException("Could not resolve certificate password.");
+        }
+
+        foreach (var nupkg in GetFiles($"{ buildDir }/*.nupkg"))
+        {
+            StartProcess("nuget", "sign \"" + nupkg.ToString() + "\" -CertificatePath \"davidglick.pfx\" -CertificatePassword \"" + certPass + "\" -Timestamper \"http://timestamp.digicert.com\" -NonInteractive");
+        }        
+    });
+
 Task("Zip")
     .Description("Zips the build output.")
     .IsDependentOn("Build")
@@ -186,7 +203,7 @@ Task("MyGet")
 
 Task("NuGet")
     .Description("Pushes the packages to the NuGet gallery.")
-    .IsDependentOn("Pack")
+    .IsDependentOn("Sign")
     .WithCriteria(() => isLocal)
     .Does(() =>
     {

diff --git a/src/MsBuildPipeLogger.Server/PipeBuffer.cs b/src/MsBuildPipeLogger.Server/PipeBuffer.cs
@@ -72,11 +72,24 @@ private bool TakeBuffer()
         {
             if (_current == null)
             {
-                if (_queue.IsCompleted)
+                // Take() can throw when marked as complete from another thread
+                // https://docs.microsoft.com/en-us/dotnet/api/system.collections.concurrent.blockingcollection-1.take?view=netcore-3.1
+                try
+                {
+                    _current = _queue.Take();
+                }
+                catch (ObjectDisposedException)
+                {
+                    return false;
+                }
+                catch (OperationCanceledException)
+                {
+                    return false;
+                }
+                catch (InvalidOperationException)
                 {
                     return false;
                 }
-                _current = _queue.Take();
             }
             return true;
         }