[release/6.0-rc2] File preallocationSize: align Windows and Unix beha…

…vior. (#59532) * Align preallocationSize behavior (#58726) Co-authored-by: Stephen Toub <[email protected]> * File preallocationSize: align Windows and Unix behavior. (#59338) * File preallocationSize: align Windows and Unix behavior. This aligns Windows and Unix behavior of preallocationSize for the intended use-case of specifing the size of a file that will be written. For this use-case, the expected FileAccess is Write, and the file should be a new one (FileMode.Create*) or a truncated file (FileMode.Truncate). Specifing a preallocationSize for other modes, or non-writable files throws ArgumentException. The opened file will have a length of zero, and is ready to be written to by the user. If the requested size cannot be allocated, an IOException is thrown. When the OS/filesystem does not support pre-allocating, preallocationSize is ignored. * fix pal_io preprocessor checks * pal_io more fixes * ctor_options_as.Windows.cs: fix compilation * Update tests * tests: use preallocationSize from all public APIs * pal_io: add back FreeBSD, fix OSX * tests: check allocated is zero when preallocation is not supported. * Only throw for not enough space errors * Fix compilation * Add some more tests * Fix ExtendedPathsAreSupported test * Apply suggestions from code review Co-authored-by: David Cantú <[email protected]> * Update System.Private.CoreLib Strings.resx * PR feedback * Remove GetPathToNonExistingFile * Fix compilation * Skip checking allocated size on mobile platforms. Co-authored-by: David Cantú <[email protected]> * Fix unused fileDescriptor * void fd when unused * Address feedback Co-authored-by: Adam Sitnik <[email protected]> Co-authored-by: Stephen Toub <[email protected]> Co-authored-by: Tom Deseyn <[email protected]>
dotnet · Sep 24, 2021 · 157d591 · 157d591
1 parent dc8ab40
commit 157d591
Show file tree

Hide file tree

Showing 26 changed files with 389 additions and 412 deletions.
diff --git a/...x/System.Native/Interop.PosixFAllocate.cs → ...p/Unix/System.Native/Interop.FAllocate.cs b/...x/System.Native/Interop.PosixFAllocate.cs → ...p/Unix/System.Native/Interop.FAllocate.cs
@@ -9,9 +9,9 @@ internal static partial class Interop
     internal static partial class Sys
     {
         /// <summary>
-        /// Returns -1 on ENOSPC, -2 on EFBIG. On success or ignorable error, 0 is returned.
+        /// Returns -1 on error, 0 on success.
         /// </summary>
-        [DllImport(Libraries.SystemNative, EntryPoint = "SystemNative_PosixFAllocate", SetLastError = false)]
-        internal static extern int PosixFAllocate(SafeFileHandle fd, long offset, long length);
+        [DllImport(Libraries.SystemNative, EntryPoint = "SystemNative_FAllocate", SetLastError = true)]
+        internal static extern int FAllocate(SafeFileHandle fd, long offset, long length);
     }
 }
diff --git a/src/libraries/Native/Unix/Common/pal_config.h.in b/src/libraries/Native/Unix/Common/pal_config.h.in
@@ -36,8 +36,7 @@
 #cmakedefine01 HAVE_STRLCAT
 #cmakedefine01 HAVE_SHM_OPEN_THAT_WORKS_WELL_ENOUGH_WITH_MMAP
 #cmakedefine01 HAVE_POSIX_ADVISE
-#cmakedefine01 HAVE_POSIX_FALLOCATE
-#cmakedefine01 HAVE_POSIX_FALLOCATE64
+#cmakedefine01 HAVE_FALLOCATE
 #cmakedefine01 HAVE_PREADV
 #cmakedefine01 HAVE_PWRITEV
 #cmakedefine01 PRIORITY_REQUIRES_INT_WHO

diff --git a/src/libraries/Native/Unix/System.Native/entrypoints.c b/src/libraries/Native/Unix/System.Native/entrypoints.c
@@ -92,7 +92,7 @@ static const Entry s_sysNative[] =
     DllImportEntry(SystemNative_FTruncate)
     DllImportEntry(SystemNative_Poll)
     DllImportEntry(SystemNative_PosixFAdvise)
