Cross-proc Windows safety and optimizations (#9)

Add new factory flag that sets a mode to require cross-process Windows mutexes for safe source file locking to avoid ReFS concurrency bug. Add optimization to allow bypassing redundant Path.GetFullPath() when caller has done it already. Fix CloneFlags to use individual bits (see #8).

Published new package version 0.1.13 with these changes.

Directory.Build.props

@@ -24,7 +24,7 @@
   <PropertyGroup>
 
     <!-- DOCSYNC: When changing version number update README.md -->
-    <Version>0.1.11.0</Version>
+    <Version>0.1.13.0</Version>
 
     <Company>Microsoft</Company>
     <Copyright>Copyright (c) Microsoft Corporation</Copyright>

README.md

@@ -32,6 +32,8 @@ if (canCloneInCurrentDirectory)
 ## Release History
 [NuGet package](https://www.nuget.org/packages/CopyOnWrite):
 
+* 0.1.13 September 2022: Fix CloneFlags to use individual bits.
+* 0.1.12 September 2022: Add new factory flag that sets a mode to require cross-process Windows mutexes for safe source file locking to avoid a ReFS concurrency bug. Add optimization to allow bypassing redundant Path.GetFullPath() when caller has done it already.
 * 0.1.11 September 2022: Serialize Windows cloning on source path to work around ReFS limitation in multithreaded cloning.
 * 0.1.10 September 2022: Fix missing destination file failure detection.
 * 0.1.9 September 2022: Add explicit cache invalidation call to interface.

lib/CopyOnWriteFilesystemFactory.cs

@@ -14,19 +14,48 @@ namespace Microsoft.CopyOnWrite;
 /// <remarks>This implementation provides access to an appdomain-wide singleton.</remarks>
 public static class CopyOnWriteFilesystemFactory
 {
-    private static readonly ICopyOnWriteFilesystem Instance = Create();
+    private static readonly Lazy<ICopyOnWriteFilesystem> InProcessLocksInstance = new(() => Create(useCrossProcessLocks: false));
+    private static readonly Lazy<ICopyOnWriteFilesystem> CrossProcessLocksInstance = new(() => Create(useCrossProcessLocks: true));
 
     /// <summary>
-    /// Gets a singleton instance of the CoW filesystem appropriate for this operating system.
+    /// Gets an instance of the CoW filesystem appropriate for this operating system.
+    /// This instance uses in-process locks where needed for the current operating system.
     /// </summary>
-    public static ICopyOnWriteFilesystem GetInstance() => Instance;
+    public static ICopyOnWriteFilesystem GetInstance() => InProcessLocksInstance.Value;
 
-    private static ICopyOnWriteFilesystem Create()
+    /// <summary>
+    /// Gets an instance of the CoW filesystem appropriate for this operating system.
+    /// </summary>
+    /// <param name="forceUniqueInstance">
+    /// Forces return of a unique instance instead of a singleton. Useful for unit tests.
+    /// This can be expensive as creating a new instance can re-scan the filesystem to fill the cache.
+    /// Consider instead using <paramref name="forceUniqueInstance"/>=false and utilize
+    /// <see cref="ICopyOnWriteFilesystem.ClearFilesystemCache"/> to clear the cache on the singleton instance.
+    /// </param>
+    /// <param name="useCrossProcessLocksWhereApplicable">
+    /// If true, uses cross-process locks where needed for the current operating system.
+    /// These locks are more expensive than in-process locks, but are required when cloning
+    /// the same source file across process boundaries. Example of cross-process requirement:
+    /// MSBuild uses many worker node processes that may all clone the same source to multiple
+    /// output locations. Counter-example for intra-process locking: If a build engine controls
+    /// all cloning of a source file from a content-addressable store into the filesystem.
+    /// </param>
+    public static ICopyOnWriteFilesystem GetInstance(bool forceUniqueInstance, bool useCrossProcessLocksWhereApplicable)
+    {
+        if (forceUniqueInstance)
+        {
+            return Create(useCrossProcessLocksWhereApplicable);
+        }
+
+        return useCrossProcessLocksWhereApplicable ? CrossProcessLocksInstance.Value : InProcessLocksInstance.Value;
+    }
+
+    private static ICopyOnWriteFilesystem Create(bool useCrossProcessLocks)
     {
         switch (Environment.OSVersion.Platform)
         {
             case PlatformID.Win32NT:
-                return new WindowsCopyOnWriteFilesystem();
+                return new WindowsCopyOnWriteFilesystem(useCrossProcessLocks);
             case PlatformID.Unix:
                 return new LinuxCopyOnWriteFilesystem();
             case PlatformID.MacOSX:

lib/ICopyOnWriteFilesystem.cs

@@ -45,8 +45,12 @@ public interface ICopyOnWriteFilesystem
     /// </summary>
     /// <param name="source">The file path to which the link will point.</param>
     /// <param name="destination">The file path that would contain the link.</param>
+    /// <param name="pathsAreFullyResolved">
+    /// When true, avoids expensive calls to <see cref="System.IO.Path.GetFullPath(string)"/> to resolve the
+    /// full path by asserting that the caller already called it for the source and destination paths.
+    /// </param>
     /// <returns>True if a link can be created, false if it cannot.</returns>
-    bool CopyOnWriteLinkSupportedBetweenPaths(string source, string destination);
+    bool CopyOnWriteLinkSupportedBetweenPaths(string source, string destination, bool pathsAreFullyResolved = false);
 
     /// <summary>
     /// Determines whether copy-on-write links can be created for files at the specified
@@ -60,8 +64,12 @@ public interface ICopyOnWriteFilesystem
     /// copy-on-write links, or it can be a path under a volume mount point within
     /// another volume.
     /// </param>
+    /// <param name="pathIsFullyResolved">
+    /// When true, avoids an expensive call to <see cref="System.IO.Path.GetFullPath(string)"/> to resolve the
+    /// full path by asserting that the caller already called it for the root path.
+    /// </param>
     /// <returns>True if a link can be created, false if it cannot.</returns>
-    bool CopyOnWriteLinkSupportedInDirectoryTree(string rootDirectory);
+    bool CopyOnWriteLinkSupportedInDirectoryTree(string rootDirectory, bool pathIsFullyResolved = false);
 
     /// <summary>
     /// Creates a copy-on-write link at <paramref name="destination"/> pointing
@@ -139,26 +147,32 @@ public enum CloneFlags
     /// <summary>
     /// Default zero value, no behavior changes.
     /// </summary>
-    None,
+    None = 0,
 
     /// <summary>
     /// Skip check for and copy of Windows file integrity settings from source to destination.
     /// Use when the filesystem and file are known not to use integrity.
     /// Saves 1-2 kernel round-trips.
     /// </summary>
-    NoFileIntegrityCheck,
+    NoFileIntegrityCheck = 0x01,
 
     /// <summary>
     /// Skip check for Windows sparse file attribute and application of sparse setting in destination.
     /// Use when the filesystem and file are known not to be sparse.
     /// Saves time by allowing use of less expensive kernel APIs.
     /// </summary>
-    NoSparseFileCheck,
+    NoSparseFileCheck = 0x02,
 
     /// <summary>
     /// Skip serialized clone creation if the OS's CoW facility cannot handle multi-threaded clone calls
     /// in a stable way (Windows as of Server 2022 / Windows 11). Skip this check if you know that only
     /// one clone of a source file will be performed at a time to improve performance.
     /// </summary>
-    NoSerializedCloning,
+    NoSerializedCloning = 0x04,
+
+    /// <summary>
+    /// Avoids expensive calls to <see cref="System.IO.Path.GetFullPath(string)"/> to resolve the full path by asserting
+    /// that the caller already called it for the source and destination paths.
+    /// </summary>
+    PathIsFullyResolved = 0x08,
 }

lib/Linux/LinuxCopyOnWriteFilesystem.cs

@@ -11,13 +11,13 @@ internal sealed class LinuxCopyOnWriteFilesystem : ICopyOnWriteFilesystem
 {
     public int MaxClonesPerFile => int.MaxValue;
 
-    public bool CopyOnWriteLinkSupportedBetweenPaths(string source, string destination)
+    public bool CopyOnWriteLinkSupportedBetweenPaths(string source, string destination, bool pathsAreFullyResolved = false)
     {
         // TODO: Implement FS probing and return a real value.
         throw new NotImplementedException();
     }
 
-    public bool CopyOnWriteLinkSupportedInDirectoryTree(string rootDirectory)
+    public bool CopyOnWriteLinkSupportedInDirectoryTree(string rootDirectory, bool pathIsFullyResolved = false)
     {
         throw new NotImplementedException();
     }

lib/Mac/MacCopyOnWriteFilesystem.cs

@@ -11,14 +11,14 @@ internal sealed class MacCopyOnWriteFilesystem : ICopyOnWriteFilesystem
 {
     public int MaxClonesPerFile => int.MaxValue;
 
-    public bool CopyOnWriteLinkSupportedBetweenPaths(string source, string destination)
+    public bool CopyOnWriteLinkSupportedBetweenPaths(string source, string destination, bool pathsAreFullyResolved = false)
     {
         // AppleFS always supports CoW.
         // return true;
         throw new NotImplementedException();
     }
 
-    public bool CopyOnWriteLinkSupportedInDirectoryTree(string rootDirectory)
+    public bool CopyOnWriteLinkSupportedInDirectoryTree(string rootDirectory, bool pathIsFullyResolved = false)
     {
         // AppleFS always supports CoW.
         // return true;

lib/Windows/WindowsCopyOnWriteFilesystem.cs

@@ -20,7 +20,13 @@ internal sealed class WindowsCopyOnWriteFilesystem : ICopyOnWriteFilesystem
     internal const long MaxChunkSize = 1L << 31;  // 2GB
 
     private VolumeInfoCache _volumeInfoCache = VolumeInfoCache.BuildFromCurrentFilesystem();
-    private readonly LockSet<string> _sourcePathLockSet = new(StringComparer.OrdinalIgnoreCase);
+    private readonly LockSet<string> _sourcePathLockSetForInProcessSerialization = new(StringComparer.OrdinalIgnoreCase);
+    private readonly bool _useCrossProcessLocks;
+
+    public WindowsCopyOnWriteFilesystem(bool useCrossProcessLocks)
+    {
+        _useCrossProcessLocks = useCrossProcessLocks;
+    }
 
     // https://docs.microsoft.com/en-us/windows-server/storage/refs/block-cloning#functionality-restrictions-and-remarks
     /// <inheritdoc />
@@ -29,15 +35,15 @@ internal sealed class WindowsCopyOnWriteFilesystem : ICopyOnWriteFilesystem
     // TODO: Deal with \??\ prefixes
     // TODO: Deal with \\?\ prefixes
     /// <inheritdoc />
-    public bool CopyOnWriteLinkSupportedBetweenPaths(string source, string destination)
+    public bool CopyOnWriteLinkSupportedBetweenPaths(string source, string destination, bool pathsAreFullyResolved = false)
     {
-        (string resolvedSource, bool sourceOk) = ResolvePathAndEnsureDriveLetterVolume(source);
+        (string resolvedSource, bool sourceOk) = ResolvePathAndEnsureDriveLetterVolume(source, pathsAreFullyResolved);
         if (!sourceOk)
         {
             return false;
         }
 
-        (string resolvedDestination, bool destOk) = ResolvePathAndEnsureDriveLetterVolume(destination);
+        (string resolvedDestination, bool destOk) = ResolvePathAndEnsureDriveLetterVolume(destination, pathsAreFullyResolved);
         if (!destOk)
         {
             return false;
@@ -55,9 +61,9 @@ public bool CopyOnWriteLinkSupportedBetweenPaths(string source, string destinati
     }
 
     /// <inheritdoc />
-    public bool CopyOnWriteLinkSupportedInDirectoryTree(string rootDirectory)
+    public bool CopyOnWriteLinkSupportedInDirectoryTree(string rootDirectory, bool pathIsFullyResolved = false)
     {
-        (string resolvedSource, bool sourceOk) = ResolvePathAndEnsureDriveLetterVolume(rootDirectory);
+        (string resolvedSource, bool sourceOk) = ResolvePathAndEnsureDriveLetterVolume(rootDirectory, pathIsFullyResolved);
         if (!sourceOk)
         {
             return false;
@@ -92,7 +98,7 @@ public async
 #endif
         CloneFileAsync(string source, string destination, CloneFlags cloneFlags, CancellationToken cancellationToken)
     {
-        (string resolvedSource, bool sourceOk) = ResolvePathAndEnsureDriveLetterVolume(source);
+        (string resolvedSource, bool sourceOk) = ResolvePathAndEnsureDriveLetterVolume(source, cloneFlags.HasFlag(CloneFlags.PathIsFullyResolved));
         if (!sourceOk)
         {
             throw new NotSupportedException($"Source path '{source}' is not in the correct format");
@@ -228,10 +234,29 @@ public async
                 $"Failed to set end of file with winerror {lastErr} on destination file '{destination}'");
         }
 
-        LockSet<string>.LockHandle lockHandle;
+        IDisposable lockHandle;
         if (!cloneFlags.HasFlag(CloneFlags.NoSerializedCloning))
         {
-            lockHandle = await _sourcePathLockSet.AcquireAsync(resolvedSource);
+            if (_useCrossProcessLocks)
+            {
+                string mutexName = $@"Global\{resolvedSource.ToUpperInvariant().Replace(':', '_').Replace('\\', '_')}";
+                var mutex = new Mutex(false, mutexName);
+                Thread.BeginThreadAffinity();
+                try
+                {
+                    mutex.WaitOne(); // No async available: affinitized to current thread.
+                }
+                catch (AbandonedMutexException)
+                {
+                    // We got the lock on the mutex despite it being abandoned by another thread/process.
+                }
+
+                lockHandle = mutex;
+            }
+            else
+            {
+                lockHandle = await _sourcePathLockSetForInProcessSerialization.AcquireAsync(resolvedSource);
+            }
         }
         else
         {
@@ -248,6 +273,11 @@ public async
             {
                 lockHandle.Dispose();
             }
+
+            if (_useCrossProcessLocks)
+            {
+                Thread.EndThreadAffinity();
+            }
         }
     }
 
@@ -317,11 +347,15 @@ internal static long RoundUpToPowerOf2(long originalValue, long roundingMultiple
         return (originalValue + mask) & ~mask;
     }
 
-    private static (string, bool) ResolvePathAndEnsureDriveLetterVolume(string path)
+    private static (string, bool) ResolvePathAndEnsureDriveLetterVolume(string path, bool pathIsFullyResolved)
     {
         if (path.Length < 2 || path[1] != ':')
         {
-            path = Path.GetFullPath(path);
+            if (!pathIsFullyResolved)
+            {
+                path = Path.GetFullPath(path);
+            }
+
             if (path.Length < 2 || path[1] != ':')
             {
                 // Possible UNC path or other strangeness.