Improved general file system reliability

src/Core/SecureFolderFS.Core.Dokany/AppModels/DokanyOptions.cs

@@ -33,7 +33,7 @@ public static DokanyOptions ToOptions(IDictionary<string, object> options)
                 FileSystemStatistics = (IFileSystemStatistics?)options.Get(nameof(FileSystemStatistics)) ?? new FileSystemStatistics(),
                 IsReadOnly = (bool?)options.Get(nameof(IsReadOnly)) ?? false,
                 IsCachingChunks = (bool?)options.Get(nameof(IsCachingChunks)) ?? true,
-                IsCachingFileNames = (bool?)options.Get(nameof(IsCachingFileNames)) ?? true,
+                IsCachingFileNames = (bool?)options.Get(nameof(IsCachingFileNames)) ?? false,
                 IsCachingDirectoryIds = (bool?)options.Get(nameof(IsCachingDirectoryIds)) ?? true,
                 RecycleBinSize = (long?)options.Get(nameof(RecycleBinSize)) ?? 0L,
 

src/Core/SecureFolderFS.Core.Dokany/Callbacks/BaseDokanyCallbacks.cs

@@ -1,17 +1,18 @@
-using DokanNet;
-using SecureFolderFS.Core.Dokany.Helpers;
-using SecureFolderFS.Core.FileSystem;
-using SecureFolderFS.Core.FileSystem.AppModels;
-using SecureFolderFS.Core.FileSystem.Exceptions;
-using SecureFolderFS.Core.FileSystem.OpenHandles;
-using System;
+using System;
 using System.Collections.Generic;
 using System.Diagnostics;
 using System.IO;
 using System.Linq;
 using System.Runtime.CompilerServices;
 using System.Security.AccessControl;
 using System.Security.Cryptography;
+using DokanNet;
+using SecureFolderFS.Core.Dokany.Helpers;
+using SecureFolderFS.Core.FileSystem;
+using SecureFolderFS.Core.FileSystem.AppModels;
+using SecureFolderFS.Core.FileSystem.Exceptions;
+using SecureFolderFS.Core.FileSystem.Helpers;
+using SecureFolderFS.Core.FileSystem.OpenHandles;
 using FileAccess = DokanNet.FileAccess;
 
 namespace SecureFolderFS.Core.Dokany.Callbacks
@@ -64,12 +65,20 @@
 
             try
             {
-                fileHandle.Stream.Flush();
+                lock (fileHandle.Stream)
+                    fileHandle.Stream.Flush();
+
                 return Trace(DokanResult.Success, fileName, info);
             }
-            catch (IOException)
+            catch (IOException ioEx)
             {
-                return DokanResult.DiskFull;
+                if (ErrorHandlingHelpers.IsDiskFullException(ioEx))
+                    return Trace(DokanResult.DiskFull, fileName, info);
+
+                if (DokanyErrorHelpers.NtStatusFromException(ioEx, out var ntStatus))
+                    return Trace((NtStatus)ntStatus, fileName, info);
+
+                return Trace(DokanResult.Unsuccessful, fileName, info);
             }
         }
 
@@ -88,7 +97,9 @@
             if (handlesManager.GetHandle<FileHandle>(GetContextValue(info)) is not { } fileHandle)
                 return Trace(DokanResult.InvalidHandle, fileName, info);
 
-            fileHandle.Stream.SetLength(length);
+            lock (fileHandle.Stream)
+                fileHandle.Stream.SetLength(length);
+
             return Trace(DokanResult.Success, fileName, info);
         }
 
@@ -131,7 +142,6 @@
         }
 
         /// <inheritdoc/>
-        [MethodImpl(MethodImplOptions.Synchronized)]
         public virtual unsafe NtStatus ReadFile(string fileName, IntPtr buffer, uint bufferLength, out int bytesRead, long offset,
             IDokanFileInfo info)
         {
@@ -164,20 +174,25 @@
 
             try
             {
-                // Check EOF
-                if (offset >= fileHandle.Stream.Length)
+                // Lock on the handle's stream. The kernel can issue concurrent operations
+                // on the same handle, and setting the position and reading must be atomic
+                lock (fileHandle.Stream)
                 {
-                    bytesRead = 0;
-                    return NtStatus.EndOfFile;
+                    // Check EOF
+                    if (offset >= fileHandle.Stream.Length)
+                    {
+                        bytesRead = 0;
+                        return NtStatus.EndOfFile;
+                    }
+
+                    // Align position
+                    fileHandle.Stream.Position = offset;
+
+                    // Read file
+                    var bufferSpan = new Span<byte>(buffer.ToPointer(), (int)bufferLength);
+                    bytesRead = fileHandle.Stream.Read(bufferSpan);
                 }
 
-                // Align position
-                fileHandle.Stream.Position = offset;
-
-                // Read file
-                var bufferSpan = new Span<byte>(buffer.ToPointer(), (int)bufferLength);
-                bytesRead = fileHandle.Stream.Read(bufferSpan);
-
                 return Trace(DokanResult.Success, fileName, info);
             }
             catch (PathTooLongException)
@@ -203,7 +218,6 @@
         }
 
         /// <inheritdoc/>
