diff --git a/GVFS/GVFS.Common/Prefetch/BackgroundPrefetcher.cs b/GVFS/GVFS.Common/Prefetch/BackgroundPrefetcher.cs index e9927393a..fd2fa1987 100644 --- a/GVFS/GVFS.Common/Prefetch/BackgroundPrefetcher.cs +++ b/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; } /// @@ -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); + } } } diff --git a/GVFS/GVFS.UnitTests/Prefetch/BackgroundPrefetcherTests.cs b/GVFS/GVFS.UnitTests/Prefetch/BackgroundPrefetcherTests.cs index 667376fbe..9d3f838a2 100644 --- a/GVFS/GVFS.UnitTests/Prefetch/BackgroundPrefetcherTests.cs +++ b/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); + } + } } } diff --git a/GVFS/GVFS.UnitTests/Virtual/CommonRepoSetup.cs b/GVFS/GVFS.UnitTests/Virtual/CommonRepoSetup.cs index 9594d9da0..ab9731039 100644 --- a/GVFS/GVFS.UnitTests/Virtual/CommonRepoSetup.cs +++ b/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() {