-    DllImportEntry(SystemNative_PosixFAllocate)
+    DllImportEntry(SystemNative_FAllocate)
     DllImportEntry(SystemNative_Read)
     DllImportEntry(SystemNative_ReadLink)
     DllImportEntry(SystemNative_Rename)

diff --git a/src/libraries/Native/Unix/System.Native/pal_io.c b/src/libraries/Native/Unix/System.Native/pal_io.c
@@ -1008,83 +1008,35 @@ int32_t SystemNative_PosixFAdvise(intptr_t fd, int64_t offset, int64_t length, i
 #endif
 }
 
-int32_t SystemNative_PosixFAllocate(intptr_t fd, int64_t offset, int64_t length)
+int32_t SystemNative_FAllocate(intptr_t fd, int64_t offset, int64_t length)
 {
     assert_msg(offset == 0, "Invalid offset value", (int)offset);
 
-    int fileDescriptor = ToFileDescriptor(fd);
     int32_t result;
-#if HAVE_POSIX_FALLOCATE64 // 64-bit Linux
-    while ((result = posix_fallocate64(fileDescriptor, (off64_t)offset, (off64_t)length)) == EINTR);
-#elif HAVE_POSIX_FALLOCATE // 32-bit Linux
-    while ((result = posix_fallocate(fileDescriptor, (off_t)offset, (off_t)length)) == EINTR);
+#if HAVE_FALLOCATE // Linux
+    int fileDescriptor = ToFileDescriptor(fd);
+    while ((result = fallocate(fileDescriptor, FALLOC_FL_KEEP_SIZE, (off_t)offset, (off_t)length)) == EINTR);
 #elif defined(F_PREALLOCATE) // macOS
+    int fileDescriptor = ToFileDescriptor(fd);
     fstore_t fstore;
-    fstore.fst_flags = F_ALLOCATECONTIG; // ensure contiguous space
-    fstore.fst_posmode = F_PEOFPOSMODE;  // allocate from the physical end of file, as offset MUST NOT be 0 for F_VOLPOSMODE
+    fstore.fst_flags = F_ALLOCATEALL; // Allocate all requested space or no space at all.
+    fstore.fst_posmode = F_PEOFPOSMODE; // Allocate from the physical end of file.
     fstore.fst_offset = (off_t)offset;
     fstore.fst_length = (off_t)length;
     fstore.fst_bytesalloc = 0; // output size, can be > length
 
     while ((result = fcntl(fileDescriptor, F_PREALLOCATE, &fstore)) == -1 && errno == EINTR);
-
-    if (result == -1)
-    {
-        // we have failed to allocate contiguous space, let's try non-contiguous
-        fstore.fst_flags = F_ALLOCATEALL; // all or nothing
-        while ((result = fcntl(fileDescriptor, F_PREALLOCATE, &fstore)) == -1 && errno == EINTR);
-    }
-#elif defined(F_ALLOCSP) || defined(F_ALLOCSP64) // FreeBSD
-    #if HAVE_FLOCK64
-    struct flock64 lockArgs;
-    int command = F_ALLOCSP64;
-    #else
-    struct flock lockArgs;
-    int command = F_ALLOCSP;
-    #endif
-
-    lockArgs.l_whence = SEEK_SET;
-    lockArgs.l_start = (off_t)offset;
-    lockArgs.l_len = (off_t)length;
-
-    while ((result = fcntl(fileDescriptor, command, &lockArgs)) == -1 && errno == EINTR);
+#else
+    (void)fd; // unused
+    (void)offset; // unused
+    (void)length; // unused
+    result = -1;
+    errno = EOPNOTSUPP;
 #endif
 
-#if defined(F_PREALLOCATE) || defined(F_ALLOCSP) || defined(F_ALLOCSP64)
-    // most of the Unixes implement posix_fallocate which does NOT set the last error
-    // fctnl does, but to mimic the posix_fallocate behaviour we just return error
-    if (result == -1)
-    {
-        result = errno;
-    }
-    else
-    {
-        // align the behaviour with what posix_fallocate does (change reported file size)
-        ftruncate(fileDescriptor, length);
-    }
-#endif
+    assert(result == 0 || errno != EINVAL);
 
-    // error codes can be OS-specific, so this is why this handling is done here rather than in the managed layer
-    switch (result)
-    {
-        case ENOSPC: // there was not enough space
-            return -1;
-        case EFBIG: // the file was too large
-            return -2;
-        case ENODEV: // not a regular file
-        case ESPIPE: // a pipe
-            // We ignore it, as FileStream contract makes it clear that allocationSize is ignored for non-regular files.
-            return 0;
-        case EINVAL:
-            // We control the offset and length so they are correct.
-            assert_msg(length >= 0, "Invalid length value", (int)length);
-            // But if the underlying filesystem does not support the operation, we just ignore it and treat as a hint.
-            return 0;
-        default:
-            assert(result != EINTR); // it can't happen here as we retry the call on EINTR
-            assert(result != EBADF); // it can't happen here as this method is being called after a succesfull call to open (with write permissions) before returning the SafeFileHandle to the user
-            return 0;
-    }
+    return result;
 }
 
 int32_t SystemNative_Read(intptr_t fd, void* buffer, int32_t bufferSize)

