Make LaunchPrefetchJobIfIdle thread safe and update it to exit when unhandled exceptions occur

GVFS/GVFS.Common/Prefetch/BackgroundPrefetcher.cs

@@ -12,6 +12,7 @@ public class BackgroundPrefetcher : IDisposable
         private const string TelemetryKey = nameof(BackgroundPrefetcher);
         private readonly TimeSpan timerPeriod = TimeSpan.FromMinutes(15);
         private readonly TimeSpan timeBetweenPrefetches = TimeSpan.FromMinutes(70);
+        private readonly object launchPrefetchLock = new object();
 
         private ITracer tracer;
         private GVFSEnlistment enlistment;
@@ -48,19 +49,30 @@ public void Dispose()
 
         public bool LaunchPrefetchJobIfIdle()
         {
-            if (this.prefetchJobThread?.IsAlive == true)
+            try
             {
-                this.tracer.RelatedInfo(nameof(BackgroundPrefetcher) + ": background thread not idle, skipping timed start");
+                lock (this.launchPrefetchLock)
+                {
+                    if (this.prefetchJobThread?.IsAlive == true)
+                    {
+                        this.tracer.RelatedInfo(nameof(BackgroundPrefetcher) + ": background thread not idle, skipping timed start");
+                    }
+                    else
+                    {
+                        this.prefetchJobThread = new Thread(() => this.BackgroundPrefetch());
+                        this.prefetchJobThread.IsBackground = true;
+                        this.prefetchJobThread.Start();
+                        return true;
+                    }
+
+                    return false;
+                }
             }
-            else
+            catch (Exception e)
             {
-                this.prefetchJobThread = new Thread(() => this.BackgroundPrefetch());
-                this.prefetchJobThread.IsBackground = true;
-                this.prefetchJobThread.Start();
-                return true;
+                this.LogUnhandledExceptionAndExit(nameof(this.LaunchPrefetchJobIfIdle), e);
+                return false;
             }
-
-            return false;
         }
 
         /// <summary>
@@ -119,16 +131,21 @@ private void BackgroundPrefetch()
             }
             catch (Exception e)
             {
-                EventMetadata metadata = new EventMetadata();
-                metadata.Add("Method", nameof(this.BackgroundPrefetch));
-                metadata.Add("ExceptionMessage", e.Message);
-                metadata.Add("StackTrace", e.StackTrace);
-                this.tracer.RelatedError(
-                    metadata: metadata,
-                    message: TelemetryKey + ": Unexpected Exception while running prefetch background thread (fatal): " + e.Message,
-                    keywords: Keywords.Telemetry);
-                Environment.Exit((int)ReturnCode.GenericError);
+                this.LogUnhandledExceptionAndExit(nameof(this.BackgroundPrefetch), e);
             }
         }
+
+        private void LogUnhandledExceptionAndExit(string methodName, Exception e)
+        {
+            EventMetadata metadata = new EventMetadata();
+            metadata.Add("Method", methodName);
+            metadata.Add("ExceptionMessage", e.Message);
+            metadata.Add("StackTrace", e.StackTrace);
+            this.tracer.RelatedError(
+                metadata: metadata,
+                message: TelemetryKey + ": Unexpected Exception while running prefetch background thread (fatal): " + e.Message,
+                keywords: Keywords.Telemetry);
+            Environment.Exit((int)ReturnCode.GenericError);
+        }
     }
 }

GVFS/GVFS.UnitTests/Prefetch/BackgroundPrefetcherTests.cs

@@ -1,7 +1,9 @@
 using GVFS.Common.Prefetch;
 using GVFS.Tests.Should;
+using GVFS.UnitTests.Mock.FileSystem;
 using GVFS.UnitTests.Virtual;
 using NUnit.Framework;
+using System.Threading;
 
 namespace GVFS.UnitTests.Prefetch
 {
@@ -32,5 +34,47 @@ public void RestartBackgroundJobSucceeds()
                 prefetcher.WaitForPrefetchToFinish();
             }
         }
+
+        [TestCase]
+        public void LaunchPrefetchJobIfIdleDoesNotLaunchSecondThreadIfFirstInProgress()
+        {
+            using (CommonRepoSetup setup = new CommonRepoSetup())
+            {
+                BlockedCreateDirectoryFileSystem fileSystem = new BlockedCreateDirectoryFileSystem(setup.FileSystem.RootDirectory);
+                using (BackgroundPrefetcher prefetcher = new BackgroundPrefetcher(
+                    setup.Context.Tracer,
+                    setup.Context.Enlistment,
+                    fileSystem,
+                    setup.GitObjects))
+                {
+                    prefetcher.LaunchPrefetchJobIfIdle().ShouldBeTrue();
+                    prefetcher.LaunchPrefetchJobIfIdle().ShouldBeFalse();
+                    fileSystem.UnblockCreateDirectory();
+                    prefetcher.WaitForPrefetchToFinish();
+                }
+            }
+        }
+
+        private class BlockedCreateDirectoryFileSystem : MockFileSystem
+        {
+            private ManualResetEvent unblockCreateDirectory;
+
+            public BlockedCreateDirectoryFileSystem(MockDirectory rootDirectory)
+                : base(rootDirectory)
+            {
+                this.unblockCreateDirectory = new ManualResetEvent(initialState: false);
+            }
+
+            public void UnblockCreateDirectory()
+            {
+                this.unblockCreateDirectory.Set();
+            }
+
+            public override void CreateDirectory(string path)
+            {
+                this.unblockCreateDirectory.WaitOne();
+                base.CreateDirectory(path);
+            }
+        }
     }
 }

GVFS/GVFS.UnitTests/Virtual/CommonRepoSetup.cs

@@ -34,14 +34,14 @@ public CommonRepoSetup()
             enlistmentDirectory.CreateFile(Path.Combine(this.GitParentPath, ".git", "info", "always_exclude"), "always_exclude Contents", createDirectories: true);
             enlistmentDirectory.CreateDirectory(enlistment.GitPackRoot);
 
-            MockFileSystem fileSystem = new MockFileSystem(enlistmentDirectory);
+            this.FileSystem = new MockFileSystem(enlistmentDirectory);
             this.Repository = new MockGitRepo(
                 tracer, 
                 enlistment,
-                fileSystem);
+                this.FileSystem);
             CreateStandardGitTree(this.Repository);
 
-            this.Context = new GVFSContext(tracer, fileSystem, this.Repository, enlistment);
+            this.Context = new GVFSContext(tracer, this.FileSystem, this.Repository, enlistment);
 
             this.HttpObjects = new MockHttpGitObjects(tracer, enlistment);
             this.GitObjects = new MockGVFSGitObjects(this.Context, this.HttpObjects);
@@ -56,6 +56,7 @@ public CommonRepoSetup()
 
         public MockGitRepo Repository { get; private set; }
         public MockHttpGitObjects HttpObjects { get; private set; }
+        public MockFileSystem FileSystem { get; private set; }
 
         public void Dispose()
         {