From 7d64626dc082aeaea5ee3614577ebcccca38b48f Mon Sep 17 00:00:00 2001 From: Adam Sitnik Date: Fri, 3 Sep 2021 19:47:56 +0200 Subject: [PATCH 1/9] there should be no differences between OSes: * file length should always be set * file content should always be zeroed * OpenOrCreate can't shrink an existing file --- .../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 | 100 ++++++++++-------- ...stem.IO.FileSystem.Net5Compat.Tests.csproj | 5 +- .../tests/System.IO.FileSystem.Tests.csproj | 5 - 6 files changed, 76 insertions(+), 109 deletions(-) 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/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 3475c8999ca6f9..00000000000000 --- 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 12e8f1641bac00..00000000000000 --- 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 bde8cd5e4692a3..b716bcb3de25b5 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 f36e55ea16d2de..48c4846385e74e 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,48 @@ 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)] // this can extend the file + [InlineData(FileMode.OpenOrCreate, 5L)] // this should NOT shink the file + public void WhenFileIsBeingOpenedWithOpenOrCreateModeTheLengthCanBeOnlyExtended(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.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(Math.Max(initialSize, preallocationSize), fs.Length); // it was not shrinked + Assert.Equal(0, fs.Position); + + AssertFileContentHasBeenZeroed(initialSize, (int)fs.Length, fs); } } @@ -121,16 +135,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 +167,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 +177,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 +205,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 = RandomNumberGenerator.GetBytes(expectedByteCount + extraByteCount); + 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 27e1b1c431d678..d9fcde23a91b79 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 439ab465cf9cb1..a9bedb8bd312bc 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 @@ - From 44f1786ba8100f7057898c006e4cec8a8f02c8d2 Mon Sep 17 00:00:00 2001 From: Adam Sitnik Date: Mon, 6 Sep 2021 13:12:17 +0200 Subject: [PATCH 2/9] align Windows implementation to behave like the Unix one --- .../Kernel32/Interop.FILE_ALLOCATION_INFO.cs | 16 ------- .../SafeHandles/SafeFileHandle.Windows.cs | 42 ++++++++++++------- .../System.Private.CoreLib.Shared.projitems | 3 -- 3 files changed, 26 insertions(+), 35 deletions(-) delete mode 100644 src/libraries/Common/src/Interop/Windows/Kernel32/Interop.FILE_ALLOCATION_INFO.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 0233626ee73c62..00000000000000 --- 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.Private.CoreLib/src/Microsoft/Win32/SafeHandles/SafeFileHandle.Windows.cs b/src/libraries/System.Private.CoreLib/src/Microsoft/Win32/SafeHandles/SafeFileHandle.Windows.cs index a7c4751a094786..167f7afdc196a8 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 @@ -36,7 +36,7 @@ internal static unsafe SafeFileHandle Open(string fullPath, FileMode mode, FileA if (FileStreamHelpers.ShouldPreallocate(preallocationSize, access, mode)) { - Preallocate(fullPath, preallocationSize, fileHandle); + Preallocate(fullPath, preallocationSize, fileHandle, mode); } if ((options & FileOptions.Asynchronous) != 0) @@ -106,35 +106,45 @@ private static unsafe SafeFileHandle CreateFile(string fullPath, FileMode mode, return fileHandle; } - 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 + private static unsafe void Preallocate(string fullPath, long preallocationSize, SafeFileHandle fileHandle, FileMode mode) { - var allocationInfo = new Interop.Kernel32.FILE_ALLOCATION_INFO + if (mode == FileMode.OpenOrCreate) { - AllocationSize = preallocationSize + Interop.Kernel32.FILE_STANDARD_INFO info; + if (!Interop.Kernel32.GetFileInformationByHandleEx(fileHandle, Interop.Kernel32.FileStandardInfo, &info, (uint)sizeof(Interop.Kernel32.FILE_STANDARD_INFO)) + || info.EndOfFile >= preallocationSize) // existing files must NOT be shrinked + { + return; + } + } + + var eofInfo = new Interop.Kernel32.FILE_END_OF_FILE_INFO + { + EndOfFile = preallocationSize }; if (!Interop.Kernel32.SetFileInformationByHandle( fileHandle, - Interop.Kernel32.FileAllocationInfo, - &allocationInfo, - (uint)sizeof(Interop.Kernel32.FILE_ALLOCATION_INFO))) + Interop.Kernel32.FileEndOfFileInfo, + &eofInfo, + (uint)sizeof(Interop.Kernel32.FILE_END_OF_FILE_INFO))) { int errorCode = Marshal.GetLastPInvokeError(); + if (errorCode != Interop.Errors.ERROR_DISK_FULL && errorCode != Interop.Errors.ERROR_FILE_TOO_LARGE) + { + return; + } // 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) - { - 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 errorCode == Interop.Errors.ERROR_DISK_FULL + ? new IOException(SR.Format(SR.IO_DiskFull_Path_AllocationSize, fullPath, preallocationSize)) + : new IOException(SR.Format(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 5f3a5490294d51..2a757e5f52589c 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 @@ -1457,9 +1457,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 From 312a9c07fb8c055a133e5bb43913b8c12e60c3ec Mon Sep 17 00:00:00 2001 From: Adam Sitnik Date: Tue, 7 Sep 2021 16:44:47 +0200 Subject: [PATCH 3/9] ensure files cant be shrinked on Unix-like systems that dont implement posix_fallocate --- .../System.Native/Interop.PosixFAllocate.cs | 3 ++- .../Native/Unix/System.Native/pal_io.c | 18 +++++++++++++++++- .../Native/Unix/System.Native/pal_io.h | 2 +- .../Win32/SafeHandles/SafeFileHandle.Unix.cs | 2 +- 4 files changed, 21 insertions(+), 4 deletions(-) diff --git a/src/libraries/Common/src/Interop/Unix/System.Native/Interop.PosixFAllocate.cs b/src/libraries/Common/src/Interop/Unix/System.Native/Interop.PosixFAllocate.cs index 2866ed38935bdf..ba7c84c90f4d50 100644 --- a/src/libraries/Common/src/Interop/Unix/System.Native/Interop.PosixFAllocate.cs +++ b/src/libraries/Common/src/Interop/Unix/System.Native/Interop.PosixFAllocate.cs @@ -1,6 +1,7 @@ // 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; using System.Runtime.InteropServices; using Microsoft.Win32.SafeHandles; @@ -12,6 +13,6 @@ internal static partial class Sys /// Returns -1 on ENOSPC, -2 on EFBIG. On success or ignorable error, 0 is returned. /// [DllImport(Libraries.SystemNative, EntryPoint = "SystemNative_PosixFAllocate", SetLastError = false)] - internal static extern int PosixFAllocate(SafeFileHandle fd, long offset, long length); + internal static extern int PosixFAllocate(SafeFileHandle fd, long offset, long length, FileMode mode); } } diff --git a/src/libraries/Native/Unix/System.Native/pal_io.c b/src/libraries/Native/Unix/System.Native/pal_io.c index d86e8feb93584e..64b40091964f44 100644 --- a/src/libraries/Native/Unix/System.Native/pal_io.c +++ b/src/libraries/Native/Unix/System.Native/pal_io.c @@ -1008,11 +1008,27 @@ 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_PosixFAllocate(intptr_t fd, int64_t offset, int64_t length, int32_t mode) { assert_msg(offset == 0, "Invalid offset value", (int)offset); int fileDescriptor = ToFileDescriptor(fd); +#if !HAVE_POSIX_FALLOCATE64 && !HAVE_POSIX_FALLOCATE + // posix_fallocate does not allow for shrinking file + // when it's not available, we need to ensure that the file won't be shrinked + if (mode == 4) // OpenOrCreate + { + struct stat_ fileStat; + int fstatResult; + while ((fstatResult = fstat_(fileDescriptor, &fileStat)) < 0 && errno == EINTR); + + if (fstatResult != 0 || (fileStat.st_mode & S_IFMT) != S_IFREG || (int64_t)fileStat.st_size >= length) + { + return 0; + } + } +#endif + int32_t result; #if HAVE_POSIX_FALLOCATE64 // 64-bit Linux while ((result = posix_fallocate64(fileDescriptor, (off64_t)offset, (off64_t)length)) == EINTR); diff --git a/src/libraries/Native/Unix/System.Native/pal_io.h b/src/libraries/Native/Unix/System.Native/pal_io.h index 3f0db054d4368b..db9f92a6c8d84d 100644 --- a/src/libraries/Native/Unix/System.Native/pal_io.h +++ b/src/libraries/Native/Unix/System.Native/pal_io.h @@ -624,7 +624,7 @@ PALEXPORT int32_t SystemNative_PosixFAdvise(intptr_t fd, int64_t offset, int64_t * * Returns -1 on ENOSPC, -2 on EFBIG. On success or ignorable error, 0 is returned. */ -PALEXPORT int32_t SystemNative_PosixFAllocate(intptr_t fd, int64_t offset, int64_t length); +PALEXPORT int32_t SystemNative_PosixFAllocate(intptr_t fd, int64_t offset, int64_t length, int32_t mode); /** * Reads the number of bytes specified into the provided buffer from the specified, opened file descriptor. 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 ee1def85b9e024..a56947f4b5aefd 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 @@ -326,7 +326,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)) { - int fallocateResult = Interop.Sys.PosixFAllocate(this, 0, preallocationSize); + int fallocateResult = Interop.Sys.PosixFAllocate(this, 0, preallocationSize, mode); if (fallocateResult != 0) { Dispose(); From 8cf52c94df10387b978a8553c6d4f4f73b4ac536 Mon Sep 17 00:00:00 2001 From: Adam Sitnik Date: Tue, 7 Sep 2021 18:01:19 +0200 Subject: [PATCH 4/9] hopefully fix the Linux build --- src/libraries/Native/Unix/System.Native/pal_io.c | 2 ++ 1 file changed, 2 insertions(+) diff --git a/src/libraries/Native/Unix/System.Native/pal_io.c b/src/libraries/Native/Unix/System.Native/pal_io.c index 64b40091964f44..5e9bbd3e2f175e 100644 --- a/src/libraries/Native/Unix/System.Native/pal_io.c +++ b/src/libraries/Native/Unix/System.Native/pal_io.c @@ -1027,6 +1027,8 @@ int32_t SystemNative_PosixFAllocate(intptr_t fd, int64_t offset, int64_t length, return 0; } } +#else + (void)mode; #endif int32_t result; From e9efb9b479680aeb77a4b60ae8b0f468e1d1bb40 Mon Sep 17 00:00:00 2001 From: Adam Sitnik Date: Thu, 9 Sep 2021 14:31:58 +0200 Subject: [PATCH 5/9] address code review feedback --- .../tests/FileStream/ctor_options_as.cs | 9 +++++++-- 1 file changed, 7 insertions(+), 2 deletions(-) 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 48c4846385e74e..e2058d42b03b58 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 @@ -120,13 +120,18 @@ public void WhenFileIsBeingOpenedWithOpenOrCreateModeTheLengthCanBeOnlyExtended( { const int initialSize = 10; string filePath = GetPathToNonExistingFile(); - File.WriteAllBytes(filePath, new byte[initialSize]); + 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(Math.Max(initialSize, preallocationSize), fs.Length); // it was not shrinked 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 + AssertFileContentHasBeenZeroed(initialSize, (int)fs.Length, fs); } } @@ -210,7 +215,7 @@ private static void AssertFileContentHasBeenZeroed(int from, int to, FileStream { int expectedByteCount = to - from; int extraByteCount = 1; - byte[] content = RandomNumberGenerator.GetBytes(expectedByteCount + extraByteCount); + 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)); From 81fbd9271c01daf13e5c9375adf4a97a77a2e551 Mon Sep 17 00:00:00 2001 From: Adam Sitnik Date: Mon, 13 Sep 2021 12:23:57 +0200 Subject: [PATCH 6/9] FileMode.OpenOrCreate: don't change existing files --- .../System.Native/Interop.PosixFAllocate.cs | 3 +-- .../Native/Unix/System.Native/pal_io.c | 20 +------------------ .../Native/Unix/System.Native/pal_io.h | 2 +- .../tests/FileStream/ctor_options_as.cs | 10 ++++------ .../Win32/SafeHandles/SafeFileHandle.Unix.cs | 4 ++-- .../SafeHandles/SafeFileHandle.Windows.cs | 16 +++------------ .../System/IO/Strategies/FileStreamHelpers.cs | 5 +++-- 7 files changed, 15 insertions(+), 45 deletions(-) diff --git a/src/libraries/Common/src/Interop/Unix/System.Native/Interop.PosixFAllocate.cs b/src/libraries/Common/src/Interop/Unix/System.Native/Interop.PosixFAllocate.cs index ba7c84c90f4d50..2866ed38935bdf 100644 --- a/src/libraries/Common/src/Interop/Unix/System.Native/Interop.PosixFAllocate.cs +++ b/src/libraries/Common/src/Interop/Unix/System.Native/Interop.PosixFAllocate.cs @@ -1,7 +1,6 @@ // 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; using System.Runtime.InteropServices; using Microsoft.Win32.SafeHandles; @@ -13,6 +12,6 @@ internal static partial class Sys /// Returns -1 on ENOSPC, -2 on EFBIG. On success or ignorable error, 0 is returned. /// [DllImport(Libraries.SystemNative, EntryPoint = "SystemNative_PosixFAllocate", SetLastError = false)] - internal static extern int PosixFAllocate(SafeFileHandle fd, long offset, long length, FileMode mode); + internal static extern int PosixFAllocate(SafeFileHandle fd, long offset, long length); } } diff --git a/src/libraries/Native/Unix/System.Native/pal_io.c b/src/libraries/Native/Unix/System.Native/pal_io.c index 5e9bbd3e2f175e..d86e8feb93584e 100644 --- a/src/libraries/Native/Unix/System.Native/pal_io.c +++ b/src/libraries/Native/Unix/System.Native/pal_io.c @@ -1008,29 +1008,11 @@ 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 mode) +int32_t SystemNative_PosixFAllocate(intptr_t fd, int64_t offset, int64_t length) { assert_msg(offset == 0, "Invalid offset value", (int)offset); int fileDescriptor = ToFileDescriptor(fd); -#if !HAVE_POSIX_FALLOCATE64 && !HAVE_POSIX_FALLOCATE - // posix_fallocate does not allow for shrinking file - // when it's not available, we need to ensure that the file won't be shrinked - if (mode == 4) // OpenOrCreate - { - struct stat_ fileStat; - int fstatResult; - while ((fstatResult = fstat_(fileDescriptor, &fileStat)) < 0 && errno == EINTR); - - if (fstatResult != 0 || (fileStat.st_mode & S_IFMT) != S_IFREG || (int64_t)fileStat.st_size >= length) - { - return 0; - } - } -#else - (void)mode; -#endif - int32_t result; #if HAVE_POSIX_FALLOCATE64 // 64-bit Linux while ((result = posix_fallocate64(fileDescriptor, (off64_t)offset, (off64_t)length)) == EINTR); diff --git a/src/libraries/Native/Unix/System.Native/pal_io.h b/src/libraries/Native/Unix/System.Native/pal_io.h index db9f92a6c8d84d..3f0db054d4368b 100644 --- a/src/libraries/Native/Unix/System.Native/pal_io.h +++ b/src/libraries/Native/Unix/System.Native/pal_io.h @@ -624,7 +624,7 @@ PALEXPORT int32_t SystemNative_PosixFAdvise(intptr_t fd, int64_t offset, int64_t * * Returns -1 on ENOSPC, -2 on EFBIG. On success or ignorable error, 0 is returned. */ -PALEXPORT int32_t SystemNative_PosixFAllocate(intptr_t fd, int64_t offset, int64_t length, int32_t mode); +PALEXPORT int32_t SystemNative_PosixFAllocate(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/System.IO.FileSystem/tests/FileStream/ctor_options_as.cs b/src/libraries/System.IO.FileSystem/tests/FileStream/ctor_options_as.cs index e2058d42b03b58..b23ccb15739832 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 @@ -114,9 +114,9 @@ public void PreallocationSizeIsIgnoredForFileModeOpenAndAppend(FileMode mode, lo } [Theory] - [InlineData(FileMode.OpenOrCreate, 20L)] // this can extend the file - [InlineData(FileMode.OpenOrCreate, 5L)] // this should NOT shink the file - public void WhenFileIsBeingOpenedWithOpenOrCreateModeTheLengthCanBeOnlyExtended(FileMode mode, long preallocationSize) + [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(); @@ -125,14 +125,12 @@ public void WhenFileIsBeingOpenedWithOpenOrCreateModeTheLengthCanBeOnlyExtended( using (var fs = new FileStream(filePath, GetOptions(mode, FileAccess.ReadWrite, FileShare.None, FileOptions.None, preallocationSize))) { - Assert.Equal(Math.Max(initialSize, preallocationSize), fs.Length); // it was not shrinked + 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 - - AssertFileContentHasBeenZeroed(initialSize, (int)fs.Length, fs); } } 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 a56947f4b5aefd..777997444848fe 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,9 +324,9 @@ 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, mode); + int fallocateResult = Interop.Sys.PosixFAllocate(this, 0, preallocationSize); if (fallocateResult != 0) { Dispose(); 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 167f7afdc196a8..454ed1c892c787 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, mode); } @@ -106,20 +106,8 @@ private static unsafe SafeFileHandle CreateFile(string fullPath, FileMode mode, return 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 private static unsafe void Preallocate(string fullPath, long preallocationSize, SafeFileHandle fileHandle, FileMode mode) { - if (mode == FileMode.OpenOrCreate) - { - Interop.Kernel32.FILE_STANDARD_INFO info; - if (!Interop.Kernel32.GetFileInformationByHandleEx(fileHandle, Interop.Kernel32.FileStandardInfo, &info, (uint)sizeof(Interop.Kernel32.FILE_STANDARD_INFO)) - || info.EndOfFile >= preallocationSize) // existing files must NOT be shrinked - { - return; - } - } - var eofInfo = new Interop.Kernel32.FILE_END_OF_FILE_INFO { EndOfFile = preallocationSize @@ -131,6 +119,8 @@ private static unsafe void Preallocate(string fullPath, long preallocationSize, &eofInfo, (uint)sizeof(Interop.Kernel32.FILE_END_OF_FILE_INFO))) { + // preallocationSize must be ignored for non-seekable files, unsupported file systems + // and other failures other than ERROR_DISK_FULL and ERROR_FILE_TOO_LARGE int errorCode = Marshal.GetLastPInvokeError(); if (errorCode != Interop.Errors.ERROR_DISK_FULL && errorCode != Interop.Errors.ERROR_FILE_TOO_LARGE) { 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 d86c70a621ca1a..72bb0490a5e35a 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 a88f1333a4b071c425eeefff43c0a7604fa7a1ea Mon Sep 17 00:00:00 2001 From: Adam Sitnik Date: Mon, 13 Sep 2021 12:39:52 +0200 Subject: [PATCH 7/9] reduce code duplication --- .../SafeHandles/SafeFileHandle.Windows.cs | 22 ++++--------------- .../Strategies/FileStreamHelpers.Windows.cs | 19 ++++++++++++---- 2 files changed, 19 insertions(+), 22 deletions(-) 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 454ed1c892c787..5f4c21f02920bd 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 @@ -108,25 +108,11 @@ private static unsafe SafeFileHandle CreateFile(string fullPath, FileMode mode, private static unsafe void Preallocate(string fullPath, long preallocationSize, SafeFileHandle fileHandle, FileMode mode) { - var eofInfo = new Interop.Kernel32.FILE_END_OF_FILE_INFO + // 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) { - EndOfFile = preallocationSize - }; - - if (!Interop.Kernel32.SetFileInformationByHandle( - fileHandle, - Interop.Kernel32.FileEndOfFileInfo, - &eofInfo, - (uint)sizeof(Interop.Kernel32.FILE_END_OF_FILE_INFO))) - { - // preallocationSize must be ignored for non-seekable files, unsupported file systems - // and other failures other than ERROR_DISK_FULL and ERROR_FILE_TOO_LARGE - int errorCode = Marshal.GetLastPInvokeError(); - if (errorCode != Interop.Errors.ERROR_DISK_FULL && errorCode != Interop.Errors.ERROR_FILE_TOO_LARGE) - { - return; - } - // we try to mimic the atomic NtCreateFile here: // if preallocation fails, close the handle and delete the file fileHandle.Dispose(); 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 779acce9373717..5ba493ca9ca215 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) From 76951538c0ae96bcf4c7fbed386f52fa6cb96089 Mon Sep 17 00:00:00 2001 From: Adam Sitnik Date: Mon, 13 Sep 2021 16:14:30 +0200 Subject: [PATCH 8/9] Apply suggestions from code review Co-authored-by: Stephen Toub --- .../src/Microsoft/Win32/SafeHandles/SafeFileHandle.Windows.cs | 4 +--- .../src/System/IO/Strategies/FileStreamHelpers.cs | 2 +- 2 files changed, 2 insertions(+), 4 deletions(-) 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 5f4c21f02920bd..030260537b5c5b 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 @@ -118,9 +118,7 @@ private static unsafe void Preallocate(string fullPath, long preallocationSize, fileHandle.Dispose(); Interop.Kernel32.DeleteFile(fullPath); - throw errorCode == Interop.Errors.ERROR_DISK_FULL - ? new IOException(SR.Format(SR.IO_DiskFull_Path_AllocationSize, fullPath, preallocationSize)) - : new IOException(SR.Format(SR.IO_FileTooLarge_Path_AllocationSize, fullPath, preallocationSize)); + 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/IO/Strategies/FileStreamHelpers.cs b/src/libraries/System.Private.CoreLib/src/System/IO/Strategies/FileStreamHelpers.cs index 72bb0490a5e35a..981776da32b81d 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 @@ -62,7 +62,7 @@ internal static bool ShouldPreallocate(long preallocationSize, FileAccess access => 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 + && (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 faca672e61668a66758257420ff081af49ce0616 Mon Sep 17 00:00:00 2001 From: Adam Sitnik Date: Mon, 13 Sep 2021 16:45:28 +0200 Subject: [PATCH 9/9] update the comment, fix the build and remove unused parameter --- .../Win32/SafeHandles/SafeFileHandle.Windows.cs | 11 ++++++----- 1 file changed, 6 insertions(+), 5 deletions(-) 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 030260537b5c5b..3dde1e2e89a6b8 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 @@ -36,7 +36,7 @@ internal static unsafe SafeFileHandle Open(string fullPath, FileMode mode, FileA if (FileStreamHelpers.ShouldPreallocate(preallocationSize, access, mode, fileHandle)) { - Preallocate(fullPath, preallocationSize, fileHandle, mode); + Preallocate(fullPath, preallocationSize, fileHandle); } if ((options & FileOptions.Asynchronous) != 0) @@ -106,19 +106,20 @@ private static unsafe SafeFileHandle CreateFile(string fullPath, FileMode mode, return fileHandle; } - private static unsafe void Preallocate(string fullPath, long preallocationSize, SafeFileHandle fileHandle, 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) { - // we try to mimic the atomic NtCreateFile here: - // if preallocation fails, close the handle and delete the file + // 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); + throw new IOException(SR.Format(errorCode == Interop.Errors.ERROR_DISK_FULL ? SR.IO_DiskFull_Path_AllocationSize : SR.IO_FileTooLarge_Path_AllocationSize, fullPath, preallocationSize)); } }