-        [MethodImpl(MethodImplOptions.Synchronized)]
         public virtual unsafe NtStatus WriteFile(string fileName, IntPtr buffer, uint bufferLength, out int bytesWritten, long offset, IDokanFileInfo info)
         {
             if (specifics.Options.IsReadOnly)
@@ -228,7 +242,9 @@
             if (handlesManager.GetHandle<FileHandle>(GetContextValue(info)) is not { } fileHandle)
             {
                 // Invalid handle...
-                contextHandle = handlesManager.OpenFileHandle(ciphertextPath, appendToFile ? FileMode.Append : FileMode.Open, System.IO.FileAccess.ReadWrite, FileShare.Read, FileOptions.None);
+                // FileMode.Append cannot be combined with FileAccess.ReadWrite - open normally
+                // and seek to the end instead (the append position is aligned below)
+                contextHandle = handlesManager.OpenFileHandle(ciphertextPath, appendToFile ? FileMode.OpenOrCreate : FileMode.Open, System.IO.FileAccess.ReadWrite, FileShare.Read, FileOptions.None);
                 fileHandle = handlesManager.GetHandle<FileHandle>(contextHandle);
                 openedNewHandle = true;
             }
@@ -242,19 +258,24 @@
 
             try
             {
-                // Align for Paging I/O
-                var alignedBytesToCopy = AlignSizeForPagingIo((int)bufferLength, offset, fileHandle.Stream.Length, info);
+                // Lock on the handle's stream as the kernel can issue concurrent operations
+                // on the same handle, and setting the position and writing must be atomic
+                lock (fileHandle.Stream)
+                {
+                    // Align for Paging I/O
+                    var alignedBytesToCopy = AlignSizeForPagingIo((int)bufferLength, offset, fileHandle.Stream.Length, info);
 
-                // Align position for offset
-                var alignedPosition = appendToFile ? fileHandle.Stream.Length : offset;
+                    // Align position for offset
+                    var alignedPosition = appendToFile ? fileHandle.Stream.Length : offset;
 
-                // Align position
-                fileHandle.Stream.Position = alignedPosition;
+                    // Align position
+                    fileHandle.Stream.Position = alignedPosition;
 
-                // Write file
-                var bufferSpan = new ReadOnlySpan<byte>(buffer.ToPointer(), alignedBytesToCopy);
-                fileHandle.Stream.Write(bufferSpan);
-                bytesWritten = alignedBytesToCopy;
+                    // Write file
+                    var bufferSpan = new ReadOnlySpan<byte>(buffer.ToPointer(), alignedBytesToCopy);
+                    fileHandle.Stream.Write(bufferSpan);
+                    bytesWritten = alignedBytesToCopy;
+                }
 
                 return Trace(DokanResult.Success, fileName, info);
             }
@@ -376,6 +397,11 @@
                 return bufferLength;
 
             var longDistanceToEnd = streamLength - offset;
+
+            // Paging I/O must not write beyond the end of the file
+            if (longDistanceToEnd <= 0L)
+                return 0;
+
             if (longDistanceToEnd > int.MaxValue)
                 return bufferLength;
 
@@ -394,10 +420,10 @@
             if (Debugger.IsAttached)
                 return result;
 
-            if (!Core.FileSystem.Constants.OPT_IN_FOR_OPTIONAL_DEBUG_TRACING)
+            if (!FileSystem.Constants.OPT_IN_FOR_OPTIONAL_DEBUG_TRACING)
                 return result;
 
             if (DisallowedTraceMethods.Contains(methodName))
                 return result;
 
             var message = FormatProviders.DokanFormat($"{methodName}('{fileName}', {info}, [{access}], [{share}], [{mode}], [{options}], [{attributes}]) -> {result}");
@@ -413,10 +439,10 @@
             return result;
 #endif
 
-            if (!Core.FileSystem.Constants.OPT_IN_FOR_OPTIONAL_DEBUG_TRACING)
+            if (!FileSystem.Constants.OPT_IN_FOR_OPTIONAL_DEBUG_TRACING)
                 return result;
 
             if (!Debugger.IsAttached)
                 return result;
 
             if (DisallowedTraceMethods.Contains(methodName))
@@ -434,7 +460,7 @@
 
         private static string[] DisallowedTraceMethods { get; } =
         {
-            "GetVolumeInformation"
+            nameof(GetVolumeInformation)
         };
     }
 }

src/Core/SecureFolderFS.Core.Dokany/Callbacks/OnDeviceDokany.cs

@@ -1,4 +1,11 @@
-using DokanNet;
+using System;
+using System.Collections.Generic;
+using System.IO;
+using System.Linq;
+using System.Runtime.InteropServices;
+using System.Security.AccessControl;
+using System.Security.Cryptography;
+using DokanNet;
 using OwlCore.Storage;
 using SecureFolderFS.Core.Dokany.Helpers;
 using SecureFolderFS.Core.Dokany.OpenHandles;
