diff --git a/src/libraries/Common/src/Interop/Unix/System.Native/Interop.ReadDir.cs b/src/libraries/Common/src/Interop/Unix/System.Native/Interop.ReadDir.cs index 9362a84a4c280f..5680dde9a78118 100644 --- a/src/libraries/Common/src/Interop/Unix/System.Native/Interop.ReadDir.cs +++ b/src/libraries/Common/src/Interop/Unix/System.Native/Interop.ReadDir.cs @@ -29,39 +29,32 @@ internal unsafe struct DirectoryEntry internal byte* Name; internal int NameLength; internal NodeType InodeType; - internal const int NameBufferSize = 256; // sizeof(dirent->d_name) == NAME_MAX + 1 internal ReadOnlySpan GetName(Span buffer) { - // -1 for null terminator (buffer will not include one), - // and -1 because GetMaxCharCount pessimistically assumes the buffer may start with a partial surrogate - Debug.Assert(buffer.Length >= Encoding.UTF8.GetMaxCharCount(NameBufferSize - 1 - 1)); - Debug.Assert(Name != null, "should not have a null name"); ReadOnlySpan nameBytes = (NameLength == -1) - // In this case the struct was allocated via struct dirent *readdir(DIR *dirp); - ? new ReadOnlySpan(Name, new ReadOnlySpan(Name, NameBufferSize).IndexOf(0)) + ? MemoryMarshal.CreateReadOnlySpanFromNullTerminated(Name) : new ReadOnlySpan(Name, NameLength); Debug.Assert(nameBytes.Length > 0, "we shouldn't have gotten a garbage value from the OS"); - int charCount = Encoding.UTF8.GetChars(nameBytes, buffer); - ReadOnlySpan value = buffer.Slice(0, charCount); - Debug.Assert(NameLength != -1 || !value.Contains('\0'), "should not have embedded nulls if we parsed the end of string"); - return value; + ReadOnlySpan result = !Encoding.UTF8.TryGetChars(nameBytes, buffer, out int charsWritten) + ? Encoding.UTF8.GetString(nameBytes) // Fallback to allocation since this is a rare case + : buffer.Slice(0, charsWritten); + + Debug.Assert(!result.Contains('\0'), "should not have embedded nulls"); + + return result; } } [LibraryImport(Libraries.SystemNative, EntryPoint = "SystemNative_OpenDir", StringMarshalling = StringMarshalling.Utf8, SetLastError = true)] internal static partial IntPtr OpenDir(string path); - [LibraryImport(Libraries.SystemNative, EntryPoint = "SystemNative_GetReadDirRBufferSize", SetLastError = false)] - [SuppressGCTransition] - internal static partial int GetReadDirRBufferSize(); - - [LibraryImport(Libraries.SystemNative, EntryPoint = "SystemNative_ReadDirR")] - internal static unsafe partial int ReadDirR(IntPtr dir, byte* buffer, int bufferSize, DirectoryEntry* outputEntry); + [LibraryImport(Libraries.SystemNative, EntryPoint = "SystemNative_ReadDir")] + internal static unsafe partial int ReadDir(IntPtr dir, DirectoryEntry* outputEntry); [LibraryImport(Libraries.SystemNative, EntryPoint = "SystemNative_CloseDir", SetLastError = true)] internal static partial int CloseDir(IntPtr dir); diff --git a/src/libraries/System.Private.CoreLib/src/System/IO/Enumeration/FileSystemEntry.Unix.cs b/src/libraries/System.Private.CoreLib/src/System/IO/Enumeration/FileSystemEntry.Unix.cs index be406ecf507146..61ff4864819851 100644 --- a/src/libraries/System.Private.CoreLib/src/System/IO/Enumeration/FileSystemEntry.Unix.cs +++ b/src/libraries/System.Private.CoreLib/src/System/IO/Enumeration/FileSystemEntry.Unix.cs @@ -3,6 +3,7 @@ using System.Diagnostics; using System.Runtime.InteropServices; +using System.Runtime.CompilerServices; namespace System.IO.Enumeration { @@ -11,6 +12,9 @@ namespace System.IO.Enumeration /// public unsafe ref partial struct FileSystemEntry { + // A reasonable size for a decoding buffer. If it's not enough, we fall back to allocating a string. + // Some filesystem can have filenames larger than that. + private const int DecodedNameBufferLength = 256; private Interop.Sys.DirectoryEntry _directoryEntry; private bool _isDirectory; private FileStatus _status; @@ -19,10 +23,10 @@ public unsafe ref partial struct FileSystemEntry private ReadOnlySpan _fileName; private FileNameBuffer _fileNameBuffer; - // Wrap the fixed buffer to workaround visibility issues in api compat verification + [InlineArray(DecodedNameBufferLength)] private struct FileNameBuffer { - internal fixed char _buffer[Interop.Sys.DirectoryEntry.NameBufferSize]; + internal char _char0; } internal static FileAttributes Initialize( @@ -95,7 +99,9 @@ public ReadOnlySpan FileName { if (_directoryEntry.NameLength != 0 && _fileName.Length == 0) { - Span buffer = MemoryMarshal.CreateSpan(ref _fileNameBuffer._buffer[0], Interop.Sys.DirectoryEntry.NameBufferSize); + // Use unsafe API to create the Span to allow it to escape. It is safe as long as + // the whole FileSystemEntry is never copied. + Span buffer = MemoryMarshal.CreateSpan(ref _fileNameBuffer._char0, DecodedNameBufferLength); _fileName = _directoryEntry.GetName(buffer); } diff --git a/src/libraries/System.Private.CoreLib/src/System/IO/Enumeration/FileSystemEnumerator.Unix.cs b/src/libraries/System.Private.CoreLib/src/System/IO/Enumeration/FileSystemEnumerator.Unix.cs index 3076be747961a8..51dcc165f6f135 100644 --- a/src/libraries/System.Private.CoreLib/src/System/IO/Enumeration/FileSystemEnumerator.Unix.cs +++ b/src/libraries/System.Private.CoreLib/src/System/IO/Enumeration/FileSystemEnumerator.Unix.cs @@ -29,8 +29,6 @@ public abstract unsafe partial class FileSystemEnumerator : CriticalFin // Used for creating full paths private char[]? _pathBuffer; - // Used to get the raw entry data - private byte[]? _entryBuffer; private void Init() { @@ -45,8 +43,6 @@ private void Init() try { _pathBuffer = ArrayPool.Shared.Rent(StandardBufferSize); - int size = Interop.Sys.GetReadDirRBufferSize(); - _entryBuffer = size > 0 ? ArrayPool.Shared.Rent(size) : null; } catch { @@ -103,13 +99,11 @@ public bool MoveNext() // If HAVE_READDIR_R is defined for the platform FindNextEntry depends on _entryBuffer being fixed since // _entry will point to a string in the middle of the array. If the array is not fixed GC can move it after // the native call and _entry will point to a bogus file name. - fixed (byte* entryBufferPtr = _entryBuffer) + do { - do - { - FindNextEntry(entryBufferPtr, _entryBuffer == null ? 0 : _entryBuffer.Length); - if (_lastEntryFound) - return false; + FindNextEntry(); + if (_lastEntryFound) + return false; FileAttributes attributes = FileSystemEntry.Initialize( ref entry, _entry, _currentPath, _rootDirectory, _originalRootDirectory, new Span(_pathBuffer)); @@ -152,32 +146,23 @@ public bool MoveNext() } } - if (ShouldIncludeEntry(ref entry)) - { - _current = TransformEntry(ref entry); - return true; - } - } while (true); - } + if (ShouldIncludeEntry(ref entry)) + { + _current = TransformEntry(ref entry); + return true; + } + } while (true); } bool ShouldSkip(FileAttributes attributeToSkip) => (_options.AttributesToSkip & attributeToSkip) != 0; } private unsafe void FindNextEntry() - { - fixed (byte* entryBufferPtr = _entryBuffer) - { - FindNextEntry(entryBufferPtr, _entryBuffer == null ? 0 : _entryBuffer.Length); - } - } - - private unsafe void FindNextEntry(byte* entryBufferPtr, int bufferLength) { int result; fixed (Interop.Sys.DirectoryEntry* e = &_entry) { - result = Interop.Sys.ReadDirR(_directoryHandle, entryBufferPtr, bufferLength, e); + result = Interop.Sys.ReadDir(_directoryHandle, e); } switch (result) @@ -245,12 +230,6 @@ private void InternalDispose(bool disposing) _pathBuffer = null; ArrayPool.Shared.Return(pathBuffer); } - - if (_entryBuffer is byte[] entryBuffer) - { - _entryBuffer = null; - ArrayPool.Shared.Return(entryBuffer); - } } } diff --git a/src/libraries/System.Private.CoreLib/src/System/TimeZoneInfo.Unix.NonAndroid.cs b/src/libraries/System.Private.CoreLib/src/System/TimeZoneInfo.Unix.NonAndroid.cs index 39f0df8a4b1ee4..7969fbd99c9a0b 100644 --- a/src/libraries/System.Private.CoreLib/src/System/TimeZoneInfo.Unix.NonAndroid.cs +++ b/src/libraries/System.Private.CoreLib/src/System/TimeZoneInfo.Unix.NonAndroid.cs @@ -260,17 +260,6 @@ private static List ParseTimeZoneIds(StreamReader reader) return id; } - private static string? GetDirectoryEntryFullPath(ref Interop.Sys.DirectoryEntry dirent, string currentPath) - { - ReadOnlySpan direntName = dirent.GetName(stackalloc char[Interop.Sys.DirectoryEntry.NameBufferSize]); - - if ((direntName.Length == 1 && direntName[0] == '.') || - (direntName.Length == 2 && direntName[0] == '.' && direntName[1] == '.')) - return null; - - return Path.Join(currentPath.AsSpan(), direntName); - } - private static bool CompareTimeZoneFile(string filePath, byte[] buffer, byte[] rawData) { try diff --git a/src/libraries/System.Runtime/tests/System.IO.FileSystem.Tests/Directory/EnumerableTests.cs b/src/libraries/System.Runtime/tests/System.IO.FileSystem.Tests/Directory/EnumerableTests.cs index dc0d6d9a58c62f..20f61989594b09 100644 --- a/src/libraries/System.Runtime/tests/System.IO.FileSystem.Tests/Directory/EnumerableTests.cs +++ b/src/libraries/System.Runtime/tests/System.IO.FileSystem.Tests/Directory/EnumerableTests.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.Collections.Concurrent; using System.Collections.Generic; using System.Threading; using System.Threading.Tasks; @@ -34,6 +35,26 @@ public void FileEnumeratorIsThreadSafe() } } + [Fact] + public void FileEnumeratorIsThreadSafe_ParallelForEach() + { + List expected = []; + string directory = Directory.CreateDirectory(GetTestFilePath()).FullName; + for (int i = 0; i < 100; i++) + { + string file = Path.Join(directory, GetTestFileName()); + File.Create(file).Dispose(); + expected.Add(file); + } + + for (int i = 0; i < 100; i++) // test multiple times to ensure thread safety. + { + ConcurrentBag result = []; + ParallelLoopResult parallelResult = Parallel.ForEach(Directory.EnumerateFiles(directory), f => result.Add(f)); + AssertExtensions.CollectionEqual(expected, result, StringComparer.Ordinal); + } + } + [Fact] public void EnumerateDirectories_NonBreakingSpace() { diff --git a/src/libraries/System.Runtime/tests/System.IO.FileSystem.Tests/ManualTests/NtfsOnLinuxSetup.cs b/src/libraries/System.Runtime/tests/System.IO.FileSystem.Tests/ManualTests/NtfsOnLinuxSetup.cs new file mode 100644 index 00000000000000..76bb3304a2f793 --- /dev/null +++ b/src/libraries/System.Runtime/tests/System.IO.FileSystem.Tests/ManualTests/NtfsOnLinuxSetup.cs @@ -0,0 +1,56 @@ +// Licensed to the .NET Foundation under one or more agreements. +// The .NET Foundation licenses this file to you under the MIT license. + +using System.Diagnostics; +using Xunit.Sdk; + +namespace System.IO.ManualTests +{ + public class NtfsOnLinuxSetup : IDisposable + { + public NtfsOnLinuxSetup() + { + if (!NtfsOnLinuxTests.IsManualTestsEnabledAndElevated) + throw new XunitException("Set MANUAL_TESTS envvar and run as elevated to execute this test setup."); + + ExecuteShell(""" + dd if=/dev/zero of=my_loop_device.img bs=1M count=100 + losetup /dev/loop99 my_loop_device.img + mkfs -t ntfs /dev/loop99 + mkdir -p /mnt/ntfs + mount /dev/loop99 /mnt/ntfs + """); + } + + public void Dispose() + { + ExecuteShell(""" + umount /mnt/ntfs + losetup -d /dev/loop99 + rm my_loop_device.img + """); + } + + private static void ExecuteShell(string command) + { + using Process process = new Process + { + StartInfo = new ProcessStartInfo + { + FileName = "/bin/sh", + ArgumentList = { "-c", command }, + RedirectStandardOutput = true, + RedirectStandardError = true, + UseShellExecute = false + } + }; + process.OutputDataReceived += (sender, e) => Console.WriteLine($"[OUTPUT] {e.Data}"); + process.ErrorDataReceived += (sender, e) => Console.WriteLine($"[ERROR] {e.Data}"); + + process.Start(); + process.BeginOutputReadLine(); + process.BeginErrorReadLine(); + process.WaitForExit(); + } + } +} diff --git a/src/libraries/System.Runtime/tests/System.IO.FileSystem.Tests/ManualTests/NtfsOnLinuxTests.cs b/src/libraries/System.Runtime/tests/System.IO.FileSystem.Tests/ManualTests/NtfsOnLinuxTests.cs new file mode 100644 index 00000000000000..3888321e9f58ec --- /dev/null +++ b/src/libraries/System.Runtime/tests/System.IO.FileSystem.Tests/ManualTests/NtfsOnLinuxTests.cs @@ -0,0 +1,29 @@ +// 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.Text; +using Xunit; + +namespace System.IO.ManualTests +{ + public class NtfsOnLinuxTests : IClassFixture + { + internal static bool IsManualTestsEnabledAndElevated => FileSystemManualTests.ManualTestsEnabled && AdminHelpers.IsProcessElevated(); + + [ConditionalTheory(nameof(IsManualTestsEnabledAndElevated))] + [PlatformSpecific(TestPlatforms.Linux)] + [InlineData("Ω", 255)] + [InlineData("あ", 255)] + [InlineData("😀", 127)] + public void NtfsOnLinux_FilenamesLongerThan255Bytes_FileEnumerationSucceeds(string codePoint, int maxAllowedLength) + { + string filename = string.Concat(Enumerable.Repeat(codePoint, maxAllowedLength)); + Assert.True(Encoding.UTF8.GetByteCount(filename) > 255); + + string filePath = $"/mnt/ntfs/{filename}"; + File.Create(filePath).Dispose(); + Assert.Contains(filePath, Directory.GetFiles("/mnt/ntfs")); + } + } +} diff --git a/src/libraries/System.Runtime/tests/System.IO.FileSystem.Tests/ManualTests/System.IO.FileSystem.Manual.Tests.csproj b/src/libraries/System.Runtime/tests/System.IO.FileSystem.Tests/ManualTests/System.IO.FileSystem.Manual.Tests.csproj index 62623a6e9757cc..4f840033a65599 100644 --- a/src/libraries/System.Runtime/tests/System.IO.FileSystem.Tests/ManualTests/System.IO.FileSystem.Manual.Tests.csproj +++ b/src/libraries/System.Runtime/tests/System.IO.FileSystem.Tests/ManualTests/System.IO.FileSystem.Manual.Tests.csproj @@ -4,5 +4,7 @@ + + diff --git a/src/native/libs/Common/pal_config.h.in b/src/native/libs/Common/pal_config.h.in index c8e43fa37dd6a7..f64132cd449968 100644 --- a/src/native/libs/Common/pal_config.h.in +++ b/src/native/libs/Common/pal_config.h.in @@ -27,7 +27,6 @@ #cmakedefine01 HAVE_STAT_FLAGS #cmakedefine01 HAVE_LCHFLAGS #cmakedefine01 HAVE_GNU_STRERROR_R -#cmakedefine01 HAVE_READDIR_R #cmakedefine01 HAVE_DIRENT_NAME_LEN #cmakedefine01 HAVE_MNTINFO #cmakedefine01 HAVE_STATFS_FSTYPENAME diff --git a/src/native/libs/System.Native/entrypoints.c b/src/native/libs/System.Native/entrypoints.c index fb6d78ceef3c55..ac9049b10557c8 100644 --- a/src/native/libs/System.Native/entrypoints.c +++ b/src/native/libs/System.Native/entrypoints.c @@ -66,8 +66,7 @@ static const Entry s_sysNative[] = DllImportEntry(SystemNative_MemfdCreate) DllImportEntry(SystemNative_ShmOpen) DllImportEntry(SystemNative_ShmUnlink) - DllImportEntry(SystemNative_GetReadDirRBufferSize) - DllImportEntry(SystemNative_ReadDirR) + DllImportEntry(SystemNative_ReadDir) DllImportEntry(SystemNative_OpenDir) DllImportEntry(SystemNative_CloseDir) DllImportEntry(SystemNative_Pipe) diff --git a/src/native/libs/System.Native/pal_io.c b/src/native/libs/System.Native/pal_io.c index 3777ad55151f9d..74bead7dc0ada9 100644 --- a/src/native/libs/System.Native/pal_io.c +++ b/src/native/libs/System.Native/pal_io.c @@ -13,6 +13,7 @@ #include #include #include +#include #include #include #include @@ -498,97 +499,13 @@ static void ConvertDirent(const struct dirent* entry, DirectoryEntry* outputEntr #endif } -#if HAVE_READDIR_R -// struct dirent typically contains 64-bit numbers (e.g. d_ino), so we align it at 8-byte. -static const size_t dirent_alignment = 8; -#endif - -int32_t SystemNative_GetReadDirRBufferSize(void) -{ -#if HAVE_READDIR_R - size_t result = sizeof(struct dirent); -#ifdef TARGET_SUNOS - // The d_name array is declared with only a single byte in it. - // We have to add pathconf("dir", _PC_NAME_MAX) more bytes. - // MAXNAMELEN is the largest possible value returned from pathconf. - result += MAXNAMELEN; -#endif - // dirent should be under 2k in size - assert(result < 2048); - // add some extra space so we can align the buffer to dirent. - return (int32_t)(result + dirent_alignment - 1); -#else - return 0; -#endif -} - -// To reduce the number of string copies, the caller of this function is responsible to ensure the memory -// referenced by outputEntry remains valid until it is read. -// If the platform supports readdir_r, the caller provides a buffer into which the data is read. -// If the platform uses readdir, the caller must ensure no calls are made to readdir/closedir since those will invalidate +// The caller must ensure no calls are made to readdir/closedir since those will invalidate // the current dirent. We assume the platform supports concurrent readdir calls to different DIRs. -int32_t SystemNative_ReadDirR(DIR* dir, uint8_t* buffer, int32_t bufferSize, DirectoryEntry* outputEntry) +int32_t SystemNative_ReadDir(DIR* dir, DirectoryEntry* outputEntry) { assert(dir != NULL); assert(outputEntry != NULL); -#if HAVE_READDIR_R - assert(buffer != NULL); - - // align to dirent - struct dirent* entry = (struct dirent*)((size_t)(buffer + dirent_alignment - 1) & ~(dirent_alignment - 1)); - - // check there is dirent size available at entry - if ((buffer + bufferSize) < ((uint8_t*)entry + sizeof(struct dirent))) - { - assert(false && "Buffer size too small; use GetReadDirRBufferSize to get required buffer size"); - return ERANGE; - } - - struct dirent* result = NULL; -#ifdef _AIX - // AIX returns 0 on success, but bizarrely, it returns 9 for both error and - // end-of-directory. result is NULL for both cases. The API returns the - // same thing for EOD/error, so disambiguation between the two is nearly - // impossible without clobbering errno for yourself and seeing if the API - // changed it. See: - // https://www.ibm.com/support/knowledgecenter/ssw_aix_71/com.ibm.aix.basetrf2/readdir_r.htm - - errno = 0; // create a success condition for the API to clobber - int error = readdir_r(dir, entry, &result); - - if (error == 9) - { - memset(outputEntry, 0, sizeof(*outputEntry)); // managed out param must be initialized - return errno == 0 ? -1 : errno; - } -#else - int error; - - // EINTR isn't documented, happens in practice on macOS. - while ((error = readdir_r(dir, entry, &result)) && errno == EINTR); - - // positive error number returned -> failure - if (error != 0) - { - assert(error > 0); - memset(outputEntry, 0, sizeof(*outputEntry)); // managed out param must be initialized - return error; - } - - // 0 returned with null result -> end-of-stream - if (result == NULL) - { - memset(outputEntry, 0, sizeof(*outputEntry)); // managed out param must be initialized - return -1; // shim convention for end-of-stream - } -#endif - - // 0 returned with non-null result (guaranteed to be set to entry arg) -> success - assert(result == entry); -#else - (void)buffer; // unused - (void)bufferSize; // unused errno = 0; struct dirent* entry = readdir(dir); @@ -605,7 +522,7 @@ int32_t SystemNative_ReadDirR(DIR* dir, uint8_t* buffer, int32_t bufferSize, Dir } return -1; } -#endif + ConvertDirent(entry, outputEntry); return 0; } diff --git a/src/native/libs/System.Native/pal_io.h b/src/native/libs/System.Native/pal_io.h index a07aa7c170219a..330fa2997b9f20 100644 --- a/src/native/libs/System.Native/pal_io.h +++ b/src/native/libs/System.Native/pal_io.h @@ -398,16 +398,11 @@ PALEXPORT intptr_t SystemNative_ShmOpen(const char* name, int32_t flags, int32_t PALEXPORT int32_t SystemNative_ShmUnlink(const char* name); /** - * Returns the size of the dirent struct on the current architecture - */ -PALEXPORT int32_t SystemNative_GetReadDirRBufferSize(void); - -/** - * Re-entrant readdir that will retrieve the next dirent from the directory stream pointed to by dir. + * Retrieves the next dirent from the directory stream pointed to by dir. * * Returns 0 when data is retrieved; returns -1 when end-of-stream is reached; returns an error code on failure */ -PALEXPORT int32_t SystemNative_ReadDirR(DIR* dir, uint8_t* buffer, int32_t bufferSize, DirectoryEntry* outputEntry); +PALEXPORT int32_t SystemNative_ReadDir(DIR* dir, DirectoryEntry* outputEntry); /** * Returns a DIR struct containing info about the current path or NULL on failure; sets errno on fail. diff --git a/src/native/libs/configure.cmake b/src/native/libs/configure.cmake index fc7332a54f9fac..561ab9229986bd 100644 --- a/src/native/libs/configure.cmake +++ b/src/native/libs/configure.cmake @@ -369,21 +369,6 @@ check_c_source_compiles( " HAVE_GNU_STRERROR_R) -check_c_source_compiles( - " - #include - #include - int main(void) - { - DIR* dir = NULL; - struct dirent* entry = NULL; - struct dirent* result; - readdir_r(dir, entry, &result); - return 0; - } - " - HAVE_READDIR_R) - check_c_source_compiles( " #include