From 0dbd989a7bcbb4d4bd15b8fb251b1fff1f608bcf Mon Sep 17 00:00:00 2001 From: Amal Abeygunawardana Date: Tue, 7 Mar 2023 22:59:32 +1100 Subject: [PATCH 01/13] Add CreateFromFile overload with SafeFileHandle The overload uses the handle the perform validations instead of instantiating a FileStream and calling the FileStream overload, because the FileStream instantiation appears to be heavy on allocations. --- .../ref/System.IO.MemoryMappedFiles.cs | 1 + .../IO/MemoryMappedFiles/MemoryMappedFile.cs | 50 +++++++++++++++++++ .../MemoryMappedFile.CreateFromFile.Tests.cs | 47 ++++++++++++++--- 3 files changed, 91 insertions(+), 7 deletions(-) diff --git a/src/libraries/System.IO.MemoryMappedFiles/ref/System.IO.MemoryMappedFiles.cs b/src/libraries/System.IO.MemoryMappedFiles/ref/System.IO.MemoryMappedFiles.cs index 2182c4a6d5b5ae..c5b786f48acdf8 100644 --- a/src/libraries/System.IO.MemoryMappedFiles/ref/System.IO.MemoryMappedFiles.cs +++ b/src/libraries/System.IO.MemoryMappedFiles/ref/System.IO.MemoryMappedFiles.cs @@ -25,6 +25,7 @@ public partial class MemoryMappedFile : System.IDisposable internal MemoryMappedFile() { } public Microsoft.Win32.SafeHandles.SafeMemoryMappedFileHandle SafeMemoryMappedFileHandle { get { throw null; } } public static System.IO.MemoryMappedFiles.MemoryMappedFile CreateFromFile(System.IO.FileStream fileStream, string? mapName, long capacity, System.IO.MemoryMappedFiles.MemoryMappedFileAccess access, System.IO.HandleInheritability inheritability, bool leaveOpen) { throw null; } + public static System.IO.MemoryMappedFiles.MemoryMappedFile CreateFromFile(Microsoft.Win32.SafeHandles.SafeFileHandle fileHandle, string? mapName, long capacity, System.IO.MemoryMappedFiles.MemoryMappedFileAccess access, System.IO.HandleInheritability inheritability, bool leaveOpen) { throw null; } public static System.IO.MemoryMappedFiles.MemoryMappedFile CreateFromFile(string path) { throw null; } public static System.IO.MemoryMappedFiles.MemoryMappedFile CreateFromFile(string path, System.IO.FileMode mode) { throw null; } public static System.IO.MemoryMappedFiles.MemoryMappedFile CreateFromFile(string path, System.IO.FileMode mode, string? mapName) { throw null; } diff --git a/src/libraries/System.IO.MemoryMappedFiles/src/System/IO/MemoryMappedFiles/MemoryMappedFile.cs b/src/libraries/System.IO.MemoryMappedFiles/src/System/IO/MemoryMappedFiles/MemoryMappedFile.cs index 57dc662749335a..5a5d4fffaed512 100644 --- a/src/libraries/System.IO.MemoryMappedFiles/src/System/IO/MemoryMappedFiles/MemoryMappedFile.cs +++ b/src/libraries/System.IO.MemoryMappedFiles/src/System/IO/MemoryMappedFiles/MemoryMappedFile.cs @@ -186,6 +186,56 @@ public static MemoryMappedFile CreateFromFile(string path, FileMode mode, string return new MemoryMappedFile(handle, fileHandle, false); } + public static MemoryMappedFile CreateFromFile(SafeFileHandle fileHandle, string? mapName, long capacity, + MemoryMappedFileAccess access, + HandleInheritability inheritability, bool leaveOpen) + { + ArgumentNullException.ThrowIfNull(fileHandle); + + if (mapName != null && mapName.Length == 0) + { + throw new ArgumentException(SR.Argument_MapNameEmptyString); + } + + if (capacity < 0) + { + throw new ArgumentOutOfRangeException(nameof(capacity), SR.ArgumentOutOfRange_PositiveOrDefaultCapacityRequired); + } + + long fileSize = RandomAccess.GetLength(fileHandle); + + if (capacity == 0 && fileSize == 0) + { + throw new ArgumentException(SR.Argument_EmptyFile); + } + + if (access < MemoryMappedFileAccess.ReadWrite || + access > MemoryMappedFileAccess.ReadWriteExecute) + { + throw new ArgumentOutOfRangeException(nameof(access)); + } + + if (access == MemoryMappedFileAccess.Write) + { + throw new ArgumentException(SR.Argument_NewMMFWriteAccessNotAllowed, nameof(access)); + } + + if (inheritability < HandleInheritability.None || inheritability > HandleInheritability.Inheritable) + { + throw new ArgumentOutOfRangeException(nameof(inheritability)); + } + + if (capacity == DefaultSize) + { + capacity = fileSize; + } + + SafeMemoryMappedFileHandle handle = CreateCore(fileHandle, mapName, inheritability, + access, MemoryMappedFileOptions.None, capacity, fileSize); + + return new MemoryMappedFile(handle, fileHandle, leaveOpen); + } + public static MemoryMappedFile CreateFromFile(FileStream fileStream, string? mapName, long capacity, MemoryMappedFileAccess access, HandleInheritability inheritability, bool leaveOpen) diff --git a/src/libraries/System.IO.MemoryMappedFiles/tests/MemoryMappedFile.CreateFromFile.Tests.cs b/src/libraries/System.IO.MemoryMappedFiles/tests/MemoryMappedFile.CreateFromFile.Tests.cs index 4e06fb078c8ef9..6f16c31b89c484 100644 --- a/src/libraries/System.IO.MemoryMappedFiles/tests/MemoryMappedFile.CreateFromFile.Tests.cs +++ b/src/libraries/System.IO.MemoryMappedFiles/tests/MemoryMappedFile.CreateFromFile.Tests.cs @@ -36,7 +36,7 @@ public void InvalidArguments_Path() public void InvalidArguments_FileStream() { // null is an invalid stream - AssertExtensions.Throws("fileStream", () => MemoryMappedFile.CreateFromFile(null, CreateUniqueMapName(), 4096, MemoryMappedFileAccess.Read, HandleInheritability.None, true)); + AssertExtensions.Throws("fileStream", () => MemoryMappedFile.CreateFromFile((FileStream)null, CreateUniqueMapName(), 4096, MemoryMappedFileAccess.Read, HandleInheritability.None, true)); } /// @@ -448,11 +448,44 @@ public static IEnumerable MemberData_ValidNameCapacityCombinationsWith } } + /// + /// Test various combinations of arguments to CreateFromFile that accepts a SafeFileHandle. + /// + [Theory] + [MemberData(nameof(ValidArgumentCombinationsWithStreamOrHandle), + new string[] { null, "CreateUniqueMapName()" }, + new long[] { 1, 256, -1 /*pagesize*/, 10000 }, + new MemoryMappedFileAccess[] { MemoryMappedFileAccess.Read, MemoryMappedFileAccess.ReadWrite, MemoryMappedFileAccess.CopyOnWrite }, + new HandleInheritability[] { HandleInheritability.None, HandleInheritability.Inheritable }, + new bool[] { false, true })] + [ActiveIssue("https://github.com/dotnet/runtime/issues/51375", TestPlatforms.iOS | TestPlatforms.tvOS | TestPlatforms.MacCatalyst)] + public void ValidArgumentCombinationsWithHandle( + string mapName, long capacity, MemoryMappedFileAccess access, HandleInheritability inheritability, bool leaveOpen) + { + // Create a file of the right size, then create the map for it. + using (TempFile file = new TempFile(GetTestFilePath(), capacity)) + using (FileStream fs = File.Open(file.Path, FileMode.Open)) + using (MemoryMappedFile mmf = MemoryMappedFile.CreateFromFile(fs.SafeFileHandle, mapName, capacity, access, inheritability, leaveOpen)) + { + ValidateMemoryMappedFile(mmf, capacity, access, inheritability); + } + + // Start with an empty file and let the map grow it to the right size. This requires write access. + if (IsWritable(access)) + { + using (FileStream fs = File.Create(GetTestFilePath())) + using (MemoryMappedFile mmf = MemoryMappedFile.CreateFromFile(fs.SafeFileHandle, mapName, capacity, access, inheritability, leaveOpen)) + { + ValidateMemoryMappedFile(mmf, capacity, access, inheritability); + } + } + } + /// /// Test various combinations of arguments to CreateFromFile that accepts a FileStream. /// [Theory] - [MemberData(nameof(MemberData_ValidArgumentCombinationsWithStream), + [MemberData(nameof(ValidArgumentCombinationsWithStreamOrHandle), new string[] { null, "CreateUniqueMapName()" }, new long[] { 1, 256, -1 /*pagesize*/, 10000 }, new MemoryMappedFileAccess[] { MemoryMappedFileAccess.Read, MemoryMappedFileAccess.ReadWrite, MemoryMappedFileAccess.CopyOnWrite }, @@ -482,10 +515,10 @@ public void ValidArgumentCombinationsWithStream( } /// - /// Provides input data to the ValidArgumentCombinationsWithStream tests, yielding the full matrix - /// of combinations of input values provided, except for those that are known to be unsupported - /// (e.g. non-null map names on Unix), and with appropriate values substituted in for placeholders - /// listed in the MemberData attribute (e.g. actual system page size instead of -1). + /// Provides input data to the ValidArgumentCombinationsWithStream or ValidArgumentCombinationsWithHandle + /// tests, yielding the full matrix of combinations of input values provided, except for those that are + /// known to be unsupported (e.g. non-null map names on Unix), and with appropriate values substituted + /// in for placeholders listed in the MemberData attribute (e.g. actual system page size instead of -1). /// /// /// The names to yield. @@ -498,7 +531,7 @@ public void ValidArgumentCombinationsWithStream( /// /// The inheritabilities to yield. /// The leaveOpen values to yield. - public static IEnumerable MemberData_ValidArgumentCombinationsWithStream( + public static IEnumerable ValidArgumentCombinationsWithStreamOrHandle( string[] mapNames, long[] capacities, MemoryMappedFileAccess[] accesses, HandleInheritability[] inheritabilities, bool[] leaveOpens) { foreach (string tmpMapName in mapNames) From 7c7c906d86d25931e87d4c77c111972b698bb628 Mon Sep 17 00:00:00 2001 From: Amal Abeygunawardana Date: Tue, 7 Mar 2023 23:23:50 +1100 Subject: [PATCH 02/13] Add invalid capacity tests --- .../MemoryMappedFile.CreateFromFile.Tests.cs | 30 +++++++++++++++++++ 1 file changed, 30 insertions(+) diff --git a/src/libraries/System.IO.MemoryMappedFiles/tests/MemoryMappedFile.CreateFromFile.Tests.cs b/src/libraries/System.IO.MemoryMappedFiles/tests/MemoryMappedFile.CreateFromFile.Tests.cs index 6f16c31b89c484..be88792edbcbc8 100644 --- a/src/libraries/System.IO.MemoryMappedFiles/tests/MemoryMappedFile.CreateFromFile.Tests.cs +++ b/src/libraries/System.IO.MemoryMappedFiles/tests/MemoryMappedFile.CreateFromFile.Tests.cs @@ -269,6 +269,36 @@ public void InvalidArguments_Capacity() } } + /// + /// Tests invalid arguments to the CreateFromFile capacity parameter + /// when used with SafeFileHandle parameter. + /// + [Fact] + public void InvalidCapacityCombinationsWithFileHandle() + { + using (TempFile file = new TempFile(GetTestFilePath())) + { + using (FileStream fs = File.Open(file.Path, FileMode.Open)) + { + SafeFileHandle fileHandle = fs.SafeFileHandle; + // Out of range values for capacity + Assert.Throws(() => MemoryMappedFile.CreateFromFile(fileHandle, null, -1, MemoryMappedFileAccess.Read, HandleInheritability.None, true)); + + // Default (0) capacity with an empty file + AssertExtensions.Throws(null, () => MemoryMappedFile.CreateFromFile(fileHandle, null, 0, MemoryMappedFileAccess.Read, HandleInheritability.None, true)); + AssertExtensions.Throws(null, () => MemoryMappedFile.CreateFromFile(fileHandle, CreateUniqueMapName(), 0, MemoryMappedFileAccess.Read, HandleInheritability.None, true)); + + // Larger capacity than the underlying file, but read-only such that we can't expand the file + fs.SetLength(4096); + AssertExtensions.Throws(null, () => MemoryMappedFile.CreateFromFile(fileHandle, null, 8192, MemoryMappedFileAccess.Read, HandleInheritability.None, true)); + AssertExtensions.Throws(null, () => MemoryMappedFile.CreateFromFile(fileHandle, CreateUniqueMapName(), 8192, MemoryMappedFileAccess.Read, HandleInheritability.None, true)); + + // Capacity can't be less than the file size (for such cases a view can be created with the smaller size) + AssertExtensions.Throws("capacity", () => MemoryMappedFile.CreateFromFile(fileHandle, null, 1, MemoryMappedFileAccess.ReadWrite, HandleInheritability.None, true)); + } + } + } + /// /// Tests invalid arguments to the CreateFromFile inheritability parameter. /// From dc96bfab9022b3f9aa8b851e0f492c3b1d297455 Mon Sep 17 00:00:00 2001 From: Amal Abeygunawardana Date: Wed, 8 Mar 2023 09:49:40 +1100 Subject: [PATCH 03/13] Refactor test name to align with existing --- .../tests/MemoryMappedFile.CreateFromFile.Tests.cs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/libraries/System.IO.MemoryMappedFiles/tests/MemoryMappedFile.CreateFromFile.Tests.cs b/src/libraries/System.IO.MemoryMappedFiles/tests/MemoryMappedFile.CreateFromFile.Tests.cs index be88792edbcbc8..631929cef37823 100644 --- a/src/libraries/System.IO.MemoryMappedFiles/tests/MemoryMappedFile.CreateFromFile.Tests.cs +++ b/src/libraries/System.IO.MemoryMappedFiles/tests/MemoryMappedFile.CreateFromFile.Tests.cs @@ -274,7 +274,7 @@ public void InvalidArguments_Capacity() /// when used with SafeFileHandle parameter. /// [Fact] - public void InvalidCapacityCombinationsWithFileHandle() + public void InvalidArguments_CapacityWithFileHandle() { using (TempFile file = new TempFile(GetTestFilePath())) { From 2fb6b0131d9d40207cfc10e43be47284f082bb57 Mon Sep 17 00:00:00 2001 From: Amal Abeygunawardana Date: Wed, 8 Mar 2023 10:48:17 +1100 Subject: [PATCH 04/13] Increase test coverage for SafeFileHandle --- .../MemoryMappedFile.CreateFromFile.Tests.cs | 174 +++++++++++++++++- 1 file changed, 171 insertions(+), 3 deletions(-) diff --git a/src/libraries/System.IO.MemoryMappedFiles/tests/MemoryMappedFile.CreateFromFile.Tests.cs b/src/libraries/System.IO.MemoryMappedFiles/tests/MemoryMappedFile.CreateFromFile.Tests.cs index 631929cef37823..cc8a3b17b8e8f4 100644 --- a/src/libraries/System.IO.MemoryMappedFiles/tests/MemoryMappedFile.CreateFromFile.Tests.cs +++ b/src/libraries/System.IO.MemoryMappedFiles/tests/MemoryMappedFile.CreateFromFile.Tests.cs @@ -39,6 +39,16 @@ public void InvalidArguments_FileStream() AssertExtensions.Throws("fileStream", () => MemoryMappedFile.CreateFromFile((FileStream)null, CreateUniqueMapName(), 4096, MemoryMappedFileAccess.Read, HandleInheritability.None, true)); } + /// + /// Tests invalid arguments to the CreateFromFile fileHandle parameter + /// + [Fact] + public void InvalidArguments_SafeFileHandle() + { + // null is an invalid stream + AssertExtensions.Throws("fileHandle", () => MemoryMappedFile.CreateFromFile((SafeFileHandle)null, CreateUniqueMapName(), 4096, MemoryMappedFileAccess.Read, HandleInheritability.None, true)); + } + /// /// Tests invalid arguments to the CreateFromFile mode parameter. /// @@ -103,6 +113,19 @@ public void InvalidArguments_Access() // Write-only access is not allowed AssertExtensions.Throws("access", () => MemoryMappedFile.CreateFromFile(fs, CreateUniqueMapName(), 4096, MemoryMappedFileAccess.Write, HandleInheritability.None, true)); } + + // Test the same things, but with a SafeFileHandle + using (TempFile file = new TempFile(GetTestFilePath())) + using (FileStream fs = File.Open(file.Path, FileMode.Open)) + { + SafeFileHandle fileHandle = fs.SafeFileHandle; + // Out of range values with a fileHandle + AssertExtensions.Throws("access", () => MemoryMappedFile.CreateFromFile(fileHandle, CreateUniqueMapName(), 4096, (MemoryMappedFileAccess)(-2), HandleInheritability.None, true)); + AssertExtensions.Throws("access", () => MemoryMappedFile.CreateFromFile(fileHandle, CreateUniqueMapName(), 4096, (MemoryMappedFileAccess)(42), HandleInheritability.None, true)); + + // Write-only access is not allowed + AssertExtensions.Throws("access", () => MemoryMappedFile.CreateFromFile(fileHandle, CreateUniqueMapName(), 4096, MemoryMappedFileAccess.Write, HandleInheritability.None, true)); + } } /// @@ -115,7 +138,7 @@ public void InvalidArguments_Access() [InlineData(FileAccess.ReadWrite, MemoryMappedFileAccess.CopyOnWrite)] [InlineData(FileAccess.Read, MemoryMappedFileAccess.Read)] [InlineData(FileAccess.Read, MemoryMappedFileAccess.CopyOnWrite)] - public void FileAccessAndMapAccessCombinations_Valid(FileAccess fileAccess, MemoryMappedFileAccess mmfAccess) + public void FileAccessAndMapAccessCombinationsWithFileStream_Valid(FileAccess fileAccess, MemoryMappedFileAccess mmfAccess) { const int Capacity = 4096; using (TempFile file = new TempFile(GetTestFilePath(), Capacity)) @@ -126,6 +149,27 @@ public void FileAccessAndMapAccessCombinations_Valid(FileAccess fileAccess, Memo } } + /// + /// Tests various values of FileAccess used to construct a SafeFileHandle and MemoryMappedFileAccess used + /// to construct a map over that file handle. The combinations should all be valid. + /// + [Theory] + [InlineData(FileAccess.ReadWrite, MemoryMappedFileAccess.Read)] + [InlineData(FileAccess.ReadWrite, MemoryMappedFileAccess.ReadWrite)] + [InlineData(FileAccess.ReadWrite, MemoryMappedFileAccess.CopyOnWrite)] + [InlineData(FileAccess.Read, MemoryMappedFileAccess.Read)] + [InlineData(FileAccess.Read, MemoryMappedFileAccess.CopyOnWrite)] + public void FileAccessAndMapAccessCombinationsWithSafeFileHandle_Valid(FileAccess fileAccess, MemoryMappedFileAccess mmfAccess) + { + const int Capacity = 4096; + using (TempFile file = new TempFile(GetTestFilePath(), Capacity)) + using (FileStream fs = new FileStream(file.Path, FileMode.Open, fileAccess)) + using (MemoryMappedFile mmf = MemoryMappedFile.CreateFromFile(fs.SafeFileHandle, null, Capacity, mmfAccess, HandleInheritability.None, true)) + { + ValidateMemoryMappedFile(mmf, Capacity, mmfAccess); + } + } + /// /// Tests various values of FileAccess used to construct a FileStream and MemoryMappedFileAccess used /// to construct a map over that stream on Windows. The combinations should all be invalid, resulting in exception. @@ -196,6 +240,7 @@ public void InvalidArguments_MapName() using (FileStream fs = File.Open(file.Path, FileMode.Open)) { AssertExtensions.Throws(null, () => MemoryMappedFile.CreateFromFile(fs, string.Empty, 4096, MemoryMappedFileAccess.ReadWrite, HandleInheritability.None, true)); + AssertExtensions.Throws(null, () => MemoryMappedFile.CreateFromFile(fs.SafeFileHandle, string.Empty, 4096, MemoryMappedFileAccess.ReadWrite, HandleInheritability.None, true)); } } } @@ -312,6 +357,7 @@ public void InvalidArguments_Inheritability(HandleInheritability inheritability) using (FileStream fs = File.Open(file.Path, FileMode.Open)) { AssertExtensions.Throws("inheritability", () => MemoryMappedFile.CreateFromFile(fs, CreateUniqueMapName(), 4096, MemoryMappedFileAccess.ReadWrite, inheritability, true)); + AssertExtensions.Throws("inheritability", () => MemoryMappedFile.CreateFromFile(fs.SafeFileHandle, CreateUniqueMapName(), 4096, MemoryMappedFileAccess.ReadWrite, inheritability, true)); } } @@ -615,6 +661,14 @@ public void DefaultCapacityIsFileLength() { ValidateMemoryMappedFile(mmf, DesiredCapacity); } + + // With file handle + using (TempFile file = new TempFile(GetTestFilePath(), DesiredCapacity)) + using (FileStream fs = File.Open(file.Path, FileMode.Open)) + using (MemoryMappedFile mmf = MemoryMappedFile.CreateFromFile(fs.SafeFileHandle, null, DefaultCapacity, MemoryMappedFileAccess.ReadWrite, HandleInheritability.None, true)) + { + ValidateMemoryMappedFile(mmf, DesiredCapacity); + } } /// @@ -774,7 +828,7 @@ public void WriteToReadOnlyFile(MemoryMappedFileAccess access, bool shouldSuccee [Theory] [InlineData(true)] [InlineData(false)] - public void LeaveOpenRespected_Basic(bool leaveOpen) + public void LeaveOpenRespectedWithFileStream_Basic(bool leaveOpen) { const int Capacity = 4096; @@ -793,6 +847,32 @@ public void LeaveOpenRespected_Basic(bool leaveOpen) } } + /// + /// Test to ensure that leaveOpen is appropriately respected, either leaving the SafeFileHandle open + /// or closing it on disposal. + /// + [Theory] + [InlineData(true)] + [InlineData(false)] + public void LeaveOpenRespectedWithSafeFileHandle_Basic(bool leaveOpen) + { + const int Capacity = 4096; + + using (TempFile file = new TempFile(GetTestFilePath())) + using (FileStream fs = File.Open(file.Path, FileMode.Open)) + { + // Handle should still be open + SafeFileHandle handle = fs.SafeFileHandle; + Assert.False(handle.IsClosed); + + // Create and close the map + MemoryMappedFile.CreateFromFile(handle, null, Capacity, MemoryMappedFileAccess.ReadWrite, HandleInheritability.None, leaveOpen).Dispose(); + + // The handle should now be open if leaveOpen + Assert.NotEqual(leaveOpen, handle.IsClosed); + } + } + /// /// Test to ensure that leaveOpen is appropriately respected, either leaving the FileStream open /// or closing it on disposal. @@ -800,7 +880,7 @@ public void LeaveOpenRespected_Basic(bool leaveOpen) [Theory] [InlineData(true)] [InlineData(false)] - public void LeaveOpenRespected_OutstandingViews(bool leaveOpen) + public void LeaveOpenRespectedWithFileStream_OutstandingViews(bool leaveOpen) { const int Capacity = 4096; using (TempFile file = new TempFile(GetTestFilePath())) @@ -826,6 +906,39 @@ public void LeaveOpenRespected_OutstandingViews(bool leaveOpen) } } + /// + /// Test to ensure that leaveOpen is appropriately respected, either leaving the SafeFileHandle open + /// or closing it on disposal. + /// + [Theory] + [InlineData(true)] + [InlineData(false)] + public void LeaveOpenRespectedWithSafeFileHandle_OutstandingViews(bool leaveOpen) + { + const int Capacity = 4096; + using (TempFile file = new TempFile(GetTestFilePath())) + using (FileStream fs = File.Open(file.Path, FileMode.Open)) + { + // Handle should still be open + SafeFileHandle handle = fs.SafeFileHandle; + Assert.False(handle.IsClosed); + + // Create the map, create each of the views, then close the map + using (MemoryMappedFile mmf = MemoryMappedFile.CreateFromFile(handle, null, Capacity, MemoryMappedFileAccess.ReadWrite, HandleInheritability.None, leaveOpen)) + using (MemoryMappedViewAccessor acc = mmf.CreateViewAccessor(0, Capacity)) + using (MemoryMappedViewStream s = mmf.CreateViewStream(0, Capacity)) + { + // Explicitly close the map. The handle should now be open iff leaveOpen. + mmf.Dispose(); + Assert.NotEqual(leaveOpen, handle.IsClosed); + + // But the views should still be usable. + ValidateMemoryMappedViewAccessor(acc, Capacity, MemoryMappedFileAccess.ReadWrite); + ValidateMemoryMappedViewStream(s, Capacity, MemoryMappedFileAccess.ReadWrite); + } + } + } + /// /// Test to validate we can create multiple maps from the same FileStream. /// @@ -863,6 +976,43 @@ public void MultipleMapsForTheSameFileStream() } } + /// + /// Test to validate we can create multiple maps from the same SafeFileHandle. + /// + [Fact] + [SkipOnPlatform(TestPlatforms.Browser, "the emscripten implementation doesn't share data")] + public void MultipleMapsForTheSameSafeFileHandle() + { + const int Capacity = 4096; + using (TempFile file = new TempFile(GetTestFilePath(), Capacity)) + using (FileStream fs = new FileStream(file.Path, FileMode.Open)) + using (MemoryMappedFile mmf1 = MemoryMappedFile.CreateFromFile(fs.SafeFileHandle, null, Capacity, MemoryMappedFileAccess.ReadWrite, HandleInheritability.None, true)) + using (MemoryMappedFile mmf2 = MemoryMappedFile.CreateFromFile(fs.SafeFileHandle, null, Capacity, MemoryMappedFileAccess.ReadWrite, HandleInheritability.None, true)) + using (MemoryMappedViewAccessor acc1 = mmf1.CreateViewAccessor()) + using (MemoryMappedViewAccessor acc2 = mmf2.CreateViewAccessor()) + { + // The capacity of the two maps should be equal + Assert.Equal(acc1.Capacity, acc2.Capacity); + + var rand = new Random(); + for (int i = 1; i <= 10; i++) + { + // Write a value to one map, then read it from the other, + // ping-ponging between the two. + int pos = rand.Next((int)acc1.Capacity - 1); + MemoryMappedViewAccessor reader = acc1, writer = acc2; + if (i % 2 == 0) + { + reader = acc2; + writer = acc1; + } + writer.Write(pos, (byte)i); + writer.Flush(); + Assert.Equal(i, reader.ReadByte(pos)); + } + } + } + /// /// Test to verify that the map's size increases the underlying file size if the map's capacity is larger. /// @@ -1026,6 +1176,24 @@ public void MapHandleMatchesFileStreamHandle() } } + /// + /// Test to verify that the MemoryMappedFile has the same underlying handle as the SafeFileHandle it's created from + /// + [PlatformSpecific(TestPlatforms.AnyUnix)] + [Fact] + public void MapHandleMatchesSafeFileHandle() + { + using (FileStream fs = File.OpenWrite(GetTestFilePath())) + { + SafeFileHandle fileHandle = fs.SafeFileHandle; + using (MemoryMappedFile mmf = MemoryMappedFile.CreateFromFile(fileHandle, null, 4096, MemoryMappedFileAccess.ReadWrite, HandleInheritability.None, false)) + { + SafeMemoryMappedFileHandle handle = mmf.SafeMemoryMappedFileHandle; + Assert.Equal(fileHandle.DangerousGetHandle(), handle.DangerousGetHandle()); + } + } + } + [Theory] [InlineData(0)] [InlineData(1)] From a6204ffec3a20b39ff80308b85c96161cdbf0dee Mon Sep 17 00:00:00 2001 From: Amal Abeygunawardana Date: Wed, 8 Mar 2023 23:01:34 +1100 Subject: [PATCH 05/13] Add API docs for the new overload --- .../IO/MemoryMappedFiles/MemoryMappedFile.cs | 37 +++++++++++++++++++ 1 file changed, 37 insertions(+) diff --git a/src/libraries/System.IO.MemoryMappedFiles/src/System/IO/MemoryMappedFiles/MemoryMappedFile.cs b/src/libraries/System.IO.MemoryMappedFiles/src/System/IO/MemoryMappedFiles/MemoryMappedFile.cs index 5a5d4fffaed512..7031f958dc241c 100644 --- a/src/libraries/System.IO.MemoryMappedFiles/src/System/IO/MemoryMappedFiles/MemoryMappedFile.cs +++ b/src/libraries/System.IO.MemoryMappedFiles/src/System/IO/MemoryMappedFiles/MemoryMappedFile.cs @@ -186,6 +186,43 @@ public static MemoryMappedFile CreateFromFile(string path, FileMode mode, string return new MemoryMappedFile(handle, fileHandle, false); } + /// + /// Creates a memory-mapped file from an existing file using a SafeFileHandle, + /// and the specified access mode, name, inheritability, and capacity. + /// + /// The SafeFileHandle to the existing file. Caller is + /// responsible for disposing fileHandle when ==true (otherwise, + /// automatically disposed by the MemoryMappedFile). + /// A name to assign to the memory-mapped file, or null for a + /// that you do not intend to share across processes. + /// The maximum size, in bytes, to allocate to the memory-mapped file. + /// Specify 0 to set the capacity to the size of filestream. + /// One of the enumeration values that specifies the type of access allowed + /// to the memory-mapped file.This parameter can't be set to Write. + /// One of the enumeration values that specifies whether a handle + /// to the memory-mapped file can be inherited by a child process. The default is None. + /// A value that indicates whether to close the source file handle when + /// the is disposed. + /// A memory-mapped file that has the specified characteristics. + /// + /// mapName is null or an empty string. + /// -or- + /// capacity and the length of the file are zero. + /// -or- + /// access is set to Write or Write enumeration value, which is not allowed. + /// -or- + /// access is set to Read and capacity is larger than the length of fileHandle. + /// + /// fileHanlde is null. + /// + /// capacity is less than zero. + /// -or- + /// capacity is less than the file size. + /// -or- + /// access is not a valid enumeration value. + /// -or- + /// inheritability is not a valid enumeration value. + /// public static MemoryMappedFile CreateFromFile(SafeFileHandle fileHandle, string? mapName, long capacity, MemoryMappedFileAccess access, HandleInheritability inheritability, bool leaveOpen) From a1e9215bf9b59325c5b70597c3a94bcb49e10592 Mon Sep 17 00:00:00 2001 From: Amal Abeygunawardana Date: Wed, 8 Mar 2023 23:22:09 +1100 Subject: [PATCH 06/13] Remove unnecessary line --- .../src/System/IO/MemoryMappedFiles/MemoryMappedFile.cs | 1 - 1 file changed, 1 deletion(-) diff --git a/src/libraries/System.IO.MemoryMappedFiles/src/System/IO/MemoryMappedFiles/MemoryMappedFile.cs b/src/libraries/System.IO.MemoryMappedFiles/src/System/IO/MemoryMappedFiles/MemoryMappedFile.cs index 7031f958dc241c..ecc687ca060e2d 100644 --- a/src/libraries/System.IO.MemoryMappedFiles/src/System/IO/MemoryMappedFiles/MemoryMappedFile.cs +++ b/src/libraries/System.IO.MemoryMappedFiles/src/System/IO/MemoryMappedFiles/MemoryMappedFile.cs @@ -240,7 +240,6 @@ public static MemoryMappedFile CreateFromFile(SafeFileHandle fileHandle, string? } long fileSize = RandomAccess.GetLength(fileHandle); - if (capacity == 0 && fileSize == 0) { throw new ArgumentException(SR.Argument_EmptyFile); From 91a50cc475b1fa5972d198f03648c8b2ce9d7625 Mon Sep 17 00:00:00 2001 From: Amal Abeygunawardana Date: Thu, 16 Mar 2023 10:56:23 +1100 Subject: [PATCH 07/13] Fix xml doc issues --- .../IO/MemoryMappedFiles/MemoryMappedFile.cs | 44 +++++++++---------- 1 file changed, 22 insertions(+), 22 deletions(-) diff --git a/src/libraries/System.IO.MemoryMappedFiles/src/System/IO/MemoryMappedFiles/MemoryMappedFile.cs b/src/libraries/System.IO.MemoryMappedFiles/src/System/IO/MemoryMappedFiles/MemoryMappedFile.cs index ecc687ca060e2d..0aa73fa49d2713 100644 --- a/src/libraries/System.IO.MemoryMappedFiles/src/System/IO/MemoryMappedFiles/MemoryMappedFile.cs +++ b/src/libraries/System.IO.MemoryMappedFiles/src/System/IO/MemoryMappedFiles/MemoryMappedFile.cs @@ -190,38 +190,38 @@ public static MemoryMappedFile CreateFromFile(string path, FileMode mode, string /// Creates a memory-mapped file from an existing file using a SafeFileHandle, /// and the specified access mode, name, inheritability, and capacity. /// - /// The SafeFileHandle to the existing file. Caller is - /// responsible for disposing fileHandle when ==true (otherwise, - /// automatically disposed by the MemoryMappedFile). - /// A name to assign to the memory-mapped file, or null for a - /// that you do not intend to share across processes. - /// The maximum size, in bytes, to allocate to the memory-mapped file. - /// Specify 0 to set the capacity to the size of filestream. - /// One of the enumeration values that specifies the type of access allowed - /// to the memory-mapped file.This parameter can't be set to Write. - /// One of the enumeration values that specifies whether a handle - /// to the memory-mapped file can be inherited by a child process. The default is None. - /// A value that indicates whether to close the source file handle when - /// the is disposed. + /// The SafeFileHandle to the existing file. Caller is + /// responsible for disposing when is (otherwise, + /// automatically disposed by the ). + /// A name to assign to the memory-mapped file, or for a + /// that you do not intend to share across processes. + /// The maximum size, in bytes, to allocate to the memory-mapped file. + /// Specify 0 to set the capacity to the size of the file. + /// One of the enumeration values that specifies the type of access allowed + /// to the memory-mapped file.This parameter can't be set to Write. + /// One of the enumeration values that specifies whether a handle + /// to the memory-mapped file can be inherited by a child process. The default is None. + /// A value that indicates whether to close the source file handle when + /// the is disposed. /// A memory-mapped file that has the specified characteristics. /// - /// mapName is null or an empty string. + /// is or an empty string. /// -or- - /// capacity and the length of the file are zero. + /// and the length of the file are zero. /// -or- - /// access is set to Write or Write enumeration value, which is not allowed. + /// is set to Write or Write enumeration value, which is not allowed. /// -or- - /// access is set to Read and capacity is larger than the length of fileHandle. + /// access is set to Read and is larger than the length of the file. /// - /// fileHanlde is null. + /// is . /// - /// capacity is less than zero. + /// is less than zero. /// -or- - /// capacity is less than the file size. + /// is less than the file size. /// -or- - /// access is not a valid enumeration value. + /// is not a valid enumeration value. /// -or- - /// inheritability is not a valid enumeration value. + /// is not a valid enumeration value. /// public static MemoryMappedFile CreateFromFile(SafeFileHandle fileHandle, string? mapName, long capacity, MemoryMappedFileAccess access, From d1d8d53abe0e84ef4c895936c4b3c2cb2053e5f6 Mon Sep 17 00:00:00 2001 From: Amal Abeygunawardana Date: Thu, 16 Mar 2023 11:01:28 +1100 Subject: [PATCH 08/13] Fix paramref and param mistakes --- .../IO/MemoryMappedFiles/MemoryMappedFile.cs | 24 +++++++++---------- 1 file changed, 12 insertions(+), 12 deletions(-) diff --git a/src/libraries/System.IO.MemoryMappedFiles/src/System/IO/MemoryMappedFiles/MemoryMappedFile.cs b/src/libraries/System.IO.MemoryMappedFiles/src/System/IO/MemoryMappedFiles/MemoryMappedFile.cs index 0aa73fa49d2713..68b35024987858 100644 --- a/src/libraries/System.IO.MemoryMappedFiles/src/System/IO/MemoryMappedFiles/MemoryMappedFile.cs +++ b/src/libraries/System.IO.MemoryMappedFiles/src/System/IO/MemoryMappedFiles/MemoryMappedFile.cs @@ -190,19 +190,19 @@ public static MemoryMappedFile CreateFromFile(string path, FileMode mode, string /// Creates a memory-mapped file from an existing file using a SafeFileHandle, /// and the specified access mode, name, inheritability, and capacity. /// - /// The SafeFileHandle to the existing file. Caller is + /// The SafeFileHandle to the existing file. Caller is /// responsible for disposing when is (otherwise, - /// automatically disposed by the ). - /// A name to assign to the memory-mapped file, or for a - /// that you do not intend to share across processes. - /// The maximum size, in bytes, to allocate to the memory-mapped file. - /// Specify 0 to set the capacity to the size of the file. - /// One of the enumeration values that specifies the type of access allowed - /// to the memory-mapped file.This parameter can't be set to Write. - /// One of the enumeration values that specifies whether a handle - /// to the memory-mapped file can be inherited by a child process. The default is None. - /// A value that indicates whether to close the source file handle when - /// the is disposed. + /// automatically disposed by the ). + /// A name to assign to the memory-mapped file, or for a + /// that you do not intend to share across processes. + /// The maximum size, in bytes, to allocate to the memory-mapped file. + /// Specify 0 to set the capacity to the size of the file. + /// One of the enumeration values that specifies the type of access allowed + /// to the memory-mapped file.This parameter can't be set to Write. + /// One of the enumeration values that specifies whether a handle + /// to the memory-mapped file can be inherited by a child process. The default is None. + /// A value that indicates whether to close the source file handle when + /// the is disposed. /// A memory-mapped file that has the specified characteristics. /// /// is or an empty string. From c52a66c244a7cf75c794d15e476882e4764bcc4f Mon Sep 17 00:00:00 2001 From: Amal Abeygunawardana Date: Thu, 16 Mar 2023 14:20:57 +1100 Subject: [PATCH 09/13] Separate and reuse validation logic --- .../IO/MemoryMappedFiles/MemoryMappedFile.cs | 89 ++++++------------- 1 file changed, 27 insertions(+), 62 deletions(-) diff --git a/src/libraries/System.IO.MemoryMappedFiles/src/System/IO/MemoryMappedFiles/MemoryMappedFile.cs b/src/libraries/System.IO.MemoryMappedFiles/src/System/IO/MemoryMappedFiles/MemoryMappedFile.cs index 68b35024987858..046bd910be33b8 100644 --- a/src/libraries/System.IO.MemoryMappedFiles/src/System/IO/MemoryMappedFiles/MemoryMappedFile.cs +++ b/src/libraries/System.IO.MemoryMappedFiles/src/System/IO/MemoryMappedFiles/MemoryMappedFile.cs @@ -107,22 +107,7 @@ public static MemoryMappedFile CreateFromFile(string path, FileMode mode, string MemoryMappedFileAccess access) { ArgumentNullException.ThrowIfNull(path); - - if (mapName != null && mapName.Length == 0) - { - throw new ArgumentException(SR.Argument_MapNameEmptyString); - } - - if (capacity < 0) - { - throw new ArgumentOutOfRangeException(nameof(capacity), SR.ArgumentOutOfRange_PositiveOrDefaultCapacityRequired); - } - - if (access < MemoryMappedFileAccess.ReadWrite || - access > MemoryMappedFileAccess.ReadWriteExecute) - { - throw new ArgumentOutOfRangeException(nameof(access)); - } + ValidateCreateFile(mapName, capacity, access); if (mode == FileMode.Append) { @@ -132,10 +117,6 @@ public static MemoryMappedFile CreateFromFile(string path, FileMode mode, string { throw new ArgumentException(SR.Argument_NewMMFTruncateModeNotAllowed, nameof(mode)); } - if (access == MemoryMappedFileAccess.Write) - { - throw new ArgumentException(SR.Argument_NewMMFWriteAccessNotAllowed, nameof(access)); - } bool existed = mode switch { @@ -228,16 +209,7 @@ public static MemoryMappedFile CreateFromFile(SafeFileHandle fileHandle, string? HandleInheritability inheritability, bool leaveOpen) { ArgumentNullException.ThrowIfNull(fileHandle); - - if (mapName != null && mapName.Length == 0) - { - throw new ArgumentException(SR.Argument_MapNameEmptyString); - } - - if (capacity < 0) - { - throw new ArgumentOutOfRangeException(nameof(capacity), SR.ArgumentOutOfRange_PositiveOrDefaultCapacityRequired); - } + ValidateCreateFile(mapName, capacity, access); long fileSize = RandomAccess.GetLength(fileHandle); if (capacity == 0 && fileSize == 0) @@ -245,17 +217,6 @@ public static MemoryMappedFile CreateFromFile(SafeFileHandle fileHandle, string? throw new ArgumentException(SR.Argument_EmptyFile); } - if (access < MemoryMappedFileAccess.ReadWrite || - access > MemoryMappedFileAccess.ReadWriteExecute) - { - throw new ArgumentOutOfRangeException(nameof(access)); - } - - if (access == MemoryMappedFileAccess.Write) - { - throw new ArgumentException(SR.Argument_NewMMFWriteAccessNotAllowed, nameof(access)); - } - if (inheritability < HandleInheritability.None || inheritability > HandleInheritability.Inheritable) { throw new ArgumentOutOfRangeException(nameof(inheritability)); @@ -277,16 +238,7 @@ public static MemoryMappedFile CreateFromFile(FileStream fileStream, string? map HandleInheritability inheritability, bool leaveOpen) { ArgumentNullException.ThrowIfNull(fileStream); - - if (mapName != null && mapName.Length == 0) - { - throw new ArgumentException(SR.Argument_MapNameEmptyString); - } - - if (capacity < 0) - { - throw new ArgumentOutOfRangeException(nameof(capacity), SR.ArgumentOutOfRange_PositiveOrDefaultCapacityRequired); - } + ValidateCreateFile(mapName, capacity, access); long fileSize = fileStream.Length; if (capacity == 0 && fileSize == 0) @@ -294,17 +246,6 @@ public static MemoryMappedFile CreateFromFile(FileStream fileStream, string? map throw new ArgumentException(SR.Argument_EmptyFile); } - if (access < MemoryMappedFileAccess.ReadWrite || - access > MemoryMappedFileAccess.ReadWriteExecute) - { - throw new ArgumentOutOfRangeException(nameof(access)); - } - - if (access == MemoryMappedFileAccess.Write) - { - throw new ArgumentException(SR.Argument_NewMMFWriteAccessNotAllowed, nameof(access)); - } - if (inheritability < HandleInheritability.None || inheritability > HandleInheritability.Inheritable) { throw new ArgumentOutOfRangeException(nameof(inheritability)); @@ -569,5 +510,29 @@ private static void CleanupFile(SafeFileHandle fileHandle, bool existed, string File.Delete(path); } } + + private static void ValidateCreateFile(string? mapName, long capacity, MemoryMappedFileAccess access) + { + if (mapName != null && mapName.Length == 0) + { + throw new ArgumentException(SR.Argument_MapNameEmptyString); + } + + if (capacity < 0) + { + throw new ArgumentOutOfRangeException(nameof(capacity), SR.ArgumentOutOfRange_PositiveOrDefaultCapacityRequired); + } + + if (access < MemoryMappedFileAccess.ReadWrite || + access > MemoryMappedFileAccess.ReadWriteExecute) + { + throw new ArgumentOutOfRangeException(nameof(access)); + } + + if (access == MemoryMappedFileAccess.Write) + { + throw new ArgumentException(SR.Argument_NewMMFWriteAccessNotAllowed, nameof(access)); + } + } } } From 1e2cb2a1643fbc04e994b3968180a48835a7ca2a Mon Sep 17 00:00:00 2001 From: Amal Abeygunawardana Date: Fri, 17 Mar 2023 22:12:05 +1100 Subject: [PATCH 10/13] Tag failing test with the related active issue --- .../tests/MemoryMappedFile.CreateFromFile.Tests.cs | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/src/libraries/System.IO.MemoryMappedFiles/tests/MemoryMappedFile.CreateFromFile.Tests.cs b/src/libraries/System.IO.MemoryMappedFiles/tests/MemoryMappedFile.CreateFromFile.Tests.cs index ee99a2d04f94ef..3a5d4ff49e1d76 100644 --- a/src/libraries/System.IO.MemoryMappedFiles/tests/MemoryMappedFile.CreateFromFile.Tests.cs +++ b/src/libraries/System.IO.MemoryMappedFiles/tests/MemoryMappedFile.CreateFromFile.Tests.cs @@ -881,7 +881,7 @@ public void LeaveOpenRespectedWithSafeFileHandle_Basic(bool leaveOpen) [InlineData(true)] [InlineData(false)] [ActiveIssue("https://github.com/dotnet/runtime/issues/83197", TestPlatforms.Browser)] - public void LeaveOpenRespected_OutstandingViews(bool leaveOpen) + public void LeaveOpenRespectedWithFileStream_OutstandingViews(bool leaveOpen) { const int Capacity = 4096; using (TempFile file = new TempFile(GetTestFilePath())) @@ -914,6 +914,7 @@ public void LeaveOpenRespected_OutstandingViews(bool leaveOpen) [Theory] [InlineData(true)] [InlineData(false)] + [ActiveIssue("https://github.com/dotnet/runtime/issues/83197", TestPlatforms.Browser)] public void LeaveOpenRespectedWithSafeFileHandle_OutstandingViews(bool leaveOpen) { const int Capacity = 4096; From 41410d1d7ae0e6d7cacb4c3166543d2357742ce4 Mon Sep 17 00:00:00 2001 From: Amal Abeygunawardana Date: Wed, 22 Mar 2023 21:03:56 +1100 Subject: [PATCH 11/13] Fix xmldoc formatting issues --- .../IO/MemoryMappedFiles/MemoryMappedFile.cs | 35 ++++++++++--------- 1 file changed, 18 insertions(+), 17 deletions(-) diff --git a/src/libraries/System.IO.MemoryMappedFiles/src/System/IO/MemoryMappedFiles/MemoryMappedFile.cs b/src/libraries/System.IO.MemoryMappedFiles/src/System/IO/MemoryMappedFiles/MemoryMappedFile.cs index 046bd910be33b8..49b33555f120dd 100644 --- a/src/libraries/System.IO.MemoryMappedFiles/src/System/IO/MemoryMappedFiles/MemoryMappedFile.cs +++ b/src/libraries/System.IO.MemoryMappedFiles/src/System/IO/MemoryMappedFiles/MemoryMappedFile.cs @@ -168,10 +168,10 @@ public static MemoryMappedFile CreateFromFile(string path, FileMode mode, string } /// - /// Creates a memory-mapped file from an existing file using a SafeFileHandle, + /// Creates a memory-mapped file from an existing file using a , /// and the specified access mode, name, inheritability, and capacity. /// - /// The SafeFileHandle to the existing file. Caller is + /// The to the existing file. Caller is /// responsible for disposing when is (otherwise, /// automatically disposed by the ). /// A name to assign to the memory-mapped file, or for a @@ -179,30 +179,31 @@ public static MemoryMappedFile CreateFromFile(string path, FileMode mode, string /// The maximum size, in bytes, to allocate to the memory-mapped file. /// Specify 0 to set the capacity to the size of the file. /// One of the enumeration values that specifies the type of access allowed - /// to the memory-mapped file.This parameter can't be set to Write. + /// to the memory-mapped file. + /// This parameter can't be set to /// One of the enumeration values that specifies whether a handle - /// to the memory-mapped file can be inherited by a child process. The default is None. + /// to the memory-mapped file can be inherited by a child process. The default is . /// A value that indicates whether to close the source file handle when /// the is disposed. /// A memory-mapped file that has the specified characteristics. /// /// is or an empty string. - /// -or- - /// and the length of the file are zero. - /// -or- - /// is set to Write or Write enumeration value, which is not allowed. - /// -or- - /// access is set to Read and is larger than the length of the file. + /// -or- + /// and the length of the file are zero. + /// -or- + /// is set to , which is not allowed. + /// -or- + /// is set to and is larger than the length of the file. /// /// is . /// - /// is less than zero. - /// -or- - /// is less than the file size. - /// -or- - /// is not a valid enumeration value. - /// -or- - /// is not a valid enumeration value. + /// is less than zero. + /// -or- + /// is less than the file size. + /// -or- + /// is not a valid enumeration value. + /// -or- + /// is not a valid enumeration value. /// public static MemoryMappedFile CreateFromFile(SafeFileHandle fileHandle, string? mapName, long capacity, MemoryMappedFileAccess access, From e582381a16230cc3471e5c30cf0abc16956c59ca Mon Sep 17 00:00:00 2001 From: Amal Abeygunawardana Date: Wed, 22 Mar 2023 21:07:01 +1100 Subject: [PATCH 12/13] Fix copy paste error --- .../tests/MemoryMappedFile.CreateFromFile.Tests.cs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/libraries/System.IO.MemoryMappedFiles/tests/MemoryMappedFile.CreateFromFile.Tests.cs b/src/libraries/System.IO.MemoryMappedFiles/tests/MemoryMappedFile.CreateFromFile.Tests.cs index ee99a2d04f94ef..902aa863c64573 100644 --- a/src/libraries/System.IO.MemoryMappedFiles/tests/MemoryMappedFile.CreateFromFile.Tests.cs +++ b/src/libraries/System.IO.MemoryMappedFiles/tests/MemoryMappedFile.CreateFromFile.Tests.cs @@ -45,7 +45,7 @@ public void InvalidArguments_FileStream() [Fact] public void InvalidArguments_SafeFileHandle() { - // null is an invalid stream + // null is an invalid handle AssertExtensions.Throws("fileHandle", () => MemoryMappedFile.CreateFromFile((SafeFileHandle)null, CreateUniqueMapName(), 4096, MemoryMappedFileAccess.Read, HandleInheritability.None, true)); } From a7c653ec806e1dacdc0bee32654fd71eecb1247e Mon Sep 17 00:00:00 2001 From: Amal Abeygunawardana Date: Wed, 22 Mar 2023 23:29:03 +1100 Subject: [PATCH 13/13] Use fileHandle from File.OpenHandle --- .../MemoryMappedFile.CreateFromFile.Tests.cs | 71 ++++++++++--------- 1 file changed, 38 insertions(+), 33 deletions(-) diff --git a/src/libraries/System.IO.MemoryMappedFiles/tests/MemoryMappedFile.CreateFromFile.Tests.cs b/src/libraries/System.IO.MemoryMappedFiles/tests/MemoryMappedFile.CreateFromFile.Tests.cs index 902aa863c64573..741d56f5c6d5d7 100644 --- a/src/libraries/System.IO.MemoryMappedFiles/tests/MemoryMappedFile.CreateFromFile.Tests.cs +++ b/src/libraries/System.IO.MemoryMappedFiles/tests/MemoryMappedFile.CreateFromFile.Tests.cs @@ -116,9 +116,8 @@ public void InvalidArguments_Access() // Test the same things, but with a SafeFileHandle using (TempFile file = new TempFile(GetTestFilePath())) - using (FileStream fs = File.Open(file.Path, FileMode.Open)) + using (SafeFileHandle fileHandle = File.OpenHandle(file.Path, FileMode.Open, FileAccess.ReadWrite)) { - SafeFileHandle fileHandle = fs.SafeFileHandle; // Out of range values with a fileHandle AssertExtensions.Throws("access", () => MemoryMappedFile.CreateFromFile(fileHandle, CreateUniqueMapName(), 4096, (MemoryMappedFileAccess)(-2), HandleInheritability.None, true)); AssertExtensions.Throws("access", () => MemoryMappedFile.CreateFromFile(fileHandle, CreateUniqueMapName(), 4096, (MemoryMappedFileAccess)(42), HandleInheritability.None, true)); @@ -163,8 +162,8 @@ public void FileAccessAndMapAccessCombinationsWithSafeFileHandle_Valid(FileAcces { const int Capacity = 4096; using (TempFile file = new TempFile(GetTestFilePath(), Capacity)) - using (FileStream fs = new FileStream(file.Path, FileMode.Open, fileAccess)) - using (MemoryMappedFile mmf = MemoryMappedFile.CreateFromFile(fs.SafeFileHandle, null, Capacity, mmfAccess, HandleInheritability.None, true)) + using (SafeFileHandle fileHandle = File.OpenHandle(file.Path, FileMode.Open, fileAccess)) + using (MemoryMappedFile mmf = MemoryMappedFile.CreateFromFile(fileHandle, null, Capacity, mmfAccess, HandleInheritability.None, true)) { ValidateMemoryMappedFile(mmf, Capacity, mmfAccess); } @@ -240,7 +239,11 @@ public void InvalidArguments_MapName() using (FileStream fs = File.Open(file.Path, FileMode.Open)) { AssertExtensions.Throws(null, () => MemoryMappedFile.CreateFromFile(fs, string.Empty, 4096, MemoryMappedFileAccess.ReadWrite, HandleInheritability.None, true)); - AssertExtensions.Throws(null, () => MemoryMappedFile.CreateFromFile(fs.SafeFileHandle, string.Empty, 4096, MemoryMappedFileAccess.ReadWrite, HandleInheritability.None, true)); + } + + using (SafeFileHandle fileHandle = File.OpenHandle(file.Path, FileMode.Open, FileAccess.ReadWrite)) + { + AssertExtensions.Throws(null, () => MemoryMappedFile.CreateFromFile(fileHandle, string.Empty, 4096, MemoryMappedFileAccess.ReadWrite, HandleInheritability.None, true)); } } } @@ -323,9 +326,8 @@ public void InvalidArguments_CapacityWithFileHandle() { using (TempFile file = new TempFile(GetTestFilePath())) { - using (FileStream fs = File.Open(file.Path, FileMode.Open)) + using (SafeFileHandle fileHandle = File.OpenHandle(file.Path, FileMode.Open, FileAccess.ReadWrite)) { - SafeFileHandle fileHandle = fs.SafeFileHandle; // Out of range values for capacity Assert.Throws(() => MemoryMappedFile.CreateFromFile(fileHandle, null, -1, MemoryMappedFileAccess.Read, HandleInheritability.None, true)); @@ -334,7 +336,7 @@ public void InvalidArguments_CapacityWithFileHandle() AssertExtensions.Throws(null, () => MemoryMappedFile.CreateFromFile(fileHandle, CreateUniqueMapName(), 0, MemoryMappedFileAccess.Read, HandleInheritability.None, true)); // Larger capacity than the underlying file, but read-only such that we can't expand the file - fs.SetLength(4096); + RandomAccess.SetLength(fileHandle, 4096); AssertExtensions.Throws(null, () => MemoryMappedFile.CreateFromFile(fileHandle, null, 8192, MemoryMappedFileAccess.Read, HandleInheritability.None, true)); AssertExtensions.Throws(null, () => MemoryMappedFile.CreateFromFile(fileHandle, CreateUniqueMapName(), 8192, MemoryMappedFileAccess.Read, HandleInheritability.None, true)); @@ -354,11 +356,17 @@ public void InvalidArguments_Inheritability(HandleInheritability inheritability) { // Out of range values for inheritability using (TempFile file = new TempFile(GetTestFilePath())) - using (FileStream fs = File.Open(file.Path, FileMode.Open)) { - AssertExtensions.Throws("inheritability", () => MemoryMappedFile.CreateFromFile(fs, CreateUniqueMapName(), 4096, MemoryMappedFileAccess.ReadWrite, inheritability, true)); - AssertExtensions.Throws("inheritability", () => MemoryMappedFile.CreateFromFile(fs.SafeFileHandle, CreateUniqueMapName(), 4096, MemoryMappedFileAccess.ReadWrite, inheritability, true)); - } + using (FileStream fs = File.Open(file.Path, FileMode.Open)) + { + AssertExtensions.Throws("inheritability", () => MemoryMappedFile.CreateFromFile(fs, CreateUniqueMapName(), 4096, MemoryMappedFileAccess.ReadWrite, inheritability, true)); + } + + using (SafeFileHandle fileHandle = File.OpenHandle(file.Path, FileMode.Open, FileAccess.ReadWrite)) + { + AssertExtensions.Throws("inheritability", () => MemoryMappedFile.CreateFromFile(fileHandle, CreateUniqueMapName(), 4096, MemoryMappedFileAccess.ReadWrite, inheritability, true)); + } + } } /// @@ -540,8 +548,8 @@ public void ValidArgumentCombinationsWithHandle( { // Create a file of the right size, then create the map for it. using (TempFile file = new TempFile(GetTestFilePath(), capacity)) - using (FileStream fs = File.Open(file.Path, FileMode.Open)) - using (MemoryMappedFile mmf = MemoryMappedFile.CreateFromFile(fs.SafeFileHandle, mapName, capacity, access, inheritability, leaveOpen)) + using (SafeFileHandle fileHandle = File.OpenHandle(file.Path, FileMode.Open, FileAccess.ReadWrite)) + using (MemoryMappedFile mmf = MemoryMappedFile.CreateFromFile(fileHandle, mapName, capacity, access, inheritability, leaveOpen)) { ValidateMemoryMappedFile(mmf, capacity, access, inheritability); } @@ -549,8 +557,8 @@ public void ValidArgumentCombinationsWithHandle( // Start with an empty file and let the map grow it to the right size. This requires write access. if (IsWritable(access)) { - using (FileStream fs = File.Create(GetTestFilePath())) - using (MemoryMappedFile mmf = MemoryMappedFile.CreateFromFile(fs.SafeFileHandle, mapName, capacity, access, inheritability, leaveOpen)) + using (SafeFileHandle fileHandle = File.OpenHandle(GetTestFilePath(), FileMode.Create, FileAccess.ReadWrite)) + using (MemoryMappedFile mmf = MemoryMappedFile.CreateFromFile(fileHandle, mapName, capacity, access, inheritability, leaveOpen)) { ValidateMemoryMappedFile(mmf, capacity, access, inheritability); } @@ -664,8 +672,8 @@ public void DefaultCapacityIsFileLength() // With file handle using (TempFile file = new TempFile(GetTestFilePath(), DesiredCapacity)) - using (FileStream fs = File.Open(file.Path, FileMode.Open)) - using (MemoryMappedFile mmf = MemoryMappedFile.CreateFromFile(fs.SafeFileHandle, null, DefaultCapacity, MemoryMappedFileAccess.ReadWrite, HandleInheritability.None, true)) + using (SafeFileHandle fileHandle = File.OpenHandle(file.Path, FileMode.Open, FileAccess.ReadWrite)) + using (MemoryMappedFile mmf = MemoryMappedFile.CreateFromFile(fileHandle, null, DefaultCapacity, MemoryMappedFileAccess.ReadWrite, HandleInheritability.None, true)) { ValidateMemoryMappedFile(mmf, DesiredCapacity); } @@ -859,17 +867,16 @@ public void LeaveOpenRespectedWithSafeFileHandle_Basic(bool leaveOpen) const int Capacity = 4096; using (TempFile file = new TempFile(GetTestFilePath())) - using (FileStream fs = File.Open(file.Path, FileMode.Open)) + using (SafeFileHandle fileHandle = File.OpenHandle(file.Path, FileMode.Open, FileAccess.ReadWrite)) { // Handle should still be open - SafeFileHandle handle = fs.SafeFileHandle; - Assert.False(handle.IsClosed); + Assert.False(fileHandle.IsClosed); // Create and close the map - MemoryMappedFile.CreateFromFile(handle, null, Capacity, MemoryMappedFileAccess.ReadWrite, HandleInheritability.None, leaveOpen).Dispose(); + MemoryMappedFile.CreateFromFile(fileHandle, null, Capacity, MemoryMappedFileAccess.ReadWrite, HandleInheritability.None, leaveOpen).Dispose(); // The handle should now be open if leaveOpen - Assert.NotEqual(leaveOpen, handle.IsClosed); + Assert.NotEqual(leaveOpen, fileHandle.IsClosed); } } @@ -918,20 +925,19 @@ public void LeaveOpenRespectedWithSafeFileHandle_OutstandingViews(bool leaveOpen { const int Capacity = 4096; using (TempFile file = new TempFile(GetTestFilePath())) - using (FileStream fs = File.Open(file.Path, FileMode.Open)) + using (SafeFileHandle fileHandle = File.OpenHandle(file.Path, FileMode.Open, FileAccess.ReadWrite)) { // Handle should still be open - SafeFileHandle handle = fs.SafeFileHandle; - Assert.False(handle.IsClosed); + Assert.False(fileHandle.IsClosed); // Create the map, create each of the views, then close the map - using (MemoryMappedFile mmf = MemoryMappedFile.CreateFromFile(handle, null, Capacity, MemoryMappedFileAccess.ReadWrite, HandleInheritability.None, leaveOpen)) + using (MemoryMappedFile mmf = MemoryMappedFile.CreateFromFile(fileHandle, null, Capacity, MemoryMappedFileAccess.ReadWrite, HandleInheritability.None, leaveOpen)) using (MemoryMappedViewAccessor acc = mmf.CreateViewAccessor(0, Capacity)) using (MemoryMappedViewStream s = mmf.CreateViewStream(0, Capacity)) { // Explicitly close the map. The handle should now be open iff leaveOpen. mmf.Dispose(); - Assert.NotEqual(leaveOpen, handle.IsClosed); + Assert.NotEqual(leaveOpen, fileHandle.IsClosed); // But the views should still be usable. ValidateMemoryMappedViewAccessor(acc, Capacity, MemoryMappedFileAccess.ReadWrite); @@ -986,9 +992,9 @@ public void MultipleMapsForTheSameSafeFileHandle() { const int Capacity = 4096; using (TempFile file = new TempFile(GetTestFilePath(), Capacity)) - using (FileStream fs = new FileStream(file.Path, FileMode.Open)) - using (MemoryMappedFile mmf1 = MemoryMappedFile.CreateFromFile(fs.SafeFileHandle, null, Capacity, MemoryMappedFileAccess.ReadWrite, HandleInheritability.None, true)) - using (MemoryMappedFile mmf2 = MemoryMappedFile.CreateFromFile(fs.SafeFileHandle, null, Capacity, MemoryMappedFileAccess.ReadWrite, HandleInheritability.None, true)) + using (SafeFileHandle fileHandle = File.OpenHandle(file.Path, FileMode.Open, FileAccess.ReadWrite)) + using (MemoryMappedFile mmf1 = MemoryMappedFile.CreateFromFile(fileHandle, null, Capacity, MemoryMappedFileAccess.ReadWrite, HandleInheritability.None, true)) + using (MemoryMappedFile mmf2 = MemoryMappedFile.CreateFromFile(fileHandle, null, Capacity, MemoryMappedFileAccess.ReadWrite, HandleInheritability.None, true)) using (MemoryMappedViewAccessor acc1 = mmf1.CreateViewAccessor()) using (MemoryMappedViewAccessor acc2 = mmf2.CreateViewAccessor()) { @@ -1184,9 +1190,8 @@ public void MapHandleMatchesFileStreamHandle() [Fact] public void MapHandleMatchesSafeFileHandle() { - using (FileStream fs = File.OpenWrite(GetTestFilePath())) + using (SafeFileHandle fileHandle = File.OpenHandle(GetTestFilePath(), FileMode.OpenOrCreate, FileAccess.ReadWrite)) { - SafeFileHandle fileHandle = fs.SafeFileHandle; using (MemoryMappedFile mmf = MemoryMappedFile.CreateFromFile(fileHandle, null, 4096, MemoryMappedFileAccess.ReadWrite, HandleInheritability.None, false)) { SafeMemoryMappedFileHandle handle = mmf.SafeMemoryMappedFileHandle;