From ee72add040d203effe92cbf0b0889547ac2a4434 Mon Sep 17 00:00:00 2001 From: Adam Sitnik Date: Tue, 14 Sep 2021 13:30:39 +0200 Subject: [PATCH 1/5] Align preallocationSize behavior (#58726) Co-authored-by: Stephen Toub --- .../Kernel32/Interop.FILE_ALLOCATION_INFO.cs | 16 --- .../FileStream/ctor_options_as.Browser.cs | 16 --- .../tests/FileStream/ctor_options_as.Unix.cs | 21 ---- .../FileStream/ctor_options_as.Windows.cs | 38 ++++--- .../tests/FileStream/ctor_options_as.cs | 103 ++++++++++-------- ...stem.IO.FileSystem.Net5Compat.Tests.csproj | 5 +- .../tests/System.IO.FileSystem.Tests.csproj | 5 - .../Win32/SafeHandles/SafeFileHandle.Unix.cs | 2 +- .../SafeHandles/SafeFileHandle.Windows.cs | 35 ++---- .../System.Private.CoreLib.Shared.projitems | 3 - .../Strategies/FileStreamHelpers.Windows.cs | 19 +++- .../System/IO/Strategies/FileStreamHelpers.cs | 5 +- 12 files changed, 108 insertions(+), 160 deletions(-) delete mode 100644 src/libraries/Common/src/Interop/Windows/Kernel32/Interop.FILE_ALLOCATION_INFO.cs delete mode 100644 src/libraries/System.IO.FileSystem/tests/FileStream/ctor_options_as.Browser.cs delete mode 100644 src/libraries/System.IO.FileSystem/tests/FileStream/ctor_options_as.Unix.cs diff --git a/src/libraries/Common/src/Interop/Windows/Kernel32/Interop.FILE_ALLOCATION_INFO.cs b/src/libraries/Common/src/Interop/Windows/Kernel32/Interop.FILE_ALLOCATION_INFO.cs deleted file mode 100644 index 0233626ee73c6..0000000000000 --- a/src/libraries/Common/src/Interop/Windows/Kernel32/Interop.FILE_ALLOCATION_INFO.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. - -internal static partial class Interop -{ - internal static partial class Kernel32 - { - // Value taken from https://docs.microsoft.com/en-us/windows/win32/api/fileapi/nf-fileapi-setfileinformationbyhandle#remarks: - internal const int FileAllocationInfo = 5; - - internal struct FILE_ALLOCATION_INFO - { - internal long AllocationSize; - } - } -} 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 3475c8999ca6f..0000000000000 --- 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 12e8f1641bac0..0000000000000 --- 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.Windows.cs b/src/libraries/System.IO.FileSystem/tests/FileStream/ctor_options_as.Windows.cs index bde8cd5e4692a..b716bcb3de25b 100644 --- a/src/libraries/System.IO.FileSystem/tests/FileStream/ctor_options_as.Windows.cs +++ b/src/libraries/System.IO.FileSystem/tests/FileStream/ctor_options_as.Windows.cs @@ -1,29 +1,16 @@ // 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_as { - 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) - { - Interop.Kernel32.FILE_STANDARD_INFO info; - - Assert.True(Interop.Kernel32.GetFileInformationByHandleEx(fileStream.SafeFileHandle, Interop.Kernel32.FileStandardInfo, &info, (uint)sizeof(Interop.Kernel32.FILE_STANDARD_INFO))); - - return info.AllocationSize; - } - [Theory] [InlineData(@"\\?\")] [InlineData(@"\??\")] @@ -36,7 +23,24 @@ public void ExtendedPathsAreSupported(string prefix) using (var fs = new FileStream(filePath, GetOptions(FileMode.CreateNew, FileAccess.Write, FileShare.None, FileOptions.None, preallocationSize))) { - Assert.True(GetActualPreallocationSize(fs) >= preallocationSize, $"Provided {preallocationSize}, actual: {GetActualPreallocationSize(fs)}"); + Assert.Equal(preallocationSize, fs.Length); + } + } + + [Fact] + public async Task PreallocationSizeIsIgnoredForNonSeekableFiles() + { + string pipeName = GetNamedPipeServerStreamName(); + string pipePath = Path.GetFullPath($@"\\.\pipe\{pipeName}"); + + FileStreamOptions options = new() { Mode = FileMode.Open, Access = FileAccess.Write, Share = FileShare.None, PreallocationSize = 123 }; + + using (var server = new NamedPipeServerStream(pipeName, PipeDirection.In)) + using (var clienStream = new FileStream(pipePath, options)) + { + await server.WaitForConnectionAsync(); + + Assert.False(clienStream.CanSeek); } } 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 index f36e55ea16d2d..b23ccb1573983 100644 --- a/src/libraries/System.IO.FileSystem/tests/FileStream/ctor_options_as.cs +++ b/src/libraries/System.IO.FileSystem/tests/FileStream/ctor_options_as.cs @@ -1,6 +1,8 @@ // 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 @@ -68,6 +70,10 @@ public partial class NoParallelTests { } [Collection("NoParallelTests")] public partial class FileStream_ctor_options_as : FileStream_ctor_options_as_base { + protected override long PreallocationSize => 10; + + protected override long InitialLength => 10; + [Fact] public virtual void NegativePreallocationSizeThrows() { @@ -80,40 +86,51 @@ public virtual void NegativePreallocationSizeThrows() [InlineData(FileMode.Create, 0L)] [InlineData(FileMode.CreateNew, 0L)] [InlineData(FileMode.OpenOrCreate, 0L)] - public void WhenFileIsCreatedWithoutPreallocationSizeSpecifiedThePreallocationSizeIsNotSet(FileMode mode, long preallocationSize) + public void WhenFileIsCreatedWithoutPreallocationSizeSpecifiedItsLengthIsZero(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) + [InlineData(FileMode.Open, 20L)] + [InlineData(FileMode.Open, 5L)] + [InlineData(FileMode.Append, 20L)] + [InlineData(FileMode.Append, 5L)] + public void PreallocationSizeIsIgnoredForFileModeOpenAndAppend(FileMode mode, long preallocationSize) { - const int initialSize = 1; + const int initialSize = 10; 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 + using (var fs = new FileStream(filePath, GetOptions(mode, FileAccess.Write, FileShare.None, FileOptions.None, preallocationSize))) { - initialPreallocationSize = GetActualPreallocationSize(fs); // just read it to ensure it's not being changed + Assert.Equal(initialSize, fs.Length); // it has NOT been changed + Assert.Equal(mode == FileMode.Append ? initialSize : 0, fs.Position); } + } - using (var fs = new FileStream(filePath, GetOptions(mode, FileAccess.Write, FileShare.None, FileOptions.None, preallocationSize))) + [Theory] + [InlineData(FileMode.OpenOrCreate, 20L)] // preallocationSize > initialSize + [InlineData(FileMode.OpenOrCreate, 5L)] // preallocationSize < initialSize + public void WhenExistingFileIsBeingOpenedWithOpenOrCreateModeTheLengthRemainsUnchanged(FileMode mode, long preallocationSize) + { + const int initialSize = 10; + string filePath = GetPathToNonExistingFile(); + byte[] initialData = RandomNumberGenerator.GetBytes(initialSize); + File.WriteAllBytes(filePath, initialData); + + using (var fs = new FileStream(filePath, GetOptions(mode, FileAccess.ReadWrite, 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); + Assert.Equal(initialSize, fs.Length); // it was not changed + Assert.Equal(0, fs.Position); + + byte[] actualContent = new byte[initialData.Length]; + Assert.Equal(actualContent.Length, fs.Read(actualContent)); + AssertExtensions.SequenceEqual(initialData, actualContent); // the initial content was not changed } } @@ -121,16 +138,16 @@ public void WhenExistingFileIsBeingOpenedWithPreallocationSizeSpecifiedThePreall [InlineData(FileMode.Create)] [InlineData(FileMode.CreateNew)] [InlineData(FileMode.OpenOrCreate)] - public void WhenFileIsCreatedWithPreallocationSizeSpecifiedThePreallocationSizeIsSet(FileMode mode) + public void WhenFileIsCreatedWithPreallocationSizeSpecifiedTheLengthIsSetAndTheContentIsZeroed(FileMode mode) { const long preallocationSize = 123; - using (var fs = new FileStream(GetPathToNonExistingFile(), GetOptions(mode, FileAccess.Write, FileShare.None, FileOptions.None, preallocationSize))) + using (var fs = new FileStream(GetPathToNonExistingFile(), GetOptions(mode, FileAccess.ReadWrite, 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(preallocationSize, fs.Length); Assert.Equal(0, fs.Position); + + AssertFileContentHasBeenZeroed(0, (int)fs.Length, fs); } } @@ -153,7 +170,7 @@ public void WhenDiskIsFullTheErrorMessageContainsAllDetails(FileMode mode) Assert.Contains(filePath, ex.Message); Assert.Contains(tooMuch.ToString(), ex.Message); - // ensure it was NOT created (provided OOTB by Windows, emulated on Unix) + // ensure it was NOT created bool exists = File.Exists(filePath); if (exists) { @@ -163,37 +180,20 @@ public void WhenDiskIsFullTheErrorMessageContainsAllDetails(FileMode mode) } [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() + public void WhenFileIsTruncatedWithPreallocationSizeSpecifiedTheLengthIsSetAndTheContentIsZeroed() { 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]); + File.WriteAllBytes(filePath, Enumerable.Repeat((byte)1, initialSize).ToArray()); - using (var fs = new FileStream(filePath, GetOptions(FileMode.Truncate, FileAccess.Write, FileShare.None, FileOptions.None, preallocationSize))) + using (var fs = new FileStream(filePath, GetOptions(FileMode.Truncate, FileAccess.ReadWrite, 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(preallocationSize, fs.Length); Assert.Equal(0, fs.Position); + + AssertFileContentHasBeenZeroed(0, (int)fs.Length, fs); } } @@ -208,5 +208,16 @@ private string GetPathToNonExistingFile() return filePath; } + + private static void AssertFileContentHasBeenZeroed(int from, int to, FileStream fs) + { + int expectedByteCount = to - from; + int extraByteCount = 1; + byte[] content = Enumerable.Repeat((byte)1, expectedByteCount + extraByteCount).ToArray(); + fs.Position = from; + Assert.Equal(expectedByteCount, fs.Read(content)); + Assert.All(content.SkipLast(extraByteCount), @byte => Assert.Equal(0, @byte)); + Assert.Equal(to, fs.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 745ff60368b2e..0343ddaf70edc 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 @@ - @@ -24,8 +23,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 bb7cac297d73a..e31a2fda63722 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 @@ -70,9 +70,7 @@ - - @@ -88,8 +86,6 @@ - - @@ -103,7 +99,6 @@ - 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 ee1def85b9e02..777997444848f 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 @@ -324,7 +324,7 @@ private void Init(string path, FileMode mode, FileAccess access, FileShare share } // If preallocationSize has been provided for a creatable and writeable file - if (FileStreamHelpers.ShouldPreallocate(preallocationSize, access, mode)) + if (FileStreamHelpers.ShouldPreallocate(preallocationSize, access, mode, this)) { int fallocateResult = Interop.Sys.PosixFAllocate(this, 0, preallocationSize); if (fallocateResult != 0) 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 a7c4751a09478..3dde1e2e89a6b 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 @@ internal static unsafe SafeFileHandle Open(string fullPath, FileMode mode, FileA // 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 (FileStreamHelpers.ShouldPreallocate(preallocationSize, access, mode, fileHandle)) { Preallocate(fullPath, preallocationSize, fileHandle); } @@ -108,33 +108,18 @@ private static unsafe SafeFileHandle CreateFile(string fullPath, FileMode mode, private static unsafe void Preallocate(string fullPath, long preallocationSize, SafeFileHandle fileHandle) { - var allocationInfo = new Interop.Kernel32.FILE_ALLOCATION_INFO - { - AllocationSize = preallocationSize - }; - - if (!Interop.Kernel32.SetFileInformationByHandle( - fileHandle, - Interop.Kernel32.FileAllocationInfo, - &allocationInfo, - (uint)sizeof(Interop.Kernel32.FILE_ALLOCATION_INFO))) - { - int errorCode = Marshal.GetLastPInvokeError(); - - // we try to mimic the atomic NtCreateFile here: - // if preallocation fails, close the handle and delete the file + // preallocationSize must be ignored for non-seekable files, unsupported file systems + // and other failures other than ERROR_DISK_FULL and ERROR_FILE_TOO_LARGE + if (!FileStreamHelpers.TrySetFileLength(fileHandle, preallocationSize, out int errorCode) + && errorCode == Interop.Errors.ERROR_DISK_FULL || errorCode == Interop.Errors.ERROR_FILE_TOO_LARGE) + { + // Windows does not modify the file size if the request can't be satisfied in atomic way. + // posix_fallocate (Unix) implementation might consume all available disk space and fail after that. + // To ensure that the behaviour is the same for every OS (no incomplete or empty file), we close the handle and delete the file. fileHandle.Dispose(); Interop.Kernel32.DeleteFile(fullPath); - switch (errorCode) - { - 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); - } + 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/System.Private.CoreLib.Shared.projitems b/src/libraries/System.Private.CoreLib/src/System.Private.CoreLib.Shared.projitems index 751a0748a35ce..6430f2d1db1db 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 @@ -1453,9 +1453,6 @@ Common\Interop\Windows\Kernel32\Interop.FILE_BASIC_INFO.cs - - Common\Interop\Windows\Kernel32\Interop.FILE_ALLOCATION_INFO.cs - Common\Interop\Windows\Kernel32\Interop.FILE_END_OF_FILE_INFO.cs 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 779acce937371..5ba493ca9ca21 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 @@ -123,6 +123,16 @@ internal static void Unlock(SafeFileHandle handle, long position, long length) } 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 { @@ -135,11 +145,12 @@ internal static unsafe void SetFileLength(SafeFileHandle handle, long length) &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 d86c70a621ca1..981776da32b81 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,10 +58,11 @@ e is UnauthorizedAccessException || e is NotSupportedException || (e is ArgumentException && !(e is ArgumentNullException)); - internal static bool ShouldPreallocate(long preallocationSize, FileAccess access, FileMode mode) + internal static bool ShouldPreallocate(long preallocationSize, FileAccess access, FileMode mode, SafeFileHandle fileHandle) => preallocationSize > 0 && (access & FileAccess.Write) != 0 - && mode != FileMode.Open && mode != FileMode.Append; + && mode != FileMode.Open && mode != FileMode.Append + && (mode != FileMode.OpenOrCreate || (fileHandle.CanSeek && RandomAccess.GetFileLength(fileHandle) == 0)); // allow to extend only new files internal static void ValidateArguments(string path, FileMode mode, FileAccess access, FileShare share, int bufferSize, FileOptions options, long preallocationSize) { From bf02a2d8547d6619f715b230632cd866e98c7bb7 Mon Sep 17 00:00:00 2001 From: Tom Deseyn Date: Thu, 23 Sep 2021 18:25:49 +0200 Subject: [PATCH 2/5] File preallocationSize: align Windows and Unix behavior. (#59338) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit * 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ú --- ...PosixFAllocate.cs => Interop.FAllocate.cs} | 6 +- .../Kernel32/Interop.FILE_ALLOCATION_INFO.cs | 16 ++ .../Native/Unix/Common/pal_config.h.in | 3 +- .../Native/Unix/System.Native/entrypoints.c | 2 +- .../Native/Unix/System.Native/pal_io.c | 74 +----- .../Native/Unix/System.Native/pal_io.h | 6 +- src/libraries/Native/Unix/configure.cmake | 9 +- .../src/Resources/Strings.resx | 6 + .../System.IO.FileSystem/tests/File/Open.cs | 22 +- .../tests/File/OpenHandle.cs | 16 +- .../tests/FileInfo/Open.cs | 22 +- .../tests/FileStream/ctor_options.Browser.cs | 17 ++ .../tests/FileStream/ctor_options.Unix.cs | 36 +++ ..._as.Windows.cs => ctor_options.Windows.cs} | 44 ++-- .../tests/FileStream/ctor_options.cs | 172 ++++++++++++++ .../tests/FileStream/ctor_options_as.cs | 223 ------------------ .../tests/FileStream/ctor_str_fm.cs | 12 +- ...stem.IO.FileSystem.Net5Compat.Tests.csproj | 2 + .../tests/System.IO.FileSystem.Tests.csproj | 8 +- .../Win32/SafeHandles/SafeFileHandle.Unix.cs | 24 +- .../SafeHandles/SafeFileHandle.Windows.cs | 41 +++- .../src/Resources/Strings.resx | 6 + .../System.Private.CoreLib.Shared.projitems | 7 +- .../src/System/IO/FileStream.cs | 5 + .../System/IO/Strategies/FileStreamHelpers.cs | 28 ++- 25 files changed, 420 insertions(+), 387 deletions(-) rename src/libraries/Common/src/Interop/Unix/System.Native/{Interop.PosixFAllocate.cs => Interop.FAllocate.cs} (63%) create mode 100644 src/libraries/Common/src/Interop/Windows/Kernel32/Interop.FILE_ALLOCATION_INFO.cs create mode 100644 src/libraries/System.IO.FileSystem/tests/FileStream/ctor_options.Browser.cs create mode 100644 src/libraries/System.IO.FileSystem/tests/FileStream/ctor_options.Unix.cs rename src/libraries/System.IO.FileSystem/tests/FileStream/{ctor_options_as.Windows.cs => ctor_options.Windows.cs} (66%) create mode 100644 src/libraries/System.IO.FileSystem/tests/FileStream/ctor_options.cs delete mode 100644 src/libraries/System.IO.FileSystem/tests/FileStream/ctor_options_as.cs 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 2866ed38935bd..71155e0f35e67 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/Common/src/Interop/Windows/Kernel32/Interop.FILE_ALLOCATION_INFO.cs b/src/libraries/Common/src/Interop/Windows/Kernel32/Interop.FILE_ALLOCATION_INFO.cs new file mode 100644 index 0000000000000..0233626ee73c6 --- /dev/null +++ b/src/libraries/Common/src/Interop/Windows/Kernel32/Interop.FILE_ALLOCATION_INFO.cs @@ -0,0 +1,16 @@ +// Licensed to the .NET Foundation under one or more agreements. +// The .NET Foundation licenses this file to you under the MIT license. + +internal static partial class Interop +{ + internal static partial class Kernel32 + { + // Value taken from https://docs.microsoft.com/en-us/windows/win32/api/fileapi/nf-fileapi-setfileinformationbyhandle#remarks: + internal const int FileAllocationInfo = 5; + + internal struct FILE_ALLOCATION_INFO + { + internal long AllocationSize; + } + } +} diff --git a/src/libraries/Native/Unix/Common/pal_config.h.in b/src/libraries/Native/Unix/Common/pal_config.h.in index d50c8d6f95297..9cffccbb0cc71 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 721be5eee63de..702ead88e6d7e 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 d86e8feb93584..9d313dd6e9951 100644 --- a/src/libraries/Native/Unix/System.Native/pal_io.c +++ b/src/libraries/Native/Unix/System.Native/pal_io.c @@ -1008,83 +1008,33 @@ 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 + while ((result = fallocate(fileDescriptor, FALLOC_FL_KEEP_SIZE, (off_t)offset, (off_t)length)) == EINTR); #elif defined(F_PREALLOCATE) // macOS 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)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 3f0db054d4368..0e5d4cae2feb8 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 fbfd5a9bae045..e1afadb0e5249 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/src/Resources/Strings.resx b/src/libraries/System.IO.FileSystem/src/Resources/Strings.resx index 3c391ca6bcde4..583d1c9cc366a 100644 --- a/src/libraries/System.IO.FileSystem/src/Resources/Strings.resx +++ b/src/libraries/System.IO.FileSystem/src/Resources/Strings.resx @@ -173,6 +173,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. + Combining FileMode: {0} with FileAccess: {1} is invalid. diff --git a/src/libraries/System.IO.FileSystem/tests/File/Open.cs b/src/libraries/System.IO.FileSystem/tests/File/Open.cs index d014cf0efb94d..bd9b38b121b88 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 @@ 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 index a8b3619fb51cf..120ed9aec222e 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 @@ 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 2324a92eafce4..36fd39a600056 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 @@ 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 new file mode 100644 index 0000000000000..45326d885c455 --- /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 0000000000000..c33c56a1fa570 --- /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 66% 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 b716bcb3de25b..2dca6baf22bd0 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 @@ -9,8 +9,21 @@ namespace System.IO.Tests { - public partial class FileStream_ctor_options_as + public partial class FileStream_ctor_options { + private unsafe long GetAllocatedSize(FileStream fileStream) + { + Interop.Kernel32.FILE_STANDARD_INFO info; + + Assert.True(Interop.Kernel32.GetFileInformationByHandleEx(fileStream.SafeFileHandle, Interop.Kernel32.FileStandardInfo, &info, (uint)sizeof(Interop.Kernel32.FILE_STANDARD_INFO))); + + return info.AllocationSize; + } + + private static bool SupportsPreallocation => true; + + private static bool IsGetAllocatedSizeImplemented => true; + [Theory] [InlineData(@"\\?\")] [InlineData(@"\??\")] @@ -19,43 +32,26 @@ public void ExtendedPathsAreSupported(string prefix) { 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.Equal(preallocationSize, fs.Length); - } - } - - [Fact] - public async Task PreallocationSizeIsIgnoredForNonSeekableFiles() - { - string pipeName = GetNamedPipeServerStreamName(); - string pipePath = Path.GetFullPath($@"\\.\pipe\{pipeName}"); - - FileStreamOptions options = new() { Mode = FileMode.Open, Access = FileAccess.Write, Share = FileShare.None, PreallocationSize = 123 }; - - using (var server = new NamedPipeServerStream(pipeName, PipeDirection.In)) - using (var clienStream = new FileStream(pipePath, options)) - { - await server.WaitForConnectionAsync(); - - Assert.False(clienStream.CanSeek); + 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 0000000000000..c4344de517449 --- /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 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 : 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.cs b/src/libraries/System.IO.FileSystem/tests/FileStream/ctor_options_as.cs deleted file mode 100644 index b23ccb1573983..0000000000000 --- a/src/libraries/System.IO.FileSystem/tests/FileStream/ctor_options_as.cs +++ /dev/null @@ -1,223 +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 System.Linq; -using System.Security.Cryptography; -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 - { - protected override long PreallocationSize => 10; - - protected override long InitialLength => 10; - - [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 WhenFileIsCreatedWithoutPreallocationSizeSpecifiedItsLengthIsZero(FileMode mode, long preallocationSize) - { - using (var fs = new FileStream(GetPathToNonExistingFile(), GetOptions(mode, FileAccess.Write, FileShare.None, FileOptions.None, preallocationSize))) - { - Assert.Equal(0, fs.Length); - Assert.Equal(0, fs.Position); - } - } - - [Theory] - [InlineData(FileMode.Open, 20L)] - [InlineData(FileMode.Open, 5L)] - [InlineData(FileMode.Append, 20L)] - [InlineData(FileMode.Append, 5L)] - public void PreallocationSizeIsIgnoredForFileModeOpenAndAppend(FileMode mode, long preallocationSize) - { - const int initialSize = 10; - string filePath = GetPathToNonExistingFile(); - File.WriteAllBytes(filePath, new byte[initialSize]); - - using (var fs = new FileStream(filePath, GetOptions(mode, FileAccess.Write, FileShare.None, FileOptions.None, preallocationSize))) - { - Assert.Equal(initialSize, fs.Length); // it has NOT been changed - Assert.Equal(mode == FileMode.Append ? initialSize : 0, fs.Position); - } - } - - [Theory] - [InlineData(FileMode.OpenOrCreate, 20L)] // preallocationSize > initialSize - [InlineData(FileMode.OpenOrCreate, 5L)] // preallocationSize < initialSize - public void WhenExistingFileIsBeingOpenedWithOpenOrCreateModeTheLengthRemainsUnchanged(FileMode mode, long preallocationSize) - { - const int initialSize = 10; - string filePath = GetPathToNonExistingFile(); - byte[] initialData = RandomNumberGenerator.GetBytes(initialSize); - File.WriteAllBytes(filePath, initialData); - - using (var fs = new FileStream(filePath, GetOptions(mode, FileAccess.ReadWrite, FileShare.None, FileOptions.None, preallocationSize))) - { - Assert.Equal(initialSize, fs.Length); // it was not changed - Assert.Equal(0, fs.Position); - - byte[] actualContent = new byte[initialData.Length]; - Assert.Equal(actualContent.Length, fs.Read(actualContent)); - AssertExtensions.SequenceEqual(initialData, actualContent); // the initial content was not changed - } - } - - [Theory] - [InlineData(FileMode.Create)] - [InlineData(FileMode.CreateNew)] - [InlineData(FileMode.OpenOrCreate)] - public void WhenFileIsCreatedWithPreallocationSizeSpecifiedTheLengthIsSetAndTheContentIsZeroed(FileMode mode) - { - const long preallocationSize = 123; - - using (var fs = new FileStream(GetPathToNonExistingFile(), GetOptions(mode, FileAccess.ReadWrite, FileShare.None, FileOptions.None, preallocationSize))) - { - Assert.Equal(preallocationSize, fs.Length); - Assert.Equal(0, fs.Position); - - AssertFileContentHasBeenZeroed(0, (int)fs.Length, fs); - } - } - - [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 - bool exists = File.Exists(filePath); - if (exists) - { - File.Delete(filePath); - } - Assert.False(exists); - } - - [Fact] - public void WhenFileIsTruncatedWithPreallocationSizeSpecifiedTheLengthIsSetAndTheContentIsZeroed() - { - 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, Enumerable.Repeat((byte)1, initialSize).ToArray()); - - using (var fs = new FileStream(filePath, GetOptions(FileMode.Truncate, FileAccess.ReadWrite, FileShare.None, FileOptions.None, preallocationSize))) - { - Assert.Equal(preallocationSize, fs.Length); - Assert.Equal(0, fs.Position); - - AssertFileContentHasBeenZeroed(0, (int)fs.Length, fs); - } - } - - private string GetPathToNonExistingFile() - { - string filePath = GetTestFilePath(); - - if (File.Exists(filePath)) - { - File.Delete(filePath); - } - - return filePath; - } - - private static void AssertFileContentHasBeenZeroed(int from, int to, FileStream fs) - { - int expectedByteCount = to - from; - int extraByteCount = 1; - byte[] content = Enumerable.Repeat((byte)1, expectedByteCount + extraByteCount).ToArray(); - fs.Position = from; - Assert.Equal(expectedByteCount, fs.Read(content)); - Assert.All(content.SkipLast(extraByteCount), @byte => Assert.Equal(0, @byte)); - Assert.Equal(to, fs.Position); - } - } -} 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 47e3b49237316..cd3b18da03422 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 @@ protected virtual FileStream CreateFileStream(string path, FileMode mode) return new FileStream(path, mode); } - protected virtual long InitialLength => 0; - protected virtual string GetExpectedParamName(string paramName) => paramName; [Fact] @@ -97,7 +95,7 @@ public void FileModeCreateExisting(string streamSpecifier) 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 @@ public void FileModeOpenExisting(string streamSpecifier) 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 @@ public void FileModeOpenOrCreateExisting(string streamSpecifier) 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 @@ public void FileModeTruncateExisting(string streamSpecifier) 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 @@ public virtual void FileModeAppendExisting(string streamSpecifier) 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 0343ddaf70edc..438e076c3e778 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 @@ -23,6 +23,8 @@ + + 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 e31a2fda63722..f8a57ec34a7db 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 @@ - + @@ -71,12 +71,13 @@ + - + @@ -86,6 +87,8 @@ + + @@ -99,6 +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 777997444848f..1edb65fe58afb 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 @@ private void Init(string path, FileMode mode, FileAccess access, FileShare share } } - // If preallocationSize has been provided for a creatable and writeable file - if (FileStreamHelpers.ShouldPreallocate(preallocationSize, access, mode, this)) + 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 3dde1e2e89a6b..3352f0194e73b 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 @@ internal static unsafe SafeFileHandle Open(string fullPath, FileMode mode, FileA // 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, fileHandle)) + if (preallocationSize > 0) { Preallocate(fullPath, preallocationSize, fileHandle); } @@ -108,18 +108,33 @@ private static unsafe SafeFileHandle CreateFile(string fullPath, FileMode mode, private static unsafe void Preallocate(string fullPath, long preallocationSize, SafeFileHandle fileHandle) { - // preallocationSize must be ignored for non-seekable files, unsupported file systems - // and other failures other than ERROR_DISK_FULL and ERROR_FILE_TOO_LARGE - if (!FileStreamHelpers.TrySetFileLength(fileHandle, preallocationSize, out int errorCode) - && errorCode == Interop.Errors.ERROR_DISK_FULL || errorCode == Interop.Errors.ERROR_FILE_TOO_LARGE) - { - // Windows does not modify the file size if the request can't be satisfied in atomic way. - // posix_fallocate (Unix) implementation might consume all available disk space and fail after that. - // To ensure that the behaviour is the same for every OS (no incomplete or empty file), we close the handle and delete the file. - fileHandle.Dispose(); - 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)); + var allocationInfo = new Interop.Kernel32.FILE_ALLOCATION_INFO + { + AllocationSize = preallocationSize + }; + + if (!Interop.Kernel32.SetFileInformationByHandle( + fileHandle, + Interop.Kernel32.FileAllocationInfo, + &allocationInfo, + (uint)sizeof(Interop.Kernel32.FILE_ALLOCATION_INFO))) + { + int errorCode = Marshal.GetLastPInvokeError(); + + // 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) + { + 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 1e16e6b577abf..c651da612d53c 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 6430f2d1db1db..df99a3ffcec74 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 @@ -1453,6 +1453,9 @@ Common\Interop\Windows\Kernel32\Interop.FILE_BASIC_INFO.cs + + Common\Interop\Windows\Kernel32\Interop.FILE_ALLOCATION_INFO.cs + Common\Interop\Windows\Kernel32\Interop.FILE_END_OF_FILE_INFO.cs @@ -2026,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 b60bb9d79e3a0..b97194b86c5f0 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 @@ public FileStream(string path, FileStreamOptions options) } } + 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.cs b/src/libraries/System.Private.CoreLib/src/System/IO/Strategies/FileStreamHelpers.cs index 981776da32b81..3db2174d3fbd1 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,12 +58,6 @@ e is UnauthorizedAccessException || e is NotSupportedException || (e is ArgumentException && !(e is ArgumentNullException)); - internal static bool ShouldPreallocate(long preallocationSize, FileAccess access, FileMode mode, SafeFileHandle fileHandle) - => preallocationSize > 0 - && (access & FileAccess.Write) != 0 - && mode != FileMode.Open && mode != FileMode.Append - && (mode != FileMode.OpenOrCreate || (fileHandle.CanSeek && RandomAccess.GetFileLength(fileHandle) == 0)); // allow to extend only new files - internal static void ValidateArguments(string path, FileMode mode, FileAccess access, FileShare share, int bufferSize, FileOptions options, long preallocationSize) { if (path == null) @@ -126,9 +120,31 @@ internal static void ValidateArguments(string path, FileMode mode, FileAccess ac 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) From 8adba3ac0cbcb39b306e2bae13438f2e6787fbdc Mon Sep 17 00:00:00 2001 From: David Cantu Date: Thu, 23 Sep 2021 10:48:47 -0700 Subject: [PATCH 3/5] Fix unused fileDescriptor --- src/libraries/Native/Unix/System.Native/pal_io.c | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/src/libraries/Native/Unix/System.Native/pal_io.c b/src/libraries/Native/Unix/System.Native/pal_io.c index 9d313dd6e9951..2faee754e4e11 100644 --- a/src/libraries/Native/Unix/System.Native/pal_io.c +++ b/src/libraries/Native/Unix/System.Native/pal_io.c @@ -1012,11 +1012,12 @@ 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_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_ALLOCATEALL; // Allocate all requested space or no space at all. fstore.fst_posmode = F_PEOFPOSMODE; // Allocate from the physical end of file. From f85919f2d242e53bff2fffc4e6de54b4461c1e4c Mon Sep 17 00:00:00 2001 From: David Cantu Date: Thu, 23 Sep 2021 11:44:44 -0700 Subject: [PATCH 4/5] void fd when unused --- src/libraries/Native/Unix/System.Native/pal_io.c | 1 + 1 file changed, 1 insertion(+) diff --git a/src/libraries/Native/Unix/System.Native/pal_io.c b/src/libraries/Native/Unix/System.Native/pal_io.c index 2faee754e4e11..049e44e12fcef 100644 --- a/src/libraries/Native/Unix/System.Native/pal_io.c +++ b/src/libraries/Native/Unix/System.Native/pal_io.c @@ -1027,6 +1027,7 @@ int32_t SystemNative_FAllocate(intptr_t fd, int64_t offset, int64_t length) while ((result = fcntl(fileDescriptor, F_PREALLOCATE, &fstore)) == -1 && errno == EINTR); #else + (void)fd; // unused (void)offset; // unused (void)length; // unused result = -1; From 861959efdfa9aed815920c3a0cbc118dfc8b84ef Mon Sep 17 00:00:00 2001 From: David Cantu Date: Fri, 24 Sep 2021 10:54:12 -0700 Subject: [PATCH 5/5] Address feedback --- .../System.IO.FileSystem/src/Resources/Strings.resx | 6 ------ .../System.IO.FileSystem/tests/FileStream/ctor_options.cs | 8 ++++---- 2 files changed, 4 insertions(+), 10 deletions(-) diff --git a/src/libraries/System.IO.FileSystem/src/Resources/Strings.resx b/src/libraries/System.IO.FileSystem/src/Resources/Strings.resx index 583d1c9cc366a..3c391ca6bcde4 100644 --- a/src/libraries/System.IO.FileSystem/src/Resources/Strings.resx +++ b/src/libraries/System.IO.FileSystem/src/Resources/Strings.resx @@ -173,12 +173,6 @@ 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. - Combining FileMode: {0} with FileAccess: {1} is invalid. diff --git a/src/libraries/System.IO.FileSystem/tests/FileStream/ctor_options.cs b/src/libraries/System.IO.FileSystem/tests/FileStream/ctor_options.cs index c4344de517449..5209d5ef76d42 100644 --- a/src/libraries/System.IO.FileSystem/tests/FileStream/ctor_options.cs +++ b/src/libraries/System.IO.FileSystem/tests/FileStream/ctor_options.cs @@ -8,7 +8,7 @@ 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 posix_fallocate works) + // 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 @@ -149,13 +149,13 @@ public void PreallocationSize(FileAccess access, FileMode mode) [InlineData(FileMode.CreateNew)] public void WhenDiskIsFullTheErrorMessageContainsAllDetails(FileMode mode) { - const long tooMuch = 1024L * 1024L * 1024L * 1024L; // 1 TB + 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)); + 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); + Assert.Contains(TooMuch.ToString(), ex.Message); // ensure it was NOT created bool exists = File.Exists(filePath);