@@ -13,15 +20,6 @@
 using SecureFolderFS.Core.FileSystem.Helpers.RecycleBin.Native;
 using SecureFolderFS.Core.FileSystem.OpenHandles;
 using SecureFolderFS.Storage.VirtualFileSystem;
-using System;
-using System.Collections.Generic;
-using System.Diagnostics;
-using System.IO;
-using System.Linq;
-using System.Runtime.CompilerServices;
-using System.Runtime.InteropServices;
-using System.Security.AccessControl;
-using System.Security.Cryptography;
 using FileAccess = DokanNet.FileAccess;
 
 namespace SecureFolderFS.Core.Dokany.Callbacks
@@ -518,15 +516,26 @@ public override NtStatus DeleteDirectory(string fileName, IDokanFileInfo info)
             if (ciphertextPath is null)
                 return Trace(NtStatus.ObjectPathInvalid, fileName, info);
 
-            using var directoryEnumerator = Directory.EnumerateFileSystemEntries(ciphertextPath).GetEnumerator();
-            while (directoryEnumerator.MoveNext())
+            try
             {
-                // Check for any files except core files
-                canDelete &= PathHelpers.IsCoreName(Path.GetFileName(directoryEnumerator.Current));
+                using var directoryEnumerator = Directory.EnumerateFileSystemEntries(ciphertextPath).GetEnumerator();
+                while (directoryEnumerator.MoveNext())
+                {
+                    // Check for any files except core files
+                    canDelete &= PathHelpers.IsCoreName(Path.GetFileName(directoryEnumerator.Current));
 
-                // If the flag changed (directory is not empty), break the loop
-                if (!canDelete)
-                    break;
+                    // If the flag changed (directory is not empty), break the loop
+                    if (!canDelete)
+                        break;
+                }
+            }
+            catch (DirectoryNotFoundException)
+            {
+                return Trace(DokanResult.PathNotFound, fileName, info);
+            }
+            catch (UnauthorizedAccessException)
+            {
+                return Trace(DokanResult.AccessDenied, fileName, info);
             }
 
             var result = canDelete ? DokanResult.Success : DokanResult.DirectoryNotEmpty;
@@ -575,9 +584,9 @@ public override NtStatus MoveFile(string oldName, string newName, bool replace,
                         return Trace(DokanResult.AccessDenied, fileNameCombined, info);
                     }
 
-                    // File
-                    File.Delete(newCiphertextPath);
-                    File.Move(oldCiphertextPath, newCiphertextPath);
+                    // Replace the destination file atomically. A separate delete and move
+                    // could lose the destination file if the operation is interrupted in between
+                    File.Replace(oldCiphertextPath, newCiphertextPath, null, true);
 
                     return Trace(DokanResult.Success, fileNameCombined, info);
                 }
@@ -758,7 +767,6 @@ public override NtStatus SetFileSecurity(string fileName, FileSystemSecurity sec
         }
 
         /// <inheritdoc/>
-        [MethodImpl(MethodImplOptions.Synchronized)]
         public override NtStatus ReadFile(string fileName, IntPtr buffer, uint bufferLength, out int bytesRead, long offset, IDokanFileInfo info)
         {
             try
@@ -775,18 +783,14 @@ public override NtStatus ReadFile(string fileName, IntPtr buffer, uint bufferLen
 
                 throw;
             }
-            catch (Exception ex)
+            catch (Exception)
             {
-                _ = ex;
                 bytesRead = 0;
-                Debugger.Break();
-
                 return Trace(DokanResult.InternalError, fileName, info);
             }
         }
 
         /// <inheritdoc/>
-        [MethodImpl(MethodImplOptions.Synchronized)]
         public override NtStatus WriteFile(string fileName, IntPtr buffer, uint bufferLength, out int bytesWritten, long offset, IDokanFileInfo info)
         {
             try
@@ -803,12 +807,9 @@ public override NtStatus WriteFile(string fileName, IntPtr buffer, uint bufferLe
 
                 throw;
             }
-            catch (Exception ex)
+            catch (Exception)
             {
-                _ = ex;
                 bytesWritten = 0;
-
-                Debugger.Break();
                 return Trace(DokanResult.InternalError, fileName, info);
             }
         }

src/Core/SecureFolderFS.Core.Dokany/DokanyFileSystem.cs

@@ -54,7 +54,7 @@ public async Task<IVfsRoot> MountAsync(IFolder folder, IDisposable unlockContrac
             if (dokanyOptions.MountPoint is null)
                 throw new DirectoryNotFoundException("No available free mount points for vault file system.");
 
-            var handlesManager = new DokanyHandlesManager(specifics.StreamsAccess, specifics.Options);
+            var handlesManager = new DokanyHandlesManager(wrapper.Inner, specifics.StreamsAccess, specifics.Options);
             var volumeModel = new VolumeModel(specifics.Options.VolumeName, Constants.Dokan.FS_TYPE_ID);
             var dokanyCallbacks = new OnDeviceDokany(specifics, handlesManager, volumeModel);
             var dokanyWrapper = new DokanyWrapper(dokanyCallbacks);