Support long module path (dotnet#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 <[email protected]>

* 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 <[email protected]>

* polishing

Co-authored-by: NextTurn <[email protected]>
Co-authored-by: Stephen Toub <[email protected]>

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<char>.Shared.Rent(1024);
+                const int StartLength =
+#if DEBUG
+                    1; // in debug, validate ArrayPool growth
+#else
+                    Interop.Kernel32.MAX_PATH;
+#endif
+                char[]? chars = ArrayPool<char>.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<char>.Shared.Rent(length * 2);
+                            ArrayPool<char>.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<char>.Shared.Rent(length * 2);
+                            ArrayPool<char>.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<char> charsSpan = chars.AsSpan(0, length);
+                        if (charsSpan.StartsWith(NtPathPrefix))
+                        {
+                            charsSpan = charsSpan.Slice(NtPathPrefix.Length);
+                        }
+                        module.FileName = charsSpan.ToString();
 
                         modules.Add(module);
                     }

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<ProcessModule>(), module => module.FileName == longNamePath);
+            string[] modulePaths = Process.GetCurrentProcess().Modules.Cast<ProcessModule>().Select(module => module.FileName).ToArray();
+            Assert.Contains(longNamePath, modulePaths);
         }
     }
 }