diff --git a/src/libraries/Native/Unix/System.Native/pal_io.h b/src/libraries/Native/Unix/System.Native/pal_io.h
@@ -620,11 +620,11 @@ PALEXPORT int32_t SystemNative_Poll(PollEvent* pollEvents, uint32_t eventCount,
 PALEXPORT int32_t SystemNative_PosixFAdvise(intptr_t fd, int64_t offset, int64_t length, int32_t advice);
 
 /**
- * Ensures that disk space is allocated.
+ * Preallocates disk space.
  *
- * Returns -1 on ENOSPC, -2 on EFBIG. On success or ignorable error, 0 is returned.
+ * Returns 0 for success, -1 for failure. Sets errno on failure.
  */
-PALEXPORT int32_t SystemNative_PosixFAllocate(intptr_t fd, int64_t offset, int64_t length);
+PALEXPORT int32_t SystemNative_FAllocate(intptr_t fd, int64_t offset, int64_t length);
 
 /**
  * Reads the number of bytes specified into the provided buffer from the specified, opened file descriptor.

diff --git a/src/libraries/Native/Unix/configure.cmake b/src/libraries/Native/Unix/configure.cmake
@@ -232,14 +232,9 @@ check_symbol_exists(
     HAVE_POSIX_ADVISE)
 
 check_symbol_exists(
-    posix_fallocate
+    fallocate
     fcntl.h
-    HAVE_POSIX_FALLOCATE)
-
-check_symbol_exists(
-    posix_fallocate64
-    fcntl.h
-    HAVE_POSIX_FALLOCATE64)
+    HAVE_FALLOCATE)
 
 check_symbol_exists(
     preadv

diff --git a/src/libraries/System.IO.FileSystem/tests/File/Open.cs b/src/libraries/System.IO.FileSystem/tests/File/Open.cs
@@ -42,15 +42,14 @@ protected override FileStream CreateFileStream(string path, FileMode mode, FileA
         }
     }
 
-    public class File_Open_str_options_as : FileStream_ctor_options_as
+    public class File_Open_str_options : FileStream_ctor_options
     {
         protected override FileStream CreateFileStream(string path, FileMode mode)
         {
             return File.Open(path,
                 new FileStreamOptions {
                     Mode = mode,
-                    Access = mode == FileMode.Append ? FileAccess.Write : FileAccess.ReadWrite,
-                    PreallocationSize = PreallocationSize
+                    Access = mode == FileMode.Append ? FileAccess.Write : FileAccess.ReadWrite
                 });
         }
 
@@ -59,12 +58,23 @@ protected override FileStream CreateFileStream(string path, FileMode mode, FileA
             return File.Open(path,
                 new FileStreamOptions {
                     Mode = mode,
-                    Access = access,
-                    PreallocationSize = PreallocationSize
+                    Access = access
                 });
         }
 
         protected override FileStream CreateFileStream(string path, FileMode mode, FileAccess access, FileShare share, int bufferSize, FileOptions options)
+        {
+            return File.Open(path,
+                new FileStreamOptions {
+                    Mode = mode,
+                    Access = access,
+                    Share = share,
+                    Options = options,
+                    BufferSize = bufferSize
+                });
+        }
+
+        protected override FileStream CreateFileStream(string path, FileMode mode, FileAccess access, FileShare share, int bufferSize, FileOptions options, long preallocationSize)
         {
             return File.Open(path,
                 new FileStreamOptions {
@@ -73,7 +83,7 @@ protected override FileStream CreateFileStream(string path, FileMode mode, FileA
                     Share = share,
                     Options = options,
                     BufferSize = bufferSize,
-                    PreallocationSize = PreallocationSize
+                    PreallocationSize = preallocationSize
                 });
         }
     }

diff --git a/src/libraries/System.IO.FileSystem/tests/File/OpenHandle.cs b/src/libraries/System.IO.FileSystem/tests/File/OpenHandle.cs
@@ -7,28 +7,24 @@
 namespace System.IO.Tests
 {
     // to avoid a lot of code duplication, we reuse FileStream tests
-    public class File_OpenHandle : FileStream_ctor_options_as
+    public class File_OpenHandle : FileStream_ctor_options
     {
         protected override string GetExpectedParamName(string paramName) => paramName;
 
         protected override FileStream CreateFileStream(string path, FileMode mode)
         {
             FileAccess access = mode == FileMode.Append ? FileAccess.Write : FileAccess.ReadWrite;
-            return new FileStream(File.OpenHandle(path, mode, access, preallocationSize: PreallocationSize), access);
+            return new FileStream(File.OpenHandle(path, mode, access), access);
         }
 
         protected override FileStream CreateFileStream(string path, FileMode mode, FileAccess access)
-            => new FileStream(File.OpenHandle(path, mode, access, preallocationSize: PreallocationSize), access);
+            => new FileStream(File.OpenHandle(path, mode, access), access);
 
         protected override FileStream CreateFileStream(string path, FileMode mode, FileAccess access, FileShare share, int bufferSize, FileOptions options)
-            => new FileStream(File.OpenHandle(path, mode, access, share, options, PreallocationSize), access, bufferSize, (options & FileOptions.Asynchronous) != 0);
+            => new FileStream(File.OpenHandle(path, mode, access, share, options), access, bufferSize, (options & FileOptions.Asynchronous) != 0);
 
-        [Fact]
-        public override void NegativePreallocationSizeThrows()
-        {
-            ArgumentOutOfRangeException ex = Assert.Throws<ArgumentOutOfRangeException>(
-                () => File.OpenHandle("validPath", FileMode.CreateNew, FileAccess.Write, FileShare.None, FileOptions.None, preallocationSize: -1));
-        }
+        protected override FileStream CreateFileStream(string path, FileMode mode, FileAccess access, FileShare share, int bufferSize, FileOptions options, long preallocationSize)
+            => new FileStream(File.OpenHandle(path, mode, access, share, options, preallocationSize), access, bufferSize, (options & FileOptions.Asynchronous) != 0);
 
         [ActiveIssue("https://github.com/dotnet/runtime/issues/53432")]
         [Theory, MemberData(nameof(StreamSpecifiers))]

diff --git a/src/libraries/System.IO.FileSystem/tests/FileInfo/Open.cs b/src/libraries/System.IO.FileSystem/tests/FileInfo/Open.cs
@@ -74,15 +74,14 @@ protected override FileStream CreateFileStream(string path, FileMode mode, FileA
         }
     }
 
-    public class FileInfo_Open_options_as : FileStream_ctor_options_as
+    public class FileInfo_Open_options : FileStream_ctor_options
     {
         protected override FileStream CreateFileStream(string path, FileMode mode)
         {
             return new FileInfo(path).Open(
                 new FileStreamOptions {
                     Mode = mode,
-                    Access = mode == FileMode.Append ? FileAccess.Write : FileAccess.ReadWrite,
-                    PreallocationSize = PreallocationSize
+                    Access = mode == FileMode.Append ? FileAccess.Write : FileAccess.ReadWrite
                 });
         }
 
@@ -91,12 +90,23 @@ protected override FileStream CreateFileStream(string path, FileMode mode, FileA
             return new FileInfo(path).Open(
                 new FileStreamOptions {
                     Mode = mode,
-                    Access = access,
-                    PreallocationSize = PreallocationSize
+                    Access = access
                 });
         }
 
         protected override FileStream CreateFileStream(string path, FileMode mode, FileAccess access, FileShare share, int bufferSize, FileOptions options)
+        {
+            return new FileInfo(path).Open(
+                new FileStreamOptions {
+                    Mode = mode,
+                    Access = access,
+                    Share = share,
+                    Options = options,
+                    BufferSize = bufferSize
+                });
+        }
+
+        protected override FileStream CreateFileStream(string path, FileMode mode, FileAccess access, FileShare share, int bufferSize, FileOptions options, long preallocationSize)
         {
             return new FileInfo(path).Open(
                 new FileStreamOptions {
@@ -105,7 +115,7 @@ protected override FileStream CreateFileStream(string path, FileMode mode, FileA
                     Share = share,
                     Options = options,
                     BufferSize = bufferSize,
-                    PreallocationSize = PreallocationSize
+                    PreallocationSize = preallocationSize
                 });
         }
     }

diff --git a/src/libraries/System.IO.FileSystem/tests/FileStream/ctor_options.Browser.cs b/src/libraries/System.IO.FileSystem/tests/FileStream/ctor_options.Browser.cs
@@ -0,0 +1,17 @@
+// Licensed to the .NET Foundation under one or more agreements.
+// The .NET Foundation licenses this file to you under the MIT license.
+
+namespace System.IO.Tests
+{
+    public partial class FileStream_ctor_options
+    {
+        private static long GetAllocatedSize(FileStream fileStream)
+        {
+            return 0;
+        }
+
+        private static bool SupportsPreallocation => false;
+
+        private static bool IsGetAllocatedSizeImplemented => false;
+    }
+}
diff --git a/src/libraries/System.IO.FileSystem/tests/FileStream/ctor_options.Unix.cs b/src/libraries/System.IO.FileSystem/tests/FileStream/ctor_options.Unix.cs
@@ -0,0 +1,36 @@
+// Licensed to the .NET Foundation under one or more agreements.
+// The .NET Foundation licenses this file to you under the MIT license.
+
+using System.Diagnostics;
+using System.Runtime.InteropServices;
+
+namespace System.IO.Tests
+{
+    public partial class FileStream_ctor_options
+    {
+        private static long GetAllocatedSize(FileStream fileStream)
+        {
+            bool isOSX = RuntimeInformation.IsOSPlatform(OSPlatform.OSX);
+            // Call 'stat' to get the number of blocks, and size of blocks.
+            using var px = Process.Start(new ProcessStartInfo
+            {
+                FileName = "stat",
+                ArgumentList = { isOSX ? "-f"    : "-c",
+                                 isOSX ? "%b %k" : "%b %B",
+                                 fileStream.Name },
+                RedirectStandardOutput = true
+            });
+            string stdout = px.StandardOutput.ReadToEnd();
+
+            string[] parts = stdout.Split(' ');
+            return long.Parse(parts[0]) * long.Parse(parts[1]);
+        }
+
+        private static bool SupportsPreallocation =>
+            RuntimeInformation.IsOSPlatform(OSPlatform.Linux) ||
+            RuntimeInformation.IsOSPlatform(OSPlatform.OSX);
+
+        // Mobile platforms don't support Process.Start.
+        private static bool IsGetAllocatedSizeImplemented => !PlatformDetection.IsMobile;
+    }
+}