Merge pull request #1915 from tyrielv/tyrielv/concurrent-mount

Add mount lock to prevent concurrent mount race condition

Author: tyrielv

GVFS/GVFS.Common/FileBasedLock.cs

@@ -20,7 +20,23 @@ public FileBasedLock(
         protected string LockPath { get; }
         protected ITracer Tracer { get; }
 
-        public abstract bool TryAcquireLock();
+        public bool TryAcquireLock()
+        {
+            return this.TryAcquireLock(out _);
+        }
+
+        /// <summary>
+        /// Attempts to acquire the lock, providing the exception that prevented acquisition.
+        /// </summary>
+        /// <param name="lockException">
+        /// When the method returns false, contains the exception that prevented lock acquisition.
+        /// Callers can pattern-match on the exception type to distinguish lock contention
+        /// (e.g. <see cref="System.IO.IOException"/> with a sharing violation HResult) from
+        /// permission errors (<see cref="UnauthorizedAccessException"/>) or other failures.
+        /// Null when the method returns true.
+        /// </param>
+        /// <returns>True if the lock was acquired, false otherwise.</returns>
+        public abstract bool TryAcquireLock(out Exception lockException);
 
         public abstract void Dispose();
     }

GVFS/GVFS.Common/GVFSConstants.cs

@@ -117,6 +117,7 @@ public static class DotGVFS
         {
             public const string CorruptObjectsName = "CorruptObjects";
             public const string LogName = "logs";
+            public const string MountLock = "mount.lock";
 
             public static class Databases
             {

GVFS/GVFS.Common/ReturnCode.cs

@@ -10,5 +10,6 @@ public enum ReturnCode
         NullRequestData = 5,
         UnableToRegisterForOfflineIO = 6,
         DehydrateFolderFailures = 7,
+        MountAlreadyRunning = 8,
     }
 }

GVFS/GVFS.Mount/InProcessMount.cs

@@ -92,6 +92,28 @@ public void Mount(EventLevel verbosity, Keywords keywords)
         {
             this.currentState = MountState.Mounting;
 
+            string mountLockPath = Path.Combine(this.enlistment.DotGVFSRoot, GVFSConstants.DotGVFS.MountLock);
+            using (FileBasedLock mountLock = GVFSPlatform.Instance.CreateFileBasedLock(
+                new PhysicalFileSystem(),
+                this.tracer,
+                mountLockPath))
+            {
+                if (!mountLock.TryAcquireLock(out Exception lockException))
+                {
+                    if (lockException is IOException)
+                    {
+                        this.FailMountAndExit(ReturnCode.MountAlreadyRunning, "Mount: Another mount process is already running.");
+                    }
+
+                    this.FailMountAndExit("Mount: Failed to acquire mount lock: {0}", lockException.Message);
+                }
+
+                this.MountWithLockAcquired(verbosity, keywords);
+            }
+        }
+
+        private void MountWithLockAcquired(EventLevel verbosity, Keywords keywords)
+        {
             // Start auth + config query immediately — these are network-bound and don't
             // depend on repo metadata or cache paths. Every millisecond of network latency
             // we can overlap with local I/O is a win.
@@ -303,6 +325,11 @@ private NamedPipeServer StartNamedPipe()
         }
 
         private void FailMountAndExit(string error, params object[] args)
+        {
+            this.FailMountAndExit(ReturnCode.GenericError, error, args);
+        }
+
+        private void FailMountAndExit(ReturnCode returnCode, string error, params object[] args)
         {
             this.currentState = MountState.MountFailed;
 
@@ -319,7 +346,7 @@ private void FailMountAndExit(string error, params object[] args)
                 this.fileSystemCallbacks = null;
             }
 
-            Environment.Exit((int)ReturnCode.GenericError);
+            Environment.Exit((int)returnCode);
         }
 
         private T CreateOrReportAndExit<T>(Func<T> factory, string reportMessage)

GVFS/GVFS.Platform.Windows/WindowsFileBasedLock.cs

@@ -36,8 +36,9 @@ public WindowsFileBasedLock(
         {
         }
 
-        public override bool TryAcquireLock()
+        public override bool TryAcquireLock(out Exception lockException)
         {
+            lockException = null;
             try
             {
                 lock (this.deleteOnCloseStreamLock)
@@ -63,13 +64,14 @@ public override bool TryAcquireLock()
             catch (IOException e)
             {
                 // HResultErrorFileExists is expected when the lock file exists
-                // HResultErrorSharingViolation is expected when the lock file exists andanother GVFS process has acquired the lock file
+                // HResultErrorSharingViolation is expected when the lock file exists and another GVFS process has acquired the lock file
                 if (e.HResult != HResultErrorFileExists && e.HResult != HResultErrorSharingViolation)
                 {
                     EventMetadata metadata = this.CreateLockMetadata(e);
                     this.Tracer.RelatedWarning(metadata, $"{nameof(this.TryAcquireLock)}: IOException caught while trying to acquire lock");
                 }
 
+                lockException = e;
                 this.DisposeStream();
                 return false;
             }
@@ -78,6 +80,7 @@ public override bool TryAcquireLock()
                 EventMetadata metadata = this.CreateLockMetadata(e);
                 this.Tracer.RelatedWarning(metadata, $"{nameof(this.TryAcquireLock)}: UnauthorizedAccessException caught while trying to acquire lock");
 
+                lockException = e;
                 this.DisposeStream();
                 return false;
             }
@@ -86,6 +89,7 @@ public override bool TryAcquireLock()
                 EventMetadata metadata = this.CreateLockMetadata(e);
                 this.Tracer.RelatedWarning(metadata, $"{nameof(this.TryAcquireLock)}: Win32Exception caught while trying to acquire lock");
 
+                lockException = e;
                 this.DisposeStream();
                 return false;
             }

GVFS/GVFS.UnitTests/Mock/Common/MockFileBasedLock.cs

@@ -1,6 +1,7 @@
 using GVFS.Common;
 using GVFS.Common.FileSystem;
 using GVFS.Common.Tracing;
+using System;
 
 namespace GVFS.UnitTests.Mock.Common
 {
@@ -14,8 +15,9 @@ public MockFileBasedLock(
         {
         }
 
-        public override bool TryAcquireLock()
+        public override bool TryAcquireLock(out Exception lockException)
         {
+            lockException = null;
             return true;
         }