From: David Cantú Date: Fri, 24 Sep 2021 21:52:17 +0000 (-0700) Subject: [release/6.0-rc2] File preallocationSize: align Windows and Unix behavior. (#59532) X-Git-Tag: accepted/tizen/unified/20220110.054933~92^2~6 X-Git-Url: http://review.tizen.org/git/?a=commitdiff_plain;h=157d5915a0e3d0f59ccc57b7dcdd81a1da7bba8b;p=platform%2Fupstream%2Fdotnet%2Fruntime.git [release/6.0-rc2] File preallocationSize: align Windows and Unix behavior. (#59532) * Align preallocationSize behavior (#58726) Co-authored-by: Stephen Toub * 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ú * Update System.Private.CoreLib Strings.resx * PR feedback * Remove GetPathToNonExistingFile * Fix compilation * Skip checking allocated size on mobile platforms. Co-authored-by: David Cantú * Fix unused fileDescriptor * void fd when unused * Address feedback Co-authored-by: Adam Sitnik Co-authored-by: Stephen Toub Co-authored-by: Tom Deseyn --- diff --git a/src/libraries/Common/src/Interop/Unix/System.Native/Interop.PosixFAllocate.cs b/src/libraries/Common/src/Interop/Unix/System.Native/Interop.FAllocate.cs similarity index 63% rename from src/libraries/Common/src/Interop/Unix/System.Native/Interop.PosixFAllocate.cs rename to src/libraries/Common/src/Interop/Unix/System.Native/Interop.FAllocate.cs index 2866ed3..71155e0 100644 --- a/src/libraries/Common/src/Interop/Unix/System.Native/Interop.PosixFAllocate.cs +++ b/src/libraries/Common/src/Interop/Unix/System.Native/Interop.FAllocate.cs @@ -9,9 +9,9 @@ internal static partial class Interop internal static partial class Sys { /// - /// Returns -1 on ENOSPC, -2 on EFBIG. On success or ignorable error, 0 is returned. + /// Returns -1 on error, 0 on success. /// - [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 index d50c8d6..9cffccb 100644 --- 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 index 721be5e..702ead8 100644 --- 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 index d86e8fe..049e44e 100644 --- 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 index 3f0db05..0e5d4ca 100644 --- 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 index fbfd5a9..e1afadb 100644 --- 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 index d014cf0..bd9b38b 100644 --- a/src/libraries/System.IO.FileSystem/tests/File/Open.cs +++ b/src/libraries/System.IO.FileSystem/tests/File/Open.cs @@ -42,15 +42,14 @@ namespace System.IO.Tests } } - 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,8 +58,7 @@ namespace System.IO.Tests return File.Open(path, new FileStreamOptions { Mode = mode, - Access = access, - PreallocationSize = PreallocationSize + Access = access }); } @@ -72,8 +70,20 @@ namespace System.IO.Tests 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 { + Mode = mode, + Access = access, + 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 index a8b3619..120ed9a 100644 --- a/src/libraries/System.IO.FileSystem/tests/File/OpenHandle.cs +++ b/src/libraries/System.IO.FileSystem/tests/File/OpenHandle.cs @@ -7,28 +7,24 @@ using Xunit; 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( - () => 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 index 2324a92..36fd39a6 100644 --- a/src/libraries/System.IO.FileSystem/tests/FileInfo/Open.cs +++ b/src/libraries/System.IO.FileSystem/tests/FileInfo/Open.cs @@ -74,15 +74,14 @@ namespace System.IO.Tests } } - 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,8 +90,7 @@ namespace System.IO.Tests return new FileInfo(path).Open( new FileStreamOptions { Mode = mode, - Access = access, - PreallocationSize = PreallocationSize + Access = access }); } @@ -104,8 +102,20 @@ namespace System.IO.Tests 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 { + Mode = mode, + Access = access, + 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 new file mode 100644 index 0000000..45326d8 --- /dev/null +++ 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 new file mode 100644 index 0000000..c33c56a --- /dev/null +++ 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; + } +} diff --git a/src/libraries/System.IO.FileSystem/tests/FileStream/ctor_options_as.Windows.cs b/src/libraries/System.IO.FileSystem/tests/FileStream/ctor_options.Windows.cs similarity index 71% rename from src/libraries/System.IO.FileSystem/tests/FileStream/ctor_options_as.Windows.cs rename to src/libraries/System.IO.FileSystem/tests/FileStream/ctor_options.Windows.cs index bde8cd5..2dca6ba 100644 --- a/src/libraries/System.IO.FileSystem/tests/FileStream/ctor_options_as.Windows.cs +++ b/src/libraries/System.IO.FileSystem/tests/FileStream/ctor_options.Windows.cs @@ -1,21 +1,17 @@ // Licensed to the .NET Foundation under one or more agreements. // The .NET Foundation licenses this file to you under the MIT license. +using System.IO.Pipes; using System.Runtime.InteropServices; using System.Text; +using System.Threading.Tasks; using Xunit; namespace System.IO.Tests { - public partial class FileStream_ctor_options_as : FileStream_ctor_options_as_base + public partial class FileStream_ctor_options { - protected override long PreallocationSize => 10; - - protected override long InitialLength => 0; // Windows modifies AllocationSize, but not EndOfFile (file length) - - private long GetExpectedFileLength(long preallocationSize) => 0; // Windows modifies AllocationSize, but not EndOfFile (file length) - - private unsafe long GetActualPreallocationSize(FileStream fileStream) + private unsafe long GetAllocatedSize(FileStream fileStream) { Interop.Kernel32.FILE_STANDARD_INFO info; @@ -24,6 +20,10 @@ namespace System.IO.Tests return info.AllocationSize; } + private static bool SupportsPreallocation => true; + + private static bool IsGetAllocatedSizeImplemented => true; + [Theory] [InlineData(@"\\?\")] [InlineData(@"\??\")] @@ -32,26 +32,26 @@ namespace System.IO.Tests { const long preallocationSize = 123; - string filePath = prefix + Path.GetFullPath(GetPathToNonExistingFile()); + string filePath = prefix + Path.GetFullPath(GetTestFilePath()); - using (var fs = new FileStream(filePath, GetOptions(FileMode.CreateNew, FileAccess.Write, FileShare.None, FileOptions.None, preallocationSize))) + using (var fs = CreateFileStream(filePath, FileMode.CreateNew, FileAccess.Write, FileShare.None, bufferSize: 4096, FileOptions.None, preallocationSize)) { - Assert.True(GetActualPreallocationSize(fs) >= preallocationSize, $"Provided {preallocationSize}, actual: {GetActualPreallocationSize(fs)}"); + Assert.Equal(0, fs.Length); + Assert.True(GetAllocatedSize(fs) >= preallocationSize); } } [ConditionalTheory(nameof(IsFat32))] [InlineData(FileMode.Create)] [InlineData(FileMode.CreateNew)] - [InlineData(FileMode.OpenOrCreate)] public void WhenFileIsTooLargeTheErrorMessageContainsAllDetails(FileMode mode) { const long tooMuch = uint.MaxValue + 1L; // more than FAT32 max size - string filePath = GetPathToNonExistingFile(); + string filePath = GetTestFilePath(); Assert.StartsWith(Path.GetTempPath(), filePath); // this is what IsFat32 method relies on - IOException ex = Assert.Throws(() => new FileStream(filePath, GetOptions(mode, FileAccess.Write, FileShare.None, FileOptions.None, tooMuch))); + IOException ex = Assert.Throws(() => CreateFileStream(filePath, mode, FileAccess.Write, FileShare.None, bufferSize: 4096, FileOptions.None, tooMuch)); Assert.Contains(filePath, ex.Message); Assert.Contains(tooMuch.ToString(), ex.Message); diff --git a/src/libraries/System.IO.FileSystem/tests/FileStream/ctor_options.cs b/src/libraries/System.IO.FileSystem/tests/FileStream/ctor_options.cs new file mode 100644 index 0000000..5209d5e --- /dev/null +++ b/src/libraries/System.IO.FileSystem/tests/FileStream/ctor_options.cs @@ -0,0 +1,172 @@ +// Licensed to the .NET Foundation under one or more agreements. +// The .NET Foundation licenses this file to you under the MIT license. + +using System.Linq; +using System.Security.Cryptography; +using Xunit; + +namespace System.IO.Tests +{ + // Don't run in parallel as the WhenDiskIsFullTheErrorMessageContainsAllDetails test + // consumes entire available free space on the disk (only on Linux, this is how fallocate works) + // and if we try to run other disk-writing test in the meantime we are going to get "No space left on device" exception. + [Collection("NoParallelTests")] + public partial class FileStream_ctor_options : FileStream_ctor_str_fm_fa_fs_buffer_fo + { + protected override string GetExpectedParamName(string paramName) => "value"; + + protected override FileStream CreateFileStream(string path, FileMode mode) + => new FileStream(path, + new FileStreamOptions + { + Mode = mode, + Access = mode == FileMode.Append ? FileAccess.Write : FileAccess.ReadWrite + }); + + protected override FileStream CreateFileStream(string path, FileMode mode, FileAccess access) + => new FileStream(path, + new FileStreamOptions + { + Mode = mode, + Access = access + }); + + protected override FileStream CreateFileStream(string path, FileMode mode, FileAccess access, FileShare share, int bufferSize, FileOptions options) + => new FileStream(path, + new FileStreamOptions + { + Mode = mode, + Access = access, + Share = share, + BufferSize = bufferSize, + Options = options + }); + + protected virtual FileStream CreateFileStream(string path, FileMode mode, FileAccess access, FileShare share, int bufferSize, FileOptions options, long preallocationSize) + => new FileStream(path, + new FileStreamOptions + { + Mode = mode, + Access = access, + Share = share, + BufferSize = bufferSize, + Options = options, + PreallocationSize = preallocationSize + }); + + [Fact] + public virtual void NegativePreallocationSizeThrows() + { + string filePath = GetTestFilePath(); + ArgumentOutOfRangeException ex = Assert.Throws( + () => CreateFileStream(filePath, FileMode.CreateNew, FileAccess.Write, FileShare.None, bufferSize: 1, FileOptions.None, preallocationSize: -1)); + } + + [Theory] + [InlineData(FileMode.Append)] + [InlineData(FileMode.Open)] + [InlineData(FileMode.OpenOrCreate)] + [InlineData(FileMode.Truncate)] + public void PreallocationSizeThrowsForFileModesThatOpenExistingFiles(FileMode mode) + { + Assert.Throws( + () => CreateFileStream(GetTestFilePath(), mode, FileAccess.Write, FileShare.None, bufferSize: 1, FileOptions.None, preallocationSize: 20)); + } + + [Theory] + [InlineData(FileMode.Create)] + [InlineData(FileMode.CreateNew)] + public void PreallocationSizeThrowsForReadOnlyAccess(FileMode mode) + { + Assert.Throws( + () => CreateFileStream(GetTestFilePath(), mode, FileAccess.Read, FileShare.None, bufferSize: 1, FileOptions.None, preallocationSize: 20)); + } + + [Theory] + [InlineData(FileMode.Create, false)] + [InlineData(FileMode.Create, true)] + [InlineData(FileMode.CreateNew, false)] + [InlineData(FileMode.Append, false)] + [InlineData(FileMode.Append, true)] + [InlineData(FileMode.Open, true)] + [InlineData(FileMode.OpenOrCreate, true)] + [InlineData(FileMode.OpenOrCreate, false)] + [InlineData(FileMode.Truncate, true)] + public void ZeroPreallocationSizeDoesNotAllocate(FileMode mode, bool createFile) + { + string filename = GetTestFilePath(); + + if (createFile) + { + File.WriteAllText(filename, ""); + } + + using (FileStream fs = CreateFileStream(filename, mode, FileAccess.Write, FileShare.None, bufferSize: 1, FileOptions.None, preallocationSize: 0)) + { + Assert.Equal(0, fs.Length); + if (IsGetAllocatedSizeImplemented) + { + Assert.Equal(0, GetAllocatedSize(fs)); + } + Assert.Equal(0, fs.Position); + } + } + + [Theory] + [InlineData(FileAccess.Write, FileMode.Create)] + [InlineData(FileAccess.Write, FileMode.CreateNew)] + [InlineData(FileAccess.ReadWrite, FileMode.Create)] + [InlineData(FileAccess.ReadWrite, FileMode.CreateNew)] + public void PreallocationSize(FileAccess access, FileMode mode) + { + const long preallocationSize = 123; + + using (var fs = CreateFileStream(GetTestFilePath(), mode, access, FileShare.None, bufferSize: 1, FileOptions.None, preallocationSize)) + { + Assert.Equal(0, fs.Length); + if (IsGetAllocatedSizeImplemented) + { + if (SupportsPreallocation) + { + Assert.True(GetAllocatedSize(fs) >= preallocationSize); + } + else + { + Assert.Equal(0, GetAllocatedSize(fs)); + } + } + Assert.Equal(0, fs.Position); + } + } + + [OuterLoop("Might allocate 1 TB file if there is enough space on the disk")] + // macOS fcntl doc does not mention ENOSPC error: https://developer.apple.com/library/archive/documentation/System/Conceptual/ManPages_iPhoneOS/man2/fcntl.2.html + // But depending on the OS version, it might actually return it. + // Since we don't want to have unstable tests, it's better to not run it on macOS at all. + [PlatformSpecific(TestPlatforms.Windows | TestPlatforms.Linux)] + [Theory] + [InlineData(FileMode.Create)] + [InlineData(FileMode.CreateNew)] + public void WhenDiskIsFullTheErrorMessageContainsAllDetails(FileMode mode) + { + const long TooMuch = 1024L * 1024L * 1024L * 1024L; // 1 TB + + string filePath = GetTestFilePath(); + + IOException ex = Assert.Throws(() => CreateFileStream(filePath, mode, FileAccess.Write, FileShare.None, bufferSize: 1, FileOptions.None, TooMuch)); + Assert.Contains(filePath, ex.Message); + Assert.Contains(TooMuch.ToString(), ex.Message); + + // ensure it was NOT created + bool exists = File.Exists(filePath); + if (exists) + { + File.Delete(filePath); + } + Assert.False(exists); + } + } + + [CollectionDefinition("NoParallelTests", DisableParallelization = true)] + public partial class NoParallelTests { } +} diff --git a/src/libraries/System.IO.FileSystem/tests/FileStream/ctor_options_as.Browser.cs b/src/libraries/System.IO.FileSystem/tests/FileStream/ctor_options_as.Browser.cs deleted file mode 100644 index 3475c89..0000000 --- a/src/libraries/System.IO.FileSystem/tests/FileStream/ctor_options_as.Browser.cs +++ /dev/null @@ -1,16 +0,0 @@ -// 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_as : FileStream_ctor_options_as_base - { - protected override long PreallocationSize => 10; - - protected override long InitialLength => 10; - - private long GetExpectedFileLength(long preallocationSize) => preallocationSize; - - private long GetActualPreallocationSize(FileStream fileStream) => fileStream.Length; - } -} diff --git a/src/libraries/System.IO.FileSystem/tests/FileStream/ctor_options_as.Unix.cs b/src/libraries/System.IO.FileSystem/tests/FileStream/ctor_options_as.Unix.cs deleted file mode 100644 index 12e8f16..0000000 --- a/src/libraries/System.IO.FileSystem/tests/FileStream/ctor_options_as.Unix.cs +++ /dev/null @@ -1,21 +0,0 @@ -// 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_as : FileStream_ctor_options_as_base - { - protected override long PreallocationSize => 10; - - protected override long InitialLength => 10; - - private long GetExpectedFileLength(long preallocationSize) => preallocationSize; - - private long GetActualPreallocationSize(FileStream fileStream) - { - // On Unix posix_fallocate modifies file length and we are using fstat to get it for verificaiton - Interop.Sys.FStat(fileStream.SafeFileHandle, out Interop.Sys.FileStatus fileStatus); - return fileStatus.Size; - } - } -} diff --git a/src/libraries/System.IO.FileSystem/tests/FileStream/ctor_options_as.cs b/src/libraries/System.IO.FileSystem/tests/FileStream/ctor_options_as.cs deleted file mode 100644 index f36e55e..0000000 --- a/src/libraries/System.IO.FileSystem/tests/FileStream/ctor_options_as.cs +++ /dev/null @@ -1,212 +0,0 @@ -// Licensed to the .NET Foundation under one or more agreements. -// The .NET Foundation licenses this file to you under the MIT license. - -using Xunit; - -namespace System.IO.Tests -{ - public abstract class FileStream_ctor_options_as_base : FileStream_ctor_str_fm_fa_fs_buffer_fo - { - protected abstract long PreallocationSize { get; } - - protected override string GetExpectedParamName(string paramName) => "value"; - - protected override FileStream CreateFileStream(string path, FileMode mode) - => new FileStream(path, - new FileStreamOptions - { - Mode = mode, - Access = mode == FileMode.Append ? FileAccess.Write : FileAccess.ReadWrite, - PreallocationSize = PreallocationSize - }); - - protected override FileStream CreateFileStream(string path, FileMode mode, FileAccess access) - => new FileStream(path, - new FileStreamOptions - { - Mode = mode, - Access = access, - PreallocationSize = PreallocationSize - }); - - protected override FileStream CreateFileStream(string path, FileMode mode, FileAccess access, FileShare share, int bufferSize, FileOptions options) - => new FileStream(path, - new FileStreamOptions - { - Mode = mode, - Access = access, - Share = share, - BufferSize = bufferSize, - Options = options, - PreallocationSize = PreallocationSize - }); - - protected FileStreamOptions GetOptions(FileMode mode, FileAccess access, FileShare share, FileOptions options, long preAllocationSize) - => new FileStreamOptions - { - Mode = mode, - Access = access, - Share = share, - Options = options, - PreallocationSize = preAllocationSize - }; - } - - public class FileStream_ctor_options_as_zero : FileStream_ctor_options_as_base - { - protected override long PreallocationSize => 0; // specifying 0 should have no effect - - protected override long InitialLength => 0; - } - - [CollectionDefinition("NoParallelTests", DisableParallelization = true)] - public partial class NoParallelTests { } - - // Don't run in parallel as the WhenDiskIsFullTheErrorMessageContainsAllDetails test - // consumes entire available free space on the disk (only on Linux, this is how posix_fallocate works) - // and if we try to run other disk-writing test in the meantime we are going to get "No space left on device" exception. - [Collection("NoParallelTests")] - public partial class FileStream_ctor_options_as : FileStream_ctor_options_as_base - { - [Fact] - public virtual void NegativePreallocationSizeThrows() - { - string filePath = GetPathToNonExistingFile(); - ArgumentOutOfRangeException ex = Assert.Throws( - () => new FileStream(filePath, GetOptions(FileMode.CreateNew, FileAccess.Write, FileShare.None, FileOptions.None, -1))); - } - - [Theory] - [InlineData(FileMode.Create, 0L)] - [InlineData(FileMode.CreateNew, 0L)] - [InlineData(FileMode.OpenOrCreate, 0L)] - public void WhenFileIsCreatedWithoutPreallocationSizeSpecifiedThePreallocationSizeIsNotSet(FileMode mode, long preallocationSize) - { - using (var fs = new FileStream(GetPathToNonExistingFile(), GetOptions(mode, FileAccess.Write, FileShare.None, FileOptions.None, preallocationSize))) - { - Assert.Equal(0, GetActualPreallocationSize(fs)); - Assert.Equal(0, fs.Length); - Assert.Equal(0, fs.Position); - } - } - - [Theory] - [InlineData(FileMode.Open, 0L)] - [InlineData(FileMode.Open, 1L)] - [InlineData(FileMode.OpenOrCreate, 0L)] - [InlineData(FileMode.OpenOrCreate, 1L)] - [InlineData(FileMode.Append, 0L)] - [InlineData(FileMode.Append, 1L)] - public void WhenExistingFileIsBeingOpenedWithPreallocationSizeSpecifiedThePreallocationSizeIsNotChanged(FileMode mode, long preallocationSize) - { - const int initialSize = 1; - string filePath = GetPathToNonExistingFile(); - File.WriteAllBytes(filePath, new byte[initialSize]); - long initialPreallocationSize; - - using (var fs = new FileStream(filePath, GetOptions(mode, FileAccess.Write, FileShare.None, FileOptions.None, 0))) // preallocationSize NOT provided - { - initialPreallocationSize = GetActualPreallocationSize(fs); // just read it to ensure it's not being changed - } - - using (var fs = new FileStream(filePath, GetOptions(mode, FileAccess.Write, FileShare.None, FileOptions.None, preallocationSize))) - { - Assert.Equal(initialPreallocationSize, GetActualPreallocationSize(fs)); // it has NOT been changed - Assert.Equal(initialSize, fs.Length); - Assert.Equal(mode == FileMode.Append ? initialSize : 0, fs.Position); - } - } - - [Theory] - [InlineData(FileMode.Create)] - [InlineData(FileMode.CreateNew)] - [InlineData(FileMode.OpenOrCreate)] - public void WhenFileIsCreatedWithPreallocationSizeSpecifiedThePreallocationSizeIsSet(FileMode mode) - { - const long preallocationSize = 123; - - using (var fs = new FileStream(GetPathToNonExistingFile(), GetOptions(mode, FileAccess.Write, FileShare.None, FileOptions.None, preallocationSize))) - { - // OS might allocate MORE than we have requested - Assert.True(GetActualPreallocationSize(fs) >= preallocationSize, $"Provided {preallocationSize}, actual: {GetActualPreallocationSize(fs)}"); - Assert.Equal(GetExpectedFileLength(preallocationSize), fs.Length); - Assert.Equal(0, fs.Position); - } - } - - [OuterLoop("Might allocate 1 TB file if there is enough space on the disk")] - // macOS fcntl doc does not mention ENOSPC error: https://developer.apple.com/library/archive/documentation/System/Conceptual/ManPages_iPhoneOS/man2/fcntl.2.html - // But depending on the OS version, it might actually return it. - // Since we don't want to have unstable tests, it's better to not run it on macOS at all. - [PlatformSpecific(TestPlatforms.Windows | TestPlatforms.Linux)] - [Theory] - [InlineData(FileMode.Create)] - [InlineData(FileMode.CreateNew)] - [InlineData(FileMode.OpenOrCreate)] - public void WhenDiskIsFullTheErrorMessageContainsAllDetails(FileMode mode) - { - const long tooMuch = 1024L * 1024L * 1024L * 1024L; // 1 TB - - string filePath = GetPathToNonExistingFile(); - - IOException ex = Assert.Throws(() => new FileStream(filePath, GetOptions(mode, FileAccess.Write, FileShare.None, FileOptions.None, tooMuch))); - Assert.Contains(filePath, ex.Message); - Assert.Contains(tooMuch.ToString(), ex.Message); - - // ensure it was NOT created (provided OOTB by Windows, emulated on Unix) - bool exists = File.Exists(filePath); - if (exists) - { - File.Delete(filePath); - } - Assert.False(exists); - } - - [Fact] - public void WhenFileIsTruncatedWithoutPreallocationSizeSpecifiedThePreallocationSizeIsNotSet() - { - const int initialSize = 10_000; - - string filePath = GetPathToNonExistingFile(); - File.WriteAllBytes(filePath, new byte[initialSize]); - - using (var fs = new FileStream(filePath, GetOptions(FileMode.Truncate, FileAccess.Write, FileShare.None, FileOptions.None, 0))) - { - Assert.Equal(0, GetActualPreallocationSize(fs)); - Assert.Equal(0, fs.Length); - Assert.Equal(0, fs.Position); - } - } - - [Fact] - public void WhenFileIsTruncatedWithPreallocationSizeSpecifiedThePreallocationSizeIsSet() - { - const int initialSize = 10_000; // this must be more than 4kb which seems to be minimum allocation size on Windows - const long preallocationSize = 100; - - string filePath = GetPathToNonExistingFile(); - File.WriteAllBytes(filePath, new byte[initialSize]); - - using (var fs = new FileStream(filePath, GetOptions(FileMode.Truncate, FileAccess.Write, FileShare.None, FileOptions.None, preallocationSize))) - { - Assert.True(GetActualPreallocationSize(fs) >= preallocationSize, $"Provided {preallocationSize}, actual: {GetActualPreallocationSize(fs)}"); - // less than initial file size (file got truncated) - Assert.True(GetActualPreallocationSize(fs) < initialSize, $"initialSize {initialSize}, actual: {GetActualPreallocationSize(fs)}"); - Assert.Equal(GetExpectedFileLength(preallocationSize), fs.Length); - Assert.Equal(0, fs.Position); - } - } - - private string GetPathToNonExistingFile() - { - string filePath = GetTestFilePath(); - - if (File.Exists(filePath)) - { - File.Delete(filePath); - } - - return filePath; - } - } -} diff --git a/src/libraries/System.IO.FileSystem/tests/FileStream/ctor_str_fm.cs b/src/libraries/System.IO.FileSystem/tests/FileStream/ctor_str_fm.cs index 47e3b49..cd3b18d 100644 --- a/src/libraries/System.IO.FileSystem/tests/FileStream/ctor_str_fm.cs +++ b/src/libraries/System.IO.FileSystem/tests/FileStream/ctor_str_fm.cs @@ -13,8 +13,6 @@ namespace System.IO.Tests return new FileStream(path, mode); } - protected virtual long InitialLength => 0; - protected virtual string GetExpectedParamName(string paramName) => paramName; [Fact] @@ -97,7 +95,7 @@ namespace System.IO.Tests using (FileStream fs = CreateFileStream(fileName, FileMode.Create)) { // Ensure that the file was re-created - Assert.Equal(InitialLength, fs.Length); + Assert.Equal(0, fs.Length); Assert.Equal(0L, fs.Position); Assert.True(fs.CanRead); Assert.True(fs.CanWrite); @@ -148,7 +146,7 @@ namespace System.IO.Tests using (FileStream fs = CreateFileStream(fileName, FileMode.Open)) { // Ensure that the file was re-opened - Assert.Equal(Math.Max(1L, InitialLength), fs.Length); + Assert.Equal(1, fs.Length); Assert.Equal(0L, fs.Position); Assert.True(fs.CanRead); Assert.True(fs.CanWrite); @@ -177,7 +175,7 @@ namespace System.IO.Tests using (FileStream fs = CreateFileStream(fileName, FileMode.OpenOrCreate)) { // Ensure that the file was re-opened - Assert.Equal(Math.Max(1L, InitialLength), fs.Length); + Assert.Equal(1, fs.Length); Assert.Equal(0L, fs.Position); Assert.True(fs.CanRead); Assert.True(fs.CanWrite); @@ -204,7 +202,7 @@ namespace System.IO.Tests using (FileStream fs = CreateFileStream(fileName, FileMode.Truncate)) { // Ensure that the file was re-opened and truncated - Assert.Equal(InitialLength, fs.Length); + Assert.Equal(0, fs.Length); Assert.Equal(0L, fs.Position); Assert.True(fs.CanRead); Assert.True(fs.CanWrite); @@ -246,7 +244,7 @@ namespace System.IO.Tests using (FileStream fs = CreateFileStream(fileName, FileMode.Append)) { // Ensure that the file was re-opened and position set to end - Assert.Equal(Math.Max(1L, InitialLength), fs.Length); + Assert.Equal(1, fs.Length); long position = fs.Position; Assert.Equal(fs.Length, position); diff --git a/src/libraries/System.IO.FileSystem/tests/Net5CompatTests/System.IO.FileSystem.Net5Compat.Tests.csproj b/src/libraries/System.IO.FileSystem/tests/Net5CompatTests/System.IO.FileSystem.Net5Compat.Tests.csproj index 745ff60..438e076 100644 --- a/src/libraries/System.IO.FileSystem/tests/Net5CompatTests/System.IO.FileSystem.Net5Compat.Tests.csproj +++ b/src/libraries/System.IO.FileSystem/tests/Net5CompatTests/System.IO.FileSystem.Net5Compat.Tests.csproj @@ -1,4 +1,4 @@ - + true true @@ -14,7 +14,6 @@ - diff --git a/src/libraries/System.IO.FileSystem/tests/System.IO.FileSystem.Tests.csproj b/src/libraries/System.IO.FileSystem/tests/System.IO.FileSystem.Tests.csproj index bb7cac2..f8a57ec 100644 --- a/src/libraries/System.IO.FileSystem/tests/System.IO.FileSystem.Tests.csproj +++ b/src/libraries/System.IO.FileSystem/tests/System.IO.FileSystem.Tests.csproj @@ -24,7 +24,7 @@ - + @@ -70,15 +70,14 @@ - - + - + @@ -103,7 +102,7 @@ - + diff --git a/src/libraries/System.Private.CoreLib/src/Microsoft/Win32/SafeHandles/SafeFileHandle.Unix.cs b/src/libraries/System.Private.CoreLib/src/Microsoft/Win32/SafeHandles/SafeFileHandle.Unix.cs index ee1def8..1edb65f 100644 --- a/src/libraries/System.Private.CoreLib/src/Microsoft/Win32/SafeHandles/SafeFileHandle.Unix.cs +++ b/src/libraries/System.Private.CoreLib/src/Microsoft/Win32/SafeHandles/SafeFileHandle.Unix.cs @@ -323,20 +323,24 @@ namespace Microsoft.Win32.SafeHandles } } - // If preallocationSize has been provided for a creatable and writeable file - if (FileStreamHelpers.ShouldPreallocate(preallocationSize, access, mode)) + if (preallocationSize > 0 && Interop.Sys.FAllocate(this, 0, preallocationSize) < 0) { - int fallocateResult = Interop.Sys.PosixFAllocate(this, 0, preallocationSize); - if (fallocateResult != 0) + Interop.ErrorInfo errorInfo = Interop.Sys.GetLastErrorInfo(); + + // Only throw for errors that indicate there is not enough space. + if (errorInfo.Error == Interop.Error.EFBIG || + errorInfo.Error == Interop.Error.ENOSPC) { Dispose(); - Interop.Sys.Unlink(path!); // remove the file to mimic Windows behaviour (atomic operation) - Debug.Assert(fallocateResult == -1 || fallocateResult == -2); - throw new IOException(SR.Format( - fallocateResult == -1 ? SR.IO_DiskFull_Path_AllocationSize : SR.IO_FileTooLarge_Path_AllocationSize, - path, - preallocationSize)); + // Delete the file we've created. + Debug.Assert(mode == FileMode.Create || mode == FileMode.CreateNew); + Interop.Sys.Unlink(path!); + + throw new IOException(SR.Format(errorInfo.Error == Interop.Error.EFBIG + ? SR.IO_FileTooLarge_Path_AllocationSize + : SR.IO_DiskFull_Path_AllocationSize, + path, preallocationSize)); } } } diff --git a/src/libraries/System.Private.CoreLib/src/Microsoft/Win32/SafeHandles/SafeFileHandle.Windows.cs b/src/libraries/System.Private.CoreLib/src/Microsoft/Win32/SafeHandles/SafeFileHandle.Windows.cs index a7c4751..3352f01 100644 --- a/src/libraries/System.Private.CoreLib/src/Microsoft/Win32/SafeHandles/SafeFileHandle.Windows.cs +++ b/src/libraries/System.Private.CoreLib/src/Microsoft/Win32/SafeHandles/SafeFileHandle.Windows.cs @@ -34,7 +34,7 @@ namespace Microsoft.Win32.SafeHandles // of converting DOS to NT file paths (RtlDosPathNameToRelativeNtPathName_U_WithStatus is not documented) SafeFileHandle fileHandle = CreateFile(fullPath, mode, access, share, options); - if (FileStreamHelpers.ShouldPreallocate(preallocationSize, access, mode)) + if (preallocationSize > 0) { Preallocate(fullPath, preallocationSize, fileHandle); } @@ -121,19 +121,19 @@ namespace Microsoft.Win32.SafeHandles { int errorCode = Marshal.GetLastPInvokeError(); - // we try to mimic the atomic NtCreateFile here: - // if preallocation fails, close the handle and delete the file - fileHandle.Dispose(); - Interop.Kernel32.DeleteFile(fullPath); - - switch (errorCode) + // Only throw for errors that indicate there is not enough space. + if (errorCode == Interop.Errors.ERROR_DISK_FULL || + errorCode == Interop.Errors.ERROR_FILE_TOO_LARGE) { - case Interop.Errors.ERROR_DISK_FULL: - throw new IOException(SR.Format(SR.IO_DiskFull_Path_AllocationSize, fullPath, preallocationSize)); - case Interop.Errors.ERROR_FILE_TOO_LARGE: - throw new IOException(SR.Format(SR.IO_FileTooLarge_Path_AllocationSize, fullPath, preallocationSize)); - default: - throw Win32Marshal.GetExceptionForWin32Error(errorCode, fullPath); + fileHandle.Dispose(); + + // Delete the file we've created. + Interop.Kernel32.DeleteFile(fullPath); + + throw new IOException(SR.Format(errorCode == Interop.Errors.ERROR_DISK_FULL + ? SR.IO_DiskFull_Path_AllocationSize + : SR.IO_FileTooLarge_Path_AllocationSize, + fullPath, preallocationSize)); } } } diff --git a/src/libraries/System.Private.CoreLib/src/Resources/Strings.resx b/src/libraries/System.Private.CoreLib/src/Resources/Strings.resx index 1e16e6b..c651da6 100644 --- a/src/libraries/System.Private.CoreLib/src/Resources/Strings.resx +++ b/src/libraries/System.Private.CoreLib/src/Resources/Strings.resx @@ -1092,6 +1092,12 @@ Append access can be requested only in write-only mode. + + Preallocation size can be requested only in write mode. + + + Preallocation size can be requested only for new files. + Type of argument is not compatible with the generic comparer. diff --git a/src/libraries/System.Private.CoreLib/src/System.Private.CoreLib.Shared.projitems b/src/libraries/System.Private.CoreLib/src/System.Private.CoreLib.Shared.projitems index 751a074..df99a3f 100644 --- a/src/libraries/System.Private.CoreLib/src/System.Private.CoreLib.Shared.projitems +++ b/src/libraries/System.Private.CoreLib/src/System.Private.CoreLib.Shared.projitems @@ -2029,8 +2029,8 @@ Common\Interop\Unix\System.Native\Interop.PosixFAdvise.cs - - Common\Interop\Unix\System.Native\Interop.PosixFAllocate.cs + + Common\Interop\Unix\System.Native\Interop.FAllocate.cs Common\Interop\Unix\System.Native\Interop.PRead.cs diff --git a/src/libraries/System.Private.CoreLib/src/System/IO/FileStream.cs b/src/libraries/System.Private.CoreLib/src/System/IO/FileStream.cs index b60bb9d..b97194b 100644 --- a/src/libraries/System.Private.CoreLib/src/System/IO/FileStream.cs +++ b/src/libraries/System.Private.CoreLib/src/System/IO/FileStream.cs @@ -197,6 +197,11 @@ namespace System.IO } } + if (options.PreallocationSize > 0) + { + FileStreamHelpers.ValidateArgumentsForPreallocation(options.Mode, options.Access); + } + FileStreamHelpers.SerializationGuard(options.Access); _strategy = FileStreamHelpers.ChooseStrategy( diff --git a/src/libraries/System.Private.CoreLib/src/System/IO/Strategies/FileStreamHelpers.Windows.cs b/src/libraries/System.Private.CoreLib/src/System/IO/Strategies/FileStreamHelpers.Windows.cs index 779acce..5ba493c 100644 --- a/src/libraries/System.Private.CoreLib/src/System/IO/Strategies/FileStreamHelpers.Windows.cs +++ b/src/libraries/System.Private.CoreLib/src/System/IO/Strategies/FileStreamHelpers.Windows.cs @@ -124,6 +124,16 @@ namespace System.IO.Strategies internal static unsafe void SetFileLength(SafeFileHandle handle, long length) { + if (!TrySetFileLength(handle, length, out int errorCode)) + { + throw errorCode == Interop.Errors.ERROR_INVALID_PARAMETER + ? new ArgumentOutOfRangeException(nameof(length), SR.ArgumentOutOfRange_FileLengthTooBig) + : Win32Marshal.GetExceptionForWin32Error(errorCode, handle.Path); + } + } + + internal static unsafe bool TrySetFileLength(SafeFileHandle handle, long length, out int errorCode) + { var eofInfo = new Interop.Kernel32.FILE_END_OF_FILE_INFO { EndOfFile = length @@ -135,11 +145,12 @@ namespace System.IO.Strategies &eofInfo, (uint)sizeof(Interop.Kernel32.FILE_END_OF_FILE_INFO))) { - int errorCode = Marshal.GetLastPInvokeError(); - if (errorCode == Interop.Errors.ERROR_INVALID_PARAMETER) - throw new ArgumentOutOfRangeException(nameof(length), SR.ArgumentOutOfRange_FileLengthTooBig); - throw Win32Marshal.GetExceptionForWin32Error(errorCode, handle.Path); + errorCode = Marshal.GetLastPInvokeError(); + return false; } + + errorCode = Interop.Errors.ERROR_SUCCESS; + return true; } internal static unsafe int ReadFileNative(SafeFileHandle handle, Span bytes, NativeOverlapped* overlapped, out int errorCode) diff --git a/src/libraries/System.Private.CoreLib/src/System/IO/Strategies/FileStreamHelpers.cs b/src/libraries/System.Private.CoreLib/src/System/IO/Strategies/FileStreamHelpers.cs index d86c70a..3db2174 100644 --- a/src/libraries/System.Private.CoreLib/src/System/IO/Strategies/FileStreamHelpers.cs +++ b/src/libraries/System.Private.CoreLib/src/System/IO/Strategies/FileStreamHelpers.cs @@ -58,11 +58,6 @@ namespace System.IO.Strategies e is NotSupportedException || (e is ArgumentException && !(e is ArgumentNullException)); - internal static bool ShouldPreallocate(long preallocationSize, FileAccess access, FileMode mode) - => preallocationSize > 0 - && (access & FileAccess.Write) != 0 - && mode != FileMode.Open && mode != FileMode.Append; - internal static void ValidateArguments(string path, FileMode mode, FileAccess access, FileShare share, int bufferSize, FileOptions options, long preallocationSize) { if (path == null) @@ -125,9 +120,31 @@ namespace System.IO.Strategies throw new ArgumentException(SR.Argument_InvalidAppendMode, nameof(access)); } + if (preallocationSize > 0) + { + ValidateArgumentsForPreallocation(mode, access); + } + SerializationGuard(access); } + internal static void ValidateArgumentsForPreallocation(FileMode mode, FileAccess access) + { + // The user will be writing into the preallocated space. + if ((access & FileAccess.Write) == 0) + { + throw new ArgumentException(SR.Argument_InvalidPreallocateAccess, nameof(access)); + } + + // Only allow preallocation for newly created/overwritten files. + // When we fail to preallocate, we'll remove the file. + if (mode != FileMode.Create && + mode != FileMode.CreateNew) + { + throw new ArgumentException(SR.Argument_InvalidPreallocateMode, nameof(mode)); + } + } + internal static void SerializationGuard(FileAccess access) { if ((access & FileAccess.Write) == FileAccess.Write)