From 225acfe78d1b308a7e1105278d72aecc9ffc2e70 Mon Sep 17 00:00:00 2001 From: Adam Sitnik Date: Sun, 15 Aug 2021 13:32:09 +0200 Subject: [PATCH] Support long module path (#57335) * Support long path * while (true) * Calculate new length based on array length * Add a test * use a smaller value to ensure that at least for DEBUG builds we test the code path that rents a bigger array from ArrayPool * add few extra asserts to the test to see why it fails in CI * Assembly.LoadFile used the way this test is implemented fails on Mono * don't run the test on Windows 7, for some reason it does not work * Apply suggestions from code review Co-authored-by: Stephen Toub * handle truncated module names, simplify the implementation and dispose tmp modules before throwing exception * make sure that when the module is not found by the test, the test runner prints available module paths * Apply suggestions from code review Co-authored-by: Stephen Toub * polishing Co-authored-by: NextTurn <45985406+NextTurn@users.noreply.github.com> Co-authored-by: Stephen Toub --- .../Diagnostics/ProcessManager.Win32.cs | 37 ++++++++++++++++--- .../tests/ProcessModuleTests.cs | 21 +++++++---- 2 files changed, 45 insertions(+), 13 deletions(-) diff --git a/src/libraries/System.Diagnostics.Process/src/System/Diagnostics/ProcessManager.Win32.cs b/src/libraries/System.Diagnostics.Process/src/System/Diagnostics/ProcessManager.Win32.cs index e272d5fa4af1f..bcefc6cbb74a0 100644 --- a/src/libraries/System.Diagnostics.Process/src/System/Diagnostics/ProcessManager.Win32.cs +++ b/src/libraries/System.Diagnostics.Process/src/System/Diagnostics/ProcessManager.Win32.cs @@ -131,7 +131,13 @@ private static ProcessModuleCollection GetModules(int processId, bool firstModul var modules = new ProcessModuleCollection(firstModuleOnly ? 1 : modulesCount); - char[] chars = ArrayPool.Shared.Rent(1024); + const int StartLength = +#if DEBUG + 1; // in debug, validate ArrayPool growth +#else + Interop.Kernel32.MAX_PATH; +#endif + char[]? chars = ArrayPool.Shared.Rent(StartLength); try { for (int i = 0; i < modulesCount; i++) @@ -162,25 +168,44 @@ private static ProcessModuleCollection GetModules(int processId, bool firstModul BaseAddress = ntModuleInfo.BaseOfDll }; - int length = Interop.Kernel32.GetModuleBaseName(processHandle, moduleHandle, chars, chars.Length); + int length = 0; + while ((length = Interop.Kernel32.GetModuleBaseName(processHandle, moduleHandle, chars, chars.Length)) == chars.Length) + { + char[] toReturn = chars; + chars = ArrayPool.Shared.Rent(length * 2); + ArrayPool.Shared.Return(toReturn); + } + if (length == 0) { + module.Dispose(); HandleLastWin32Error(); continue; } module.ModuleName = new string(chars, 0, length); - length = Interop.Kernel32.GetModuleFileNameEx(processHandle, moduleHandle, chars, chars.Length); + while ((length = Interop.Kernel32.GetModuleFileNameEx(processHandle, moduleHandle, chars, chars.Length)) == chars.Length) + { + char[] toReturn = chars; + chars = ArrayPool.Shared.Rent(length * 2); + ArrayPool.Shared.Return(toReturn); + } + if (length == 0) { + module.Dispose(); HandleLastWin32Error(); continue; } - module.FileName = (length >= 4 && chars[0] == '\\' && chars[1] == '\\' && chars[2] == '?' && chars[3] == '\\') ? - new string(chars, 4, length - 4) : - new string(chars, 0, length); + const string NtPathPrefix = @"\\?\"; + ReadOnlySpan charsSpan = chars.AsSpan(0, length); + if (charsSpan.StartsWith(NtPathPrefix)) + { + charsSpan = charsSpan.Slice(NtPathPrefix.Length); + } + module.FileName = charsSpan.ToString(); modules.Add(module); } diff --git a/src/libraries/System.Diagnostics.Process/tests/ProcessModuleTests.cs b/src/libraries/System.Diagnostics.Process/tests/ProcessModuleTests.cs index 2631cb8b1a3dc..59fae43b84f19 100644 --- a/src/libraries/System.Diagnostics.Process/tests/ProcessModuleTests.cs +++ b/src/libraries/System.Diagnostics.Process/tests/ProcessModuleTests.cs @@ -92,32 +92,39 @@ public void ModulesAreDisposedWhenProcessIsDisposed() Assert.Equal(expectedCount, disposedCount); } - [ActiveIssue("https://github.com/dotnet/runtime/pull/335059")] - [ConditionalFact(typeof(PathFeatures), nameof(PathFeatures.AreAllLongPathsAvailable))] - [PlatformSpecific(TestPlatforms.Windows)] + public static bool Is_LongModuleFileNamesAreSupported_TestEnabled + => PathFeatures.AreAllLongPathsAvailable() // we want to test long paths + && !PlatformDetection.IsMonoRuntime // Assembly.LoadFile used the way this test is implemented fails on Mono + && OperatingSystem.IsWindowsVersionAtLeast(8); // it's specific to Windows and does not work on Windows 7 + + [ConditionalFact(typeof(ProcessModuleTests), nameof(Is_LongModuleFileNamesAreSupported_TestEnabled))] public void LongModuleFileNamesAreSupported() { - // To be able to test Long Path support for ProcessModule.FileName we need a .dll that has a path >= 260 chars. + // To be able to test Long Path support for ProcessModule.FileName we need a .dll that has a path > 260 chars. // Since Long Paths support can be disabled (see the ConditionalFact attribute usage above), // we just copy "LongName.dll" from bin to a temp directory with a long name and load it from there. // Loading from new path is possible because the type exposed by the assembly is not referenced in any explicit way. const string libraryName = "LongPath.dll"; + const int minPathLength = 261; string testBinPath = Path.GetDirectoryName(typeof(ProcessModuleTests).Assembly.Location); string libraryToCopy = Path.Combine(testBinPath, libraryName); Assert.True(File.Exists(libraryToCopy), $"{libraryName} was not present in bin folder '{testBinPath}'"); - string directoryWithLongName = Path.Combine(TestDirectory, new string('a', Math.Max(1, 261 - TestDirectory.Length))); + string directoryWithLongName = Path.Combine(TestDirectory, new string('a', Math.Max(1, minPathLength - TestDirectory.Length))); Directory.CreateDirectory(directoryWithLongName); string longNamePath = Path.Combine(directoryWithLongName, libraryName); - Assert.True(longNamePath.Length > 260); + Assert.True(longNamePath.Length > minPathLength); File.Copy(libraryToCopy, longNamePath); + Assert.True(File.Exists(longNamePath)); Assembly loaded = Assembly.LoadFile(longNamePath); + Assert.Equal(longNamePath, loaded.Location); - Assert.Contains(Process.GetCurrentProcess().Modules.Cast(), module => module.FileName == longNamePath); + string[] modulePaths = Process.GetCurrentProcess().Modules.Cast().Select(module => module.FileName).ToArray(); + Assert.Contains(longNamePath, modulePaths); } } }