From 638594bf7d12f5fc6bcf832bc442107d366c3448 Mon Sep 17 00:00:00 2001 From: NextTurn <45985406+NextTurn@users.noreply.github.com> Date: Sun, 9 Feb 2020 00:00:00 +0800 Subject: [PATCH 01/13] Support long path --- .../Diagnostics/ProcessManager.Win32.cs | 28 ++++++++++++++++--- 1 file changed, 24 insertions(+), 4 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 18243a75c56702..0e698427c86030 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,9 @@ private static ProcessModuleCollection GetModules(int processId, bool firstModul var modules = new ProcessModuleCollection(firstModuleOnly ? 1 : modulesCount); - char[] chars = ArrayPool.Shared.Rent(1024); + const int MaxShortPath = 260; + int minimumLength = MaxShortPath; + char[]? chars = ArrayPool.Shared.Rent(minimumLength); try { for (int i = 0; i < modulesCount; i++) @@ -171,14 +173,29 @@ private static ProcessModuleCollection GetModules(int processId, bool firstModul module.ModuleName = new string(chars, 0, length); - length = Interop.Kernel32.GetModuleFileNameEx(processHandle, moduleHandle, chars, chars.Length); + for (; ; ) + { + length = Interop.Kernel32.GetModuleFileNameEx(processHandle, moduleHandle, chars, chars.Length); + if (length == chars.Length) + { + char[] toReturn = chars; + chars = null; + ArrayPool.Shared.Return(toReturn); + minimumLength = Math.Min(minimumLength * 2, short.MaxValue); + chars = ArrayPool.Shared.Rent(minimumLength); + continue; + } + + break; + } + if (length == 0) { HandleLastWin32Error(); continue; } - module.FileName = (length >= 4 && chars[0] == '\\' && chars[1] == '\\' && chars[2] == '?' && chars[3] == '\\') ? + module.FileName = chars.AsSpan().StartsWith(@"\\?\") ? new string(chars, 4, length - 4) : new string(chars, 0, length); @@ -187,7 +204,10 @@ private static ProcessModuleCollection GetModules(int processId, bool firstModul } finally { - ArrayPool.Shared.Return(chars); + if (chars != null) + { + ArrayPool.Shared.Return(chars); + } } return modules; From ff306e0d9e8003f81182a53bbc5cdc0ac537f960 Mon Sep 17 00:00:00 2001 From: NextTurn <45985406+NextTurn@users.noreply.github.com> Date: Sun, 15 Mar 2020 00:00:00 +0800 Subject: [PATCH 02/13] while (true) --- .../src/System/Diagnostics/ProcessManager.Win32.cs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) 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 0e698427c86030..85bf719530b3da 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 @@ -173,7 +173,7 @@ private static ProcessModuleCollection GetModules(int processId, bool firstModul module.ModuleName = new string(chars, 0, length); - for (; ; ) + while (true) { length = Interop.Kernel32.GetModuleFileNameEx(processHandle, moduleHandle, chars, chars.Length); if (length == chars.Length) From 20187cf93e90bcbe18d891f847e943c908bb6f70 Mon Sep 17 00:00:00 2001 From: NextTurn <45985406+NextTurn@users.noreply.github.com> Date: Sun, 15 Mar 2020 00:00:00 +0800 Subject: [PATCH 03/13] Calculate new length based on array length --- .../src/System/Diagnostics/ProcessManager.Win32.cs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) 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 85bf719530b3da..dcd5a7363fb1f4 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 @@ -178,10 +178,10 @@ private static ProcessModuleCollection GetModules(int processId, bool firstModul length = Interop.Kernel32.GetModuleFileNameEx(processHandle, moduleHandle, chars, chars.Length); if (length == chars.Length) { + minimumLength = chars.Length * 2; char[] toReturn = chars; chars = null; ArrayPool.Shared.Return(toReturn); - minimumLength = Math.Min(minimumLength * 2, short.MaxValue); chars = ArrayPool.Shared.Rent(minimumLength); continue; } From ed89f18a048624c9629e16371b762e6208bd8a63 Mon Sep 17 00:00:00 2001 From: NextTurn <45985406+NextTurn@users.noreply.github.com> Date: Sun, 15 Mar 2020 00:00:00 +0800 Subject: [PATCH 04/13] Add a test --- .../Diagnostics/ProcessManager.Win32.cs | 3 +- .../tests/ProcessModuleTests.cs | 57 +++++++++++++++++++ 2 files changed, 58 insertions(+), 2 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 dcd5a7363fb1f4..1904dac55812c7 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,8 +131,7 @@ private static ProcessModuleCollection GetModules(int processId, bool firstModul var modules = new ProcessModuleCollection(firstModuleOnly ? 1 : modulesCount); - const int MaxShortPath = 260; - int minimumLength = MaxShortPath; + int minimumLength = Interop.Kernel32.MAX_PATH; char[]? chars = ArrayPool.Shared.Rent(minimumLength); try { diff --git a/src/libraries/System.Diagnostics.Process/tests/ProcessModuleTests.cs b/src/libraries/System.Diagnostics.Process/tests/ProcessModuleTests.cs index 2a7d67888c3291..bfe645ea44dc8f 100644 --- a/src/libraries/System.Diagnostics.Process/tests/ProcessModuleTests.cs +++ b/src/libraries/System.Diagnostics.Process/tests/ProcessModuleTests.cs @@ -1,7 +1,9 @@ // Licensed to the .NET Foundation under one or more agreements. // The .NET Foundation licenses this file to you under the MIT license. +using System.IO; using System.Linq; +using System.Runtime.InteropServices; using Microsoft.DotNet.RemoteExecutor; using Xunit; @@ -28,6 +30,61 @@ public void TestModuleProperties() } } + [ConditionalFact(nameof(IsProcessElevated))] + [PlatformSpecific(TestPlatforms.Windows)] + public void TestModuleLongPath() + { + // // Metadata version: v4.0.30319 + // .assembly Test + // { + // .ver 0:0:0:0 + // } + // .module Test.dll + // // MVID: {48E60D10-353B-45DC-AA09-3A1E6B0FD382} + // .imagebase 0x00400000 + // .file alignment 0x00000200 + // .stackreserve 0x00100000 + // .subsystem 0x0003 // WINDOWS_CUI + // .corflags 0x00000001 // ILONLY + byte[] assemblyImage = Convert.FromBase64String( + "TVqQAAMAAAAEAAAA//8AALgAAAAAAAAAQAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAgAAAAA4fug4AtAnNIbgBTM0hVGhpcyBwcm9ncmFt" + + "IGNhbm5vdCBiZSBydW4gaW4gRE9TIG1vZGUuDQ0KJAAAAAAAAABQRQAATAECAJyj7l8AAAAAAAAAAOAAAiELAQsAAAIAAAACAAAAAAAAfiEAAAAgAAAAQAAA" + + "AABAAAAgAAAAAgAABAAAAAAAAAAEAAAAAAAAAABgAAAAAgAAAAAAAAMAQIUAABAAABAAAAAAEAAAEAAAAAAAABAAAAAAAAAAAAAAADAhAABLAAAAAAAAAAAA" + + "AAAAAAAAAAAAAAAAAAAAAAAAAEAAAAwAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAIAAACAAAAAAAAAAAAAAA" + + "CCAAAEgAAAAAAAAAAAAAAC50ZXh0AAAAhAEAAAAgAAAAAgAAAAIAAAAAAAAAAAAAAAAAACAAAGAucmVsb2MAAAwAAAAAQAAAAAIAAAAEAAAAAAAAAAAAAAAA" + + "AABAAABCAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAABgIQAAAAAAAEgAAAACAAUAUCAAAOAAAAABAAAA" + + "AAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAEJTSkIBAAEAAAAAAAwAAAB2NC4wLjMwMzE5AAAAAAQAXAAAAFQA" + + "AAAjfgAAsAAAABgAAAAjU3RyaW5ncwAAAADIAAAACAAAACNVUwDQAAAAEAAAACNHVUlEAAAAAAAAAAIAAAEFAAAAAQAAAAD6JTMAFgAAAQAAAAEAAAABAAAA" + + "AAAKAAEAAAAAAAAAAAABAAAAAAABAAEAAAAAAAAAAAAAAAAAAAAAAAAAEwAAAAAAADxNb2R1bGU+AFRlc3QuZGxsAFRlc3QAAAMgAAAAAAAQDeZIOzXcRaoJ" + + "Oh5rD9OCWCEAAAAAAAAAAAAAbiEAAAAgAAAAAAAAAAAAAAAAAAAAAAAAAAAAAGAhAAAAAAAAAABfQ29yRGxsTWFpbgBtc2NvcmVlLmRsbAAAAAAA/yUAIEAA" + + "AAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAA" + + "AAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAgAAAMAAAAgDEAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAA" + + "AAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAA" + + "AAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAA" + + "AAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAA" + + "AAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAA" + + "AAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAA" + + "AAAAAAAA" + ); + + string libraryDirectory = GetTestFilePath(); + Directory.CreateDirectory(libraryDirectory); + string libraryPath = Path.Combine(libraryDirectory, new string('_', 250) + ".dll"); + Assert.True(libraryPath.Length > 260); + File.WriteAllBytes(libraryPath, assemblyImage); + + IntPtr library = NativeLibrary.Load(libraryPath); + Assert.True(library != IntPtr.Zero); + try + { + Assert.Contains(Process.GetCurrentProcess().Modules.Cast(), module => module.FileName == libraryPath); + } + finally + { + NativeLibrary.Free(library); + } + } + [Fact] public void Modules_Get_ContainsHostFileName() { From 969b96bd616e14ab6de04b96d175b4240bbb8b18 Mon Sep 17 00:00:00 2001 From: Adam Sitnik Date: Thu, 25 Feb 2021 14:45:04 +0100 Subject: [PATCH 05/13] use a smaller value to ensure that at least for DEBUG builds we test the code path that rents a bigger array from ArrayPool --- .../src/System/Diagnostics/ProcessManager.Win32.cs | 6 ++++++ 1 file changed, 6 insertions(+) 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 1904dac55812c7..0fdb798af5334e 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); +#if RELEASE int minimumLength = Interop.Kernel32.MAX_PATH; +#else + // use a smaller value to ensure that at least for DEBUG builds + // we test the code path that rents a bigger array from ArrayPool + int minimumLength = Interop.Kernel32.MAX_PATH / 4; +#endif char[]? chars = ArrayPool.Shared.Rent(minimumLength); try { From c3db3e6aa77aa448888e44333212a635045eb050 Mon Sep 17 00:00:00 2001 From: Adam Sitnik Date: Thu, 25 Feb 2021 14:45:49 +0100 Subject: [PATCH 06/13] add few extra asserts to the test to see why it fails in CI --- .../tests/ProcessModuleTests.cs | 9 ++++++--- 1 file changed, 6 insertions(+), 3 deletions(-) diff --git a/src/libraries/System.Diagnostics.Process/tests/ProcessModuleTests.cs b/src/libraries/System.Diagnostics.Process/tests/ProcessModuleTests.cs index 55dd435d1a5f86..121669794e133c 100644 --- a/src/libraries/System.Diagnostics.Process/tests/ProcessModuleTests.cs +++ b/src/libraries/System.Diagnostics.Process/tests/ProcessModuleTests.cs @@ -96,25 +96,28 @@ public void ModulesAreDisposedWhenProcessIsDisposed() [PlatformSpecific(TestPlatforms.Windows)] 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); } From e26b5169fd3065894e6a91c9b8bbeb918d63f6c0 Mon Sep 17 00:00:00 2001 From: Adam Sitnik Date: Wed, 3 Mar 2021 10:26:18 +0100 Subject: [PATCH 07/13] Assembly.LoadFile used the way this test is implemented fails on Mono --- .../System.Diagnostics.Process/tests/ProcessModuleTests.cs | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/src/libraries/System.Diagnostics.Process/tests/ProcessModuleTests.cs b/src/libraries/System.Diagnostics.Process/tests/ProcessModuleTests.cs index 121669794e133c..b8f7d8f6ae0844 100644 --- a/src/libraries/System.Diagnostics.Process/tests/ProcessModuleTests.cs +++ b/src/libraries/System.Diagnostics.Process/tests/ProcessModuleTests.cs @@ -92,6 +92,7 @@ public void ModulesAreDisposedWhenProcessIsDisposed() Assert.Equal(expectedCount, disposedCount); } + [SkipOnMono("Assembly.LoadFile used the way this test is implemented fails on Mono")] [ConditionalFact(typeof(PathFeatures), nameof(PathFeatures.AreAllLongPathsAvailable))] [PlatformSpecific(TestPlatforms.Windows)] public void LongModuleFileNamesAreSupported() @@ -119,7 +120,8 @@ public void LongModuleFileNamesAreSupported() Assembly loaded = Assembly.LoadFile(longNamePath); Assert.Equal(longNamePath, loaded.Location); - Assert.Contains(Process.GetCurrentProcess().Modules.Cast(), module => module.FileName == longNamePath); + ProcessModule[] longPathModules = Process.GetCurrentProcess().Modules.Cast().Where(module => module.FileName.Contains(libraryName)).ToArray(); + Assert.Contains(longPathModules, module => module.FileName == longNamePath); } } } From c2cfde0685fe0c235c2872a0b06070856fd0e4d1 Mon Sep 17 00:00:00 2001 From: Adam Sitnik Date: Fri, 13 Aug 2021 13:56:18 +0200 Subject: [PATCH 08/13] don't run the test on Windows 7, for some reason it does not work --- .../tests/ProcessModuleTests.cs | 9 ++++++--- 1 file changed, 6 insertions(+), 3 deletions(-) diff --git a/src/libraries/System.Diagnostics.Process/tests/ProcessModuleTests.cs b/src/libraries/System.Diagnostics.Process/tests/ProcessModuleTests.cs index b8f7d8f6ae0844..73f096442f3f58 100644 --- a/src/libraries/System.Diagnostics.Process/tests/ProcessModuleTests.cs +++ b/src/libraries/System.Diagnostics.Process/tests/ProcessModuleTests.cs @@ -92,9 +92,12 @@ public void ModulesAreDisposedWhenProcessIsDisposed() Assert.Equal(expectedCount, disposedCount); } - [SkipOnMono("Assembly.LoadFile used the way this test is implemented fails on Mono")] - [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. From 46965ed5672252827836ecc987ed44f55a20ab13 Mon Sep 17 00:00:00 2001 From: Adam Sitnik Date: Fri, 13 Aug 2021 14:03:04 +0200 Subject: [PATCH 09/13] Apply suggestions from code review Co-authored-by: Stephen Toub --- .../System/Diagnostics/ProcessManager.Win32.cs | 17 ++++++----------- 1 file changed, 6 insertions(+), 11 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 8a0d8b7c5ca0a1..c317b61329bb90 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,12 +131,11 @@ private static ProcessModuleCollection GetModules(int processId, bool firstModul var modules = new ProcessModuleCollection(firstModuleOnly ? 1 : modulesCount); -#if RELEASE - int minimumLength = Interop.Kernel32.MAX_PATH; + int minimumLength = +#if DEBUG + 1; // in debug, validate ArrayPool growth #else - // use a smaller value to ensure that at least for DEBUG builds - // we test the code path that rents a bigger array from ArrayPool - int minimumLength = Interop.Kernel32.MAX_PATH / 4; + Interop.Kernel32.MAX_PATH; #endif char[]? chars = ArrayPool.Shared.Rent(minimumLength); try @@ -185,9 +184,8 @@ private static ProcessModuleCollection GetModules(int processId, bool firstModul { minimumLength = chars.Length * 2; char[] toReturn = chars; - chars = null; - ArrayPool.Shared.Return(toReturn); chars = ArrayPool.Shared.Rent(minimumLength); + ArrayPool.Shared.Return(toReturn); continue; } @@ -209,10 +207,7 @@ private static ProcessModuleCollection GetModules(int processId, bool firstModul } finally { - if (chars != null) - { - ArrayPool.Shared.Return(chars); - } + ArrayPool.Shared.Return(chars); } return modules; From b1e9b11fd6fcb59b910d24c7a4e48ef596035709 Mon Sep 17 00:00:00 2001 From: Adam Sitnik Date: Fri, 13 Aug 2021 14:47:15 +0200 Subject: [PATCH 10/13] handle truncated module names, simplify the implementation and dispose tmp modules before throwing exception --- .../Diagnostics/ProcessManager.Win32.cs | 46 ++++++++++--------- 1 file changed, 24 insertions(+), 22 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 c317b61329bb90..8dcf5472cb5a8e 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,13 +131,13 @@ private static ProcessModuleCollection GetModules(int processId, bool firstModul var modules = new ProcessModuleCollection(firstModuleOnly ? 1 : modulesCount); - int minimumLength = + const int startLength = #if DEBUG 1; // in debug, validate ArrayPool growth #else Interop.Kernel32.MAX_PATH; #endif - char[]? chars = ArrayPool.Shared.Rent(minimumLength); + char[]? chars = ArrayPool.Shared.Rent(startLength); try { for (int i = 0; i < modulesCount; i++) @@ -157,7 +157,7 @@ private static ProcessModuleCollection GetModules(int processId, bool firstModul Interop.Kernel32.NtModuleInfo ntModuleInfo; if (!Interop.Kernel32.GetModuleInformation(processHandle, moduleHandle, out ntModuleInfo)) { - HandleLastWin32Error(); + HandleLastWin32Error(null); continue; } @@ -168,39 +168,40 @@ 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) + { + ArrayPool.Shared.Return(chars); + chars = ArrayPool.Shared.Rent(length * 2); + } + if (length == 0) { - HandleLastWin32Error(); + HandleLastWin32Error(module); continue; } module.ModuleName = new string(chars, 0, length); - while (true) + while ((length = Interop.Kernel32.GetModuleFileNameEx(processHandle, moduleHandle, chars, chars.Length)) == chars.Length) { - length = Interop.Kernel32.GetModuleFileNameEx(processHandle, moduleHandle, chars, chars.Length); - if (length == chars.Length) - { - minimumLength = chars.Length * 2; - char[] toReturn = chars; - chars = ArrayPool.Shared.Rent(minimumLength); - ArrayPool.Shared.Return(toReturn); - continue; - } - - break; + ArrayPool.Shared.Return(chars); + chars = ArrayPool.Shared.Rent(length * 2); } if (length == 0) { - HandleLastWin32Error(); + HandleLastWin32Error(module); continue; } - module.FileName = chars.AsSpan().StartsWith(@"\\?\") ? - 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); } @@ -243,7 +244,7 @@ private static void EnumProcessModulesUntilSuccess(SafeProcessHandle processHand } } - private static void HandleLastWin32Error() + private static void HandleLastWin32Error(ProcessModule? processModule) { int lastError = Marshal.GetLastWin32Error(); switch (lastError) @@ -255,6 +256,7 @@ private static void HandleLastWin32Error() // move on. break; default: + processModule?.Dispose(); throw new Win32Exception(lastError); } } From 93414f6fa5550adfc4ca16bb5fdfbcd135b533f2 Mon Sep 17 00:00:00 2001 From: Adam Sitnik Date: Fri, 13 Aug 2021 15:04:08 +0200 Subject: [PATCH 11/13] make sure that when the module is not found by the test, the test runner prints available module paths --- .../System.Diagnostics.Process/tests/ProcessModuleTests.cs | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/libraries/System.Diagnostics.Process/tests/ProcessModuleTests.cs b/src/libraries/System.Diagnostics.Process/tests/ProcessModuleTests.cs index 73f096442f3f58..59fae43b84f194 100644 --- a/src/libraries/System.Diagnostics.Process/tests/ProcessModuleTests.cs +++ b/src/libraries/System.Diagnostics.Process/tests/ProcessModuleTests.cs @@ -123,8 +123,8 @@ public void LongModuleFileNamesAreSupported() Assembly loaded = Assembly.LoadFile(longNamePath); Assert.Equal(longNamePath, loaded.Location); - ProcessModule[] longPathModules = Process.GetCurrentProcess().Modules.Cast().Where(module => module.FileName.Contains(libraryName)).ToArray(); - Assert.Contains(longPathModules, module => module.FileName == longNamePath); + string[] modulePaths = Process.GetCurrentProcess().Modules.Cast().Select(module => module.FileName).ToArray(); + Assert.Contains(longNamePath, modulePaths); } } } From e391f714ace4a14ed3cf3474202118912a0b7e19 Mon Sep 17 00:00:00 2001 From: Adam Sitnik Date: Sun, 15 Aug 2021 11:26:52 +0200 Subject: [PATCH 12/13] Apply suggestions from code review Co-authored-by: Stephen Toub --- .../src/System/Diagnostics/ProcessManager.Win32.cs | 6 ++++-- 1 file changed, 4 insertions(+), 2 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 8dcf5472cb5a8e..d81f4da6189c0f 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 @@ -171,8 +171,9 @@ private static ProcessModuleCollection GetModules(int processId, bool firstModul int length = 0; while ((length = Interop.Kernel32.GetModuleBaseName(processHandle, moduleHandle, chars, chars.Length)) == chars.Length) { - ArrayPool.Shared.Return(chars); + char[] toReturn = chars; chars = ArrayPool.Shared.Rent(length * 2); + ArrayPool.Shared.Return(toReturn); } if (length == 0) @@ -185,8 +186,9 @@ private static ProcessModuleCollection GetModules(int processId, bool firstModul while ((length = Interop.Kernel32.GetModuleFileNameEx(processHandle, moduleHandle, chars, chars.Length)) == chars.Length) { - ArrayPool.Shared.Return(chars); + char[] toReturn = chars; chars = ArrayPool.Shared.Rent(length * 2); + ArrayPool.Shared.Return(toReturn); } if (length == 0) From 6b4f2bcd7b09229ca804736b05ad94331923ae5b Mon Sep 17 00:00:00 2001 From: Adam Sitnik Date: Sun, 15 Aug 2021 11:31:01 +0200 Subject: [PATCH 13/13] polishing --- .../System/Diagnostics/ProcessManager.Win32.cs | 15 ++++++++------- 1 file changed, 8 insertions(+), 7 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 d81f4da6189c0f..bcefc6cbb74a0a 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,13 +131,13 @@ private static ProcessModuleCollection GetModules(int processId, bool firstModul var modules = new ProcessModuleCollection(firstModuleOnly ? 1 : modulesCount); - const int startLength = + const int StartLength = #if DEBUG 1; // in debug, validate ArrayPool growth #else Interop.Kernel32.MAX_PATH; #endif - char[]? chars = ArrayPool.Shared.Rent(startLength); + char[]? chars = ArrayPool.Shared.Rent(StartLength); try { for (int i = 0; i < modulesCount; i++) @@ -157,7 +157,7 @@ private static ProcessModuleCollection GetModules(int processId, bool firstModul Interop.Kernel32.NtModuleInfo ntModuleInfo; if (!Interop.Kernel32.GetModuleInformation(processHandle, moduleHandle, out ntModuleInfo)) { - HandleLastWin32Error(null); + HandleLastWin32Error(); continue; } @@ -178,7 +178,8 @@ private static ProcessModuleCollection GetModules(int processId, bool firstModul if (length == 0) { - HandleLastWin32Error(module); + module.Dispose(); + HandleLastWin32Error(); continue; } @@ -193,7 +194,8 @@ private static ProcessModuleCollection GetModules(int processId, bool firstModul if (length == 0) { - HandleLastWin32Error(module); + module.Dispose(); + HandleLastWin32Error(); continue; } @@ -246,7 +248,7 @@ private static void EnumProcessModulesUntilSuccess(SafeProcessHandle processHand } } - private static void HandleLastWin32Error(ProcessModule? processModule) + private static void HandleLastWin32Error() { int lastError = Marshal.GetLastWin32Error(); switch (lastError) @@ -258,7 +260,6 @@ private static void HandleLastWin32Error(ProcessModule? processModule) // move on. break; default: - processModule?.Dispose(); throw new Win32Exception(lastError); } }