From 02f7f4f0908bdbc8dcee61b7a1de28e6cb837398 Mon Sep 17 00:00:00 2001 From: Adam Sitnik Date: Thu, 14 Nov 2024 13:51:22 +0100 Subject: [PATCH 01/13] macOS does not define IOV_MAX, it uses UIO_MAXIOV instead --- src/native/libs/System.Native/pal_io.c | 13 ++++++++++--- 1 file changed, 10 insertions(+), 3 deletions(-) diff --git a/src/native/libs/System.Native/pal_io.c b/src/native/libs/System.Native/pal_io.c index 4e237caa21145..a49b286fe1e8b 100644 --- a/src/native/libs/System.Native/pal_io.c +++ b/src/native/libs/System.Native/pal_io.c @@ -1954,15 +1954,22 @@ static int GetAllowedVectorCount(int32_t vectorCount) { int allowedCount = (int)vectorCount; + // We need to respect the limit of items that can be passed in iov. + // In case of writes, the managed code is responsible for handling incomplete writes. + // In case of reads, we simply returns the number of bytes read and it's up to the users. #if defined(IOV_MAX) if (IOV_MAX < allowedCount) { - // We need to respect the limit of items that can be passed in iov. - // In case of writes, the managed code is reponsible for handling incomplete writes. - // In case of reads, we simply returns the number of bytes read and it's up to the users. allowedCount = IOV_MAX; } +#elif defined(UIO_MAXIOV) // macOS and iOS + if (UIO_MAXIOV < allowedCount) + { + allowedCount = UIO_MAXIOV; + } #endif + + return allowedCount; } #endif // (HAVE_PREADV || HAVE_PWRITEV) && !defined(TARGET_WASM) From 93c6b8c3447354224b94ca508d14b650803f0284 Mon Sep 17 00:00:00 2001 From: Adam Sitnik Date: Thu, 14 Nov 2024 13:57:34 +0100 Subject: [PATCH 02/13] don't run these tests in parallel, as each test cases uses more than 4 GB ram and disk! --- .../System.IO.FileSystem.Tests/RandomAccess/WriteGatherAsync.cs | 1 + 1 file changed, 1 insertion(+) diff --git a/src/libraries/System.Runtime/tests/System.IO.FileSystem.Tests/RandomAccess/WriteGatherAsync.cs b/src/libraries/System.Runtime/tests/System.IO.FileSystem.Tests/RandomAccess/WriteGatherAsync.cs index 843cf942f42c7..a8edc71d46af8 100644 --- a/src/libraries/System.Runtime/tests/System.IO.FileSystem.Tests/RandomAccess/WriteGatherAsync.cs +++ b/src/libraries/System.Runtime/tests/System.IO.FileSystem.Tests/RandomAccess/WriteGatherAsync.cs @@ -13,6 +13,7 @@ namespace System.IO.Tests { [SkipOnPlatform(TestPlatforms.Browser, "async file IO is not supported on browser")] + [Collection(nameof(DisableParallelization))] // don't run in parallel, as some of these tests use a LOT of resources public class RandomAccess_WriteGatherAsync : RandomAccess_Base { protected override ValueTask MethodUnderTest(SafeFileHandle handle, byte[] bytes, long fileOffset) From e7a288cd830218b6533ccbdfc1518316b547c63f Mon Sep 17 00:00:00 2001 From: Adam Sitnik Date: Thu, 14 Nov 2024 13:58:37 +0100 Subject: [PATCH 03/13] fix the test: handle incomplete reads that should happen when we hit the max buffer limit --- .../RandomAccess/WriteGatherAsync.cs | 51 +++++++++++++------ 1 file changed, 36 insertions(+), 15 deletions(-) diff --git a/src/libraries/System.Runtime/tests/System.IO.FileSystem.Tests/RandomAccess/WriteGatherAsync.cs b/src/libraries/System.Runtime/tests/System.IO.FileSystem.Tests/RandomAccess/WriteGatherAsync.cs index a8edc71d46af8..38229dcc4d978 100644 --- a/src/libraries/System.Runtime/tests/System.IO.FileSystem.Tests/RandomAccess/WriteGatherAsync.cs +++ b/src/libraries/System.Runtime/tests/System.IO.FileSystem.Tests/RandomAccess/WriteGatherAsync.cs @@ -181,29 +181,50 @@ public async Task NoInt32OverflowForLargeInputs(bool asyncFile, bool asyncMethod throw new SkipTestException("Not enough disk space."); } - long fileOffset = 0, bytesRead = 0; - try + using (sfh) { if (asyncMethod) { - await RandomAccess.WriteAsync(sfh, writeBuffers, fileOffset); - bytesRead = await RandomAccess.ReadAsync(sfh, readBuffers, fileOffset); + await RandomAccess.WriteAsync(sfh, writeBuffers, 0); } else { - RandomAccess.Write(sfh, writeBuffers, fileOffset); - bytesRead = RandomAccess.Read(sfh, readBuffers, fileOffset); + RandomAccess.Write(sfh, writeBuffers, 0); } - } - finally - { - sfh.Dispose(); // delete the file ASAP to avoid running out of resources in CI - } - Assert.Equal(FileSize, bytesRead); - for (int i = 0; i < BufferCount; i++) - { - Assert.Equal(writeBuffer, readBuffers[i]); + Assert.Equal(FileSize, RandomAccess.GetLength(sfh)); + + long fileOffset = 0; + while (fileOffset < FileSize) + { + long bytesRead = asyncMethod + ? await RandomAccess.ReadAsync(sfh, readBuffers, fileOffset) + : RandomAccess.Read(sfh, readBuffers, fileOffset); + + Assert.InRange(bytesRead, 0, FileSize); + + while (bytesRead > 0) + { + Memory readBuffer = readBuffers[0]; + if (bytesRead >= readBuffer.Length) + { + AssertExtensions.SequenceEqual(writeBuffer.Span, readBuffer.Span); + + bytesRead -= readBuffer.Length; + fileOffset += readBuffer.Length; + + readBuffers.RemoveAt(0); + } + else + { + // A read has finished somewhere in the middle of one of the read buffers. + // Example: buffer had 30 bytes and only 10 were read. + // We don't read the missing part, but try to read the whole buffer again. + // It's not optimal from performance perspective, but it keeps the test logic simple. + break; + } + } + } } } From cb37a355c31e6332c3606202a72b0c9263a63d18 Mon Sep 17 00:00:00 2001 From: Adam Sitnik Date: Thu, 14 Nov 2024 15:10:49 +0100 Subject: [PATCH 04/13] incomplete write fix: - pin the buffers only once - when re-trying, do that only for the actual reminder --- .../src/System/IO/RandomAccess.Unix.cs | 61 +++++++++---------- 1 file changed, 30 insertions(+), 31 deletions(-) diff --git a/src/libraries/System.Private.CoreLib/src/System/IO/RandomAccess.Unix.cs b/src/libraries/System.Private.CoreLib/src/System/IO/RandomAccess.Unix.cs index bcddd3f4654fc..76742c72cccb9 100644 --- a/src/libraries/System.Private.CoreLib/src/System/IO/RandomAccess.Unix.cs +++ b/src/libraries/System.Private.CoreLib/src/System/IO/RandomAccess.Unix.cs @@ -171,35 +171,27 @@ internal static unsafe void WriteGatherAtOffset(SafeFileHandle handle, IReadOnly stackalloc Interop.Sys.IOVector[IovStackThreshold] : new Interop.Sys.IOVector[buffersCount]; - try + long totalBytesToWrite = 0; + for (int i = 0; i < buffersCount; i++) { - int buffersOffset = 0, firstBufferOffset = 0; - while (true) - { - long totalBytesToWrite = 0; - - for (int i = buffersOffset; i < buffersCount; i++) - { - ReadOnlyMemory buffer = buffers[i]; - totalBytesToWrite += buffer.Length; - - MemoryHandle memoryHandle = buffer.Pin(); - vectors[i] = new Interop.Sys.IOVector { Base = firstBufferOffset + (byte*)memoryHandle.Pointer, Count = (UIntPtr)(buffer.Length - firstBufferOffset) }; - handles[i] = memoryHandle; + ReadOnlyMemory buffer = buffers[i]; + totalBytesToWrite += buffer.Length; - firstBufferOffset = 0; - } - - if (totalBytesToWrite == 0) - { - break; - } + MemoryHandle memoryHandle = buffer.Pin(); + vectors[i] = new Interop.Sys.IOVector { Base = (byte*)memoryHandle.Pointer, Count = (UIntPtr)buffer.Length }; + handles[i] = memoryHandle; + } + try + { + int buffersOffset = 0; + while (totalBytesToWrite > 0) + { long bytesWritten; - Span left = vectors.Slice(buffersOffset); + Span left = vectors.Slice(buffersOffset, buffersCount - buffersOffset); fixed (Interop.Sys.IOVector* pinnedVectors = &MemoryMarshal.GetReference(left)) { - bytesWritten = Interop.Sys.PWriteV(handle, pinnedVectors, buffersCount - buffersOffset, fileOffset); + bytesWritten = Interop.Sys.PWriteV(handle, pinnedVectors, left.Length, fileOffset); } FileStreamHelpers.CheckFileCall(bytesWritten, handle.Path); @@ -211,22 +203,29 @@ internal static unsafe void WriteGatherAtOffset(SafeFileHandle handle, IReadOnly // The write completed successfully but for fewer bytes than requested. // We need to perform next write where the previous one has finished. fileOffset += bytesWritten; + totalBytesToWrite -= bytesWritten; // We need to try again for the remainder. - for (int i = 0; i < buffersCount; i++) + while (buffersOffset < buffersCount && bytesWritten > 0) { - int n = buffers[i].Length; + int n = buffers[buffersOffset].Length; if (n <= bytesWritten) { - buffersOffset++; bytesWritten -= n; - if (bytesWritten == 0) - { - break; - } + buffersOffset++; } else { - firstBufferOffset = (int)(bytesWritten - n); + // A partial read: the vector needs to point to the new offset. + // But that offset needs to be relative to the previous attempt. + // Example: we have a single buffer with 30 bytes and the first read returned 10. + // The next read should try to read the remaining 20 bytes, but in case it also reads just 10, + // the third attempt should read last 10 bytes (not 20 again). + Interop.Sys.IOVector current = vectors[buffersOffset]; + vectors[buffersOffset] = new Interop.Sys.IOVector + { + Base = current.Base + (int)(bytesWritten), + Count = current.Count - (UIntPtr)(bytesWritten) + }; break; } } From 00d209a5f7394dc015ee316fc70547846c14a44b Mon Sep 17 00:00:00 2001 From: Adam Sitnik Date: Fri, 15 Nov 2024 11:36:31 +0100 Subject: [PATCH 05/13] try one more thing: IOV_MAX should be available everywhere --- src/native/libs/System.Native/pal_io.c | 8 -------- 1 file changed, 8 deletions(-) diff --git a/src/native/libs/System.Native/pal_io.c b/src/native/libs/System.Native/pal_io.c index a49b286fe1e8b..05f46d3201d58 100644 --- a/src/native/libs/System.Native/pal_io.c +++ b/src/native/libs/System.Native/pal_io.c @@ -1957,18 +1957,10 @@ static int GetAllowedVectorCount(int32_t vectorCount) // We need to respect the limit of items that can be passed in iov. // In case of writes, the managed code is responsible for handling incomplete writes. // In case of reads, we simply returns the number of bytes read and it's up to the users. -#if defined(IOV_MAX) if (IOV_MAX < allowedCount) { allowedCount = IOV_MAX; } -#elif defined(UIO_MAXIOV) // macOS and iOS - if (UIO_MAXIOV < allowedCount) - { - allowedCount = UIO_MAXIOV; - } -#endif - return allowedCount; } From 31a3a0087ddfe2eb0640c20217fb7e3fdc7ddca6 Mon Sep 17 00:00:00 2001 From: Adam Sitnik Date: Mon, 18 Nov 2024 14:03:01 +0100 Subject: [PATCH 06/13] I can't reproduce it locally, so here we go... --- .../src/System/IO/RandomAccess.Unix.cs | 7 +++++++ src/native/libs/System.Native/pal_io.c | 10 +++++++++- 2 files changed, 16 insertions(+), 1 deletion(-) diff --git a/src/libraries/System.Private.CoreLib/src/System/IO/RandomAccess.Unix.cs b/src/libraries/System.Private.CoreLib/src/System/IO/RandomAccess.Unix.cs index 76742c72cccb9..e467e49997afc 100644 --- a/src/libraries/System.Private.CoreLib/src/System/IO/RandomAccess.Unix.cs +++ b/src/libraries/System.Private.CoreLib/src/System/IO/RandomAccess.Unix.cs @@ -194,6 +194,13 @@ internal static unsafe void WriteGatherAtOffset(SafeFileHandle handle, IReadOnly bytesWritten = Interop.Sys.PWriteV(handle, pinnedVectors, left.Length, fileOffset); } +#if TARGET_OSX && TARGET_ARM64 + if (bytesWritten < 0) + { + throw new Exception($"PWriteV has failed and IOV_MAX was {bytesWritten}."); + } +#endif + FileStreamHelpers.CheckFileCall(bytesWritten, handle.Path); if (bytesWritten == totalBytesToWrite) { diff --git a/src/native/libs/System.Native/pal_io.c b/src/native/libs/System.Native/pal_io.c index 05f46d3201d58..2703d4dd81e26 100644 --- a/src/native/libs/System.Native/pal_io.c +++ b/src/native/libs/System.Native/pal_io.c @@ -2017,6 +2017,14 @@ int64_t SystemNative_PWriteV(intptr_t fd, IOVector* vectors, int32_t vectorCount #if HAVE_PWRITEV && !defined(TARGET_WASM) // pwritev is buggy on WASM int allowedVectorCount = GetAllowedVectorCount(vectorCount); while ((count = pwritev(fileDescriptor, (struct iovec*)vectors, allowedVectorCount, (off_t)fileOffset)) < 0 && errno == EINTR); + +#if defined(TARGET_APPLE) && (defined(__arm__) || defined(__aarch64__)) + if (count < 0 && errno == EINVAL) + { + count = -1 * IOV_MAX; + } +#endif + #else int64_t current; for (int i = 0; i < vectorCount; i++) @@ -2044,6 +2052,6 @@ int64_t SystemNative_PWriteV(intptr_t fd, IOVector* vectors, int32_t vectorCount } #endif - assert(count >= -1); + // assert(count >= -1); return count; } From 2de6b8c6bc4765cefe0b3acf7951ee9d496ccea3 Mon Sep 17 00:00:00 2001 From: Adam Sitnik Date: Mon, 18 Nov 2024 15:55:34 +0100 Subject: [PATCH 07/13] IOV_MAX is reported as 1024, but it fails with EINVAL. What value is going to work? --- .../src/System/IO/RandomAccess.Unix.cs | 2 +- src/native/libs/System.Native/pal_io.c | 8 +++++++- 2 files changed, 8 insertions(+), 2 deletions(-) diff --git a/src/libraries/System.Private.CoreLib/src/System/IO/RandomAccess.Unix.cs b/src/libraries/System.Private.CoreLib/src/System/IO/RandomAccess.Unix.cs index e467e49997afc..b50594de62955 100644 --- a/src/libraries/System.Private.CoreLib/src/System/IO/RandomAccess.Unix.cs +++ b/src/libraries/System.Private.CoreLib/src/System/IO/RandomAccess.Unix.cs @@ -197,7 +197,7 @@ internal static unsafe void WriteGatherAtOffset(SafeFileHandle handle, IReadOnly #if TARGET_OSX && TARGET_ARM64 if (bytesWritten < 0) { - throw new Exception($"PWriteV has failed and IOV_MAX was {bytesWritten}."); + throw new Exception($"PWriteV succeeded for {bytesWritten} vectors."); } #endif diff --git a/src/native/libs/System.Native/pal_io.c b/src/native/libs/System.Native/pal_io.c index 2703d4dd81e26..8bbdd32fb5b07 100644 --- a/src/native/libs/System.Native/pal_io.c +++ b/src/native/libs/System.Native/pal_io.c @@ -2021,7 +2021,13 @@ int64_t SystemNative_PWriteV(intptr_t fd, IOVector* vectors, int32_t vectorCount #if defined(TARGET_APPLE) && (defined(__arm__) || defined(__aarch64__)) if (count < 0 && errno == EINVAL) { - count = -1 * IOV_MAX; + do + { + allowedVectorCount = allowedVectorCount / 2; + } + while ((count = pwritev(fileDescriptor, (struct iovec*)vectors, allowedVectorCount, (off_t)fileOffset)) < 0 && errno == EINVAL); + + count = -1 * allowedVectorCount; } #endif From 6a53ac430753b4a0d966c43459a4dd07fc2fa48f Mon Sep 17 00:00:00 2001 From: Adam Sitnik Date: Mon, 18 Nov 2024 18:49:23 +0100 Subject: [PATCH 08/13] new approach: prefer sysconf(_SC_IOV_MAX) over other options --- .../src/System/IO/RandomAccess.Unix.cs | 7 ---- src/native/libs/System.Native/pal_io.c | 36 ++++++++++--------- 2 files changed, 20 insertions(+), 23 deletions(-) diff --git a/src/libraries/System.Private.CoreLib/src/System/IO/RandomAccess.Unix.cs b/src/libraries/System.Private.CoreLib/src/System/IO/RandomAccess.Unix.cs index b50594de62955..76742c72cccb9 100644 --- a/src/libraries/System.Private.CoreLib/src/System/IO/RandomAccess.Unix.cs +++ b/src/libraries/System.Private.CoreLib/src/System/IO/RandomAccess.Unix.cs @@ -194,13 +194,6 @@ internal static unsafe void WriteGatherAtOffset(SafeFileHandle handle, IReadOnly bytesWritten = Interop.Sys.PWriteV(handle, pinnedVectors, left.Length, fileOffset); } -#if TARGET_OSX && TARGET_ARM64 - if (bytesWritten < 0) - { - throw new Exception($"PWriteV succeeded for {bytesWritten} vectors."); - } -#endif - FileStreamHelpers.CheckFileCall(bytesWritten, handle.Path); if (bytesWritten == totalBytesToWrite) { diff --git a/src/native/libs/System.Native/pal_io.c b/src/native/libs/System.Native/pal_io.c index 8bbdd32fb5b07..b1c6504b00dcc 100644 --- a/src/native/libs/System.Native/pal_io.c +++ b/src/native/libs/System.Native/pal_io.c @@ -1952,14 +1952,32 @@ int32_t SystemNative_PWrite(intptr_t fd, void* buffer, int32_t bufferSize, int64 #if (HAVE_PREADV || HAVE_PWRITEV) && !defined(TARGET_WASM) static int GetAllowedVectorCount(int32_t vectorCount) { + static volatile int s_iovMax = 0; + + int iovMax = s_iovMax; + if (iovMax == 0) + { +#if defined(_SC_IOV_MAX) + // For macOS arm64 the IOV_MAX reports 1024, but fails with EINVAL for such inputs. + // That is why we prefer _SC_IOV_MAX over IOV_MAX. + iovMax = sysconf(_SC_IOV_MAX); +#elif defined(IOV_MAX) + iovMax = IOV_MAX; +#else + // 16 is low, but supported on every platform. + iovMax = 16; +#endif + s_iovMax = iovMax; + } + int allowedCount = (int)vectorCount; // We need to respect the limit of items that can be passed in iov. // In case of writes, the managed code is responsible for handling incomplete writes. // In case of reads, we simply returns the number of bytes read and it's up to the users. - if (IOV_MAX < allowedCount) + if (iovMax < allowedCount) { - allowedCount = IOV_MAX; + allowedCount = iovMax; } return allowedCount; @@ -2017,20 +2035,6 @@ int64_t SystemNative_PWriteV(intptr_t fd, IOVector* vectors, int32_t vectorCount #if HAVE_PWRITEV && !defined(TARGET_WASM) // pwritev is buggy on WASM int allowedVectorCount = GetAllowedVectorCount(vectorCount); while ((count = pwritev(fileDescriptor, (struct iovec*)vectors, allowedVectorCount, (off_t)fileOffset)) < 0 && errno == EINTR); - -#if defined(TARGET_APPLE) && (defined(__arm__) || defined(__aarch64__)) - if (count < 0 && errno == EINVAL) - { - do - { - allowedVectorCount = allowedVectorCount / 2; - } - while ((count = pwritev(fileDescriptor, (struct iovec*)vectors, allowedVectorCount, (off_t)fileOffset)) < 0 && errno == EINVAL); - - count = -1 * allowedVectorCount; - } -#endif - #else int64_t current; for (int i = 0; i < vectorCount; i++) From e389425c2b23dfeaed97e2219d5f9d990033720a Mon Sep 17 00:00:00 2001 From: Adam Sitnik Date: Tue, 19 Nov 2024 09:20:41 +0100 Subject: [PATCH 09/13] Apply suggestions from code review MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Co-authored-by: Michał Petryka <35800402+MichalPetryka@users.noreply.github.com> --- src/native/libs/System.Native/pal_io.c | 14 +++++++------- 1 file changed, 7 insertions(+), 7 deletions(-) diff --git a/src/native/libs/System.Native/pal_io.c b/src/native/libs/System.Native/pal_io.c index b1c6504b00dcc..4cf6f59fb9028 100644 --- a/src/native/libs/System.Native/pal_io.c +++ b/src/native/libs/System.Native/pal_io.c @@ -1952,23 +1952,23 @@ int32_t SystemNative_PWrite(intptr_t fd, void* buffer, int32_t bufferSize, int64 #if (HAVE_PREADV || HAVE_PWRITEV) && !defined(TARGET_WASM) static int GetAllowedVectorCount(int32_t vectorCount) { +#if defined(_SC_IOV_MAX) static volatile int s_iovMax = 0; int iovMax = s_iovMax; if (iovMax == 0) { -#if defined(_SC_IOV_MAX) // For macOS arm64 the IOV_MAX reports 1024, but fails with EINVAL for such inputs. // That is why we prefer _SC_IOV_MAX over IOV_MAX. - iovMax = sysconf(_SC_IOV_MAX); + iovMax = (int)sysconf(_SC_IOV_MAX); + s_iovMax = iovMax; + } #elif defined(IOV_MAX) - iovMax = IOV_MAX; + int iovMax = IOV_MAX; #else - // 16 is low, but supported on every platform. - iovMax = 16; + // 16 is low, but supported on every platform. + int iovMax = 16; #endif - s_iovMax = iovMax; - } int allowedCount = (int)vectorCount; From 6e5c47a49d46394c5187180c925ec383cb7fdb9f Mon Sep 17 00:00:00 2001 From: Adam Sitnik Date: Tue, 19 Nov 2024 14:21:17 +0100 Subject: [PATCH 10/13] Use native memory to get OOM a soon as we run out of memory (hoping to avoid the process getting killed on Linux when OOM happens) --- .../RandomAccess/WriteGatherAsync.cs | 52 +++++++++++++------ 1 file changed, 37 insertions(+), 15 deletions(-) diff --git a/src/libraries/System.Runtime/tests/System.IO.FileSystem.Tests/RandomAccess/WriteGatherAsync.cs b/src/libraries/System.Runtime/tests/System.IO.FileSystem.Tests/RandomAccess/WriteGatherAsync.cs index 38229dcc4d978..8cf7d5233c6d3 100644 --- a/src/libraries/System.Runtime/tests/System.IO.FileSystem.Tests/RandomAccess/WriteGatherAsync.cs +++ b/src/libraries/System.Runtime/tests/System.IO.FileSystem.Tests/RandomAccess/WriteGatherAsync.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.Buffers; using System.Collections.Generic; using System.Linq; using System.Security.Cryptography; @@ -152,21 +153,6 @@ public async Task NoInt32OverflowForLargeInputs(bool asyncFile, bool asyncMethod const int BufferSize = int.MaxValue / 1000; const long FileSize = (long)BufferCount * BufferSize; string filePath = GetTestFilePath(); - ReadOnlyMemory writeBuffer = RandomNumberGenerator.GetBytes(BufferSize); - List> writeBuffers = Enumerable.Repeat(writeBuffer, BufferCount).ToList(); - List> readBuffers = new List>(BufferCount); - - try - { - for (int i = 0; i < BufferCount; i++) - { - readBuffers.Add(new byte[BufferSize]); - } - } - catch (OutOfMemoryException) - { - throw new SkipTestException("Not enough memory."); - } FileOptions options = asyncFile ? FileOptions.Asynchronous : FileOptions.None; // we need to test both code paths options |= FileOptions.DeleteOnClose; @@ -182,6 +168,42 @@ public async Task NoInt32OverflowForLargeInputs(bool asyncFile, bool asyncMethod } using (sfh) + { + ReadOnlyMemory writeBuffer = RandomNumberGenerator.GetBytes(BufferSize); + List> writeBuffers = Enumerable.Repeat(writeBuffer, BufferCount).ToList(); + + List memoryManagers = new List(BufferCount); + List> readBuffers = new List>(BufferCount); + + try + { + try + { + for (int i = 0; i < BufferCount; i++) + { + // We are using native memory here to get OOM as soon as possible. + NativeMemoryManager nativeMemoryManager = new(BufferSize); + memoryManagers.Add(nativeMemoryManager); + readBuffers.Add(nativeMemoryManager.Memory); + } + } + catch (OutOfMemoryException) + { + throw new SkipTestException("Not enough memory."); + } + + await Verify(asyncMethod, FileSize, sfh, writeBuffer, writeBuffers, readBuffers); + } + finally + { + foreach (IDisposable memoryManager in memoryManagers) + { + memoryManager.Dispose(); + } + } + } + + static async Task Verify(bool asyncMethod, long FileSize, SafeFileHandle sfh, ReadOnlyMemory writeBuffer, List> writeBuffers, List> readBuffers) { if (asyncMethod) { From 87ee2e5c99420169826cf482637a7197da0b1a8f Mon Sep 17 00:00:00 2001 From: Adam Sitnik Date: Tue, 19 Nov 2024 15:04:33 +0100 Subject: [PATCH 11/13] For macOS preadv and pwritev can fail with EINVAL when the total length of all vectors overflows a 32-bit integer. --- src/native/libs/System.Native/pal_io.c | 50 +++++++++++++++----------- 1 file changed, 30 insertions(+), 20 deletions(-) diff --git a/src/native/libs/System.Native/pal_io.c b/src/native/libs/System.Native/pal_io.c index 4cf6f59fb9028..29db1be09b290 100644 --- a/src/native/libs/System.Native/pal_io.c +++ b/src/native/libs/System.Native/pal_io.c @@ -1950,24 +1950,16 @@ int32_t SystemNative_PWrite(intptr_t fd, void* buffer, int32_t bufferSize, int64 } #if (HAVE_PREADV || HAVE_PWRITEV) && !defined(TARGET_WASM) -static int GetAllowedVectorCount(int32_t vectorCount) +static int GetAllowedVectorCount(IOVector* vectors, int32_t vectorCount) { -#if defined(_SC_IOV_MAX) - static volatile int s_iovMax = 0; - - int iovMax = s_iovMax; - if (iovMax == 0) - { - // For macOS arm64 the IOV_MAX reports 1024, but fails with EINVAL for such inputs. - // That is why we prefer _SC_IOV_MAX over IOV_MAX. - iovMax = (int)sysconf(_SC_IOV_MAX); - s_iovMax = iovMax; - } -#elif defined(IOV_MAX) - int iovMax = IOV_MAX; +#if defined(IOV_MAX) + const int IovMax = IOV_MAX; #else + // In theory all the platforms that we support define IOV_MAX, + // but we want to be extra safe and provde a fallback + // in case it turns out to not be true. // 16 is low, but supported on every platform. - int iovMax = 16; + const int IovMax = 16; #endif int allowedCount = (int)vectorCount; @@ -1975,11 +1967,29 @@ static int GetAllowedVectorCount(int32_t vectorCount) // We need to respect the limit of items that can be passed in iov. // In case of writes, the managed code is responsible for handling incomplete writes. // In case of reads, we simply returns the number of bytes read and it's up to the users. - if (iovMax < allowedCount) + if (IovMax < allowedCount) { - allowedCount = iovMax; + allowedCount = IovMax; } +#if defined(TARGET_APPLE) + // For macOS preadv and pwritev can fail with EINVAL when the total length + // of all vectors overflows a 32-bit integer. + size_t totalLength = 0; + for (int i = 0; i < allowedCount; i++) + { + totalLength += vectors[i].Count; + + if (totalLength > INT_MAX && i > 0) + { + allowedCount = i; + break; + } + } +#else + (void)vectors; +#endif + return allowedCount; } #endif // (HAVE_PREADV || HAVE_PWRITEV) && !defined(TARGET_WASM) @@ -1992,7 +2002,7 @@ int64_t SystemNative_PReadV(intptr_t fd, IOVector* vectors, int32_t vectorCount, int64_t count = 0; int fileDescriptor = ToFileDescriptor(fd); #if HAVE_PREADV && !defined(TARGET_WASM) // preadv is buggy on WASM - int allowedVectorCount = GetAllowedVectorCount(vectorCount); + int allowedVectorCount = GetAllowedVectorCount(vectors, vectorCount); while ((count = preadv(fileDescriptor, (struct iovec*)vectors, allowedVectorCount, (off_t)fileOffset)) < 0 && errno == EINTR); #else int64_t current; @@ -2033,7 +2043,7 @@ int64_t SystemNative_PWriteV(intptr_t fd, IOVector* vectors, int32_t vectorCount int64_t count = 0; int fileDescriptor = ToFileDescriptor(fd); #if HAVE_PWRITEV && !defined(TARGET_WASM) // pwritev is buggy on WASM - int allowedVectorCount = GetAllowedVectorCount(vectorCount); + int allowedVectorCount = GetAllowedVectorCount(vectors, vectorCount); while ((count = pwritev(fileDescriptor, (struct iovec*)vectors, allowedVectorCount, (off_t)fileOffset)) < 0 && errno == EINTR); #else int64_t current; @@ -2062,6 +2072,6 @@ int64_t SystemNative_PWriteV(intptr_t fd, IOVector* vectors, int32_t vectorCount } #endif - // assert(count >= -1); + assert(count >= -1); return count; } From 3c11b65c9df14fc578cf8bcd6da75816333c3277 Mon Sep 17 00:00:00 2001 From: Adam Sitnik Date: Tue, 19 Nov 2024 15:32:51 +0100 Subject: [PATCH 12/13] move the code to finally block so when Pin throws, we dispose other handles --- .../src/System/IO/RandomAccess.Unix.cs | 20 +++++++++---------- 1 file changed, 10 insertions(+), 10 deletions(-) diff --git a/src/libraries/System.Private.CoreLib/src/System/IO/RandomAccess.Unix.cs b/src/libraries/System.Private.CoreLib/src/System/IO/RandomAccess.Unix.cs index 76742c72cccb9..e1b292140f976 100644 --- a/src/libraries/System.Private.CoreLib/src/System/IO/RandomAccess.Unix.cs +++ b/src/libraries/System.Private.CoreLib/src/System/IO/RandomAccess.Unix.cs @@ -171,19 +171,19 @@ internal static unsafe void WriteGatherAtOffset(SafeFileHandle handle, IReadOnly stackalloc Interop.Sys.IOVector[IovStackThreshold] : new Interop.Sys.IOVector[buffersCount]; - long totalBytesToWrite = 0; - for (int i = 0; i < buffersCount; i++) + try { - ReadOnlyMemory buffer = buffers[i]; - totalBytesToWrite += buffer.Length; + long totalBytesToWrite = 0; + for (int i = 0; i < buffersCount; i++) + { + ReadOnlyMemory buffer = buffers[i]; + totalBytesToWrite += buffer.Length; - MemoryHandle memoryHandle = buffer.Pin(); - vectors[i] = new Interop.Sys.IOVector { Base = (byte*)memoryHandle.Pointer, Count = (UIntPtr)buffer.Length }; - handles[i] = memoryHandle; - } + MemoryHandle memoryHandle = buffer.Pin(); + vectors[i] = new Interop.Sys.IOVector { Base = (byte*)memoryHandle.Pointer, Count = (UIntPtr)buffer.Length }; + handles[i] = memoryHandle; + } - try - { int buffersOffset = 0; while (totalBytesToWrite > 0) { From 2c685d4cdee28493dd1903dc5e3a8ac1aa00d7b4 Mon Sep 17 00:00:00 2001 From: Adam Sitnik Date: Wed, 20 Nov 2024 11:20:08 +0100 Subject: [PATCH 13/13] add an assert that is going to warn us if vector.Count is ever more than Int32.MaxValue --- src/native/libs/System.Native/pal_io.c | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/src/native/libs/System.Native/pal_io.c b/src/native/libs/System.Native/pal_io.c index 29db1be09b290..3777ad55151f9 100644 --- a/src/native/libs/System.Native/pal_io.c +++ b/src/native/libs/System.Native/pal_io.c @@ -1978,9 +1978,11 @@ static int GetAllowedVectorCount(IOVector* vectors, int32_t vectorCount) size_t totalLength = 0; for (int i = 0; i < allowedCount; i++) { + assert(INT_MAX >= vectors[i].Count); + totalLength += vectors[i].Count; - if (totalLength > INT_MAX && i > 0) + if (totalLength > INT_MAX) { allowedCount = i; break;