Skip to content

Commit 225acfe

Browse files
adamsitniknxtnstephentoub
authored
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 <[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]>
1 parent 0a1ff5b commit 225acfe

File tree

2 files changed

+45
-13
lines changed

2 files changed

+45
-13
lines changed

src/libraries/System.Diagnostics.Process/src/System/Diagnostics/ProcessManager.Win32.cs

Lines changed: 31 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -131,7 +131,13 @@ private static ProcessModuleCollection GetModules(int processId, bool firstModul
131131

132132
var modules = new ProcessModuleCollection(firstModuleOnly ? 1 : modulesCount);
133133

134-
char[] chars = ArrayPool<char>.Shared.Rent(1024);
134+
const int StartLength =
135+
#if DEBUG
136+
1; // in debug, validate ArrayPool growth
137+
#else
138+
Interop.Kernel32.MAX_PATH;
139+
#endif
140+
char[]? chars = ArrayPool<char>.Shared.Rent(StartLength);
135141
try
136142
{
137143
for (int i = 0; i < modulesCount; i++)
@@ -162,25 +168,44 @@ private static ProcessModuleCollection GetModules(int processId, bool firstModul
162168
BaseAddress = ntModuleInfo.BaseOfDll
163169
};
164170

165-
int length = Interop.Kernel32.GetModuleBaseName(processHandle, moduleHandle, chars, chars.Length);
171+
int length = 0;
172+
while ((length = Interop.Kernel32.GetModuleBaseName(processHandle, moduleHandle, chars, chars.Length)) == chars.Length)
173+
{
174+
char[] toReturn = chars;
175+
chars = ArrayPool<char>.Shared.Rent(length * 2);
176+
ArrayPool<char>.Shared.Return(toReturn);
177+
}
178+
166179
if (length == 0)
167180
{
181+
module.Dispose();
168182
HandleLastWin32Error();
169183
continue;
170184
}
171185

172186
module.ModuleName = new string(chars, 0, length);
173187

174-
length = Interop.Kernel32.GetModuleFileNameEx(processHandle, moduleHandle, chars, chars.Length);
188+
while ((length = Interop.Kernel32.GetModuleFileNameEx(processHandle, moduleHandle, chars, chars.Length)) == chars.Length)
189+
{
190+
char[] toReturn = chars;
191+
chars = ArrayPool<char>.Shared.Rent(length * 2);
192+
ArrayPool<char>.Shared.Return(toReturn);
193+
}
194+
175195
if (length == 0)
176196
{
197+
module.Dispose();
177198
HandleLastWin32Error();
178199
continue;
179200
}
180201

181-
module.FileName = (length >= 4 && chars[0] == '\\' && chars[1] == '\\' && chars[2] == '?' && chars[3] == '\\') ?
182-
new string(chars, 4, length - 4) :
183-
new string(chars, 0, length);
202+
const string NtPathPrefix = @"\\?\";
203+
ReadOnlySpan<char> charsSpan = chars.AsSpan(0, length);
204+
if (charsSpan.StartsWith(NtPathPrefix))
205+
{
206+
charsSpan = charsSpan.Slice(NtPathPrefix.Length);
207+
}
208+
module.FileName = charsSpan.ToString();
184209

185210
modules.Add(module);
186211
}

src/libraries/System.Diagnostics.Process/tests/ProcessModuleTests.cs

Lines changed: 14 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -92,32 +92,39 @@ public void ModulesAreDisposedWhenProcessIsDisposed()
9292
Assert.Equal(expectedCount, disposedCount);
9393
}
9494

95-
[ActiveIssue("https://github.com/dotnet/runtime/pull/335059")]
96-
[ConditionalFact(typeof(PathFeatures), nameof(PathFeatures.AreAllLongPathsAvailable))]
97-
[PlatformSpecific(TestPlatforms.Windows)]
95+
public static bool Is_LongModuleFileNamesAreSupported_TestEnabled
96+
=> PathFeatures.AreAllLongPathsAvailable() // we want to test long paths
97+
&& !PlatformDetection.IsMonoRuntime // Assembly.LoadFile used the way this test is implemented fails on Mono
98+
&& OperatingSystem.IsWindowsVersionAtLeast(8); // it's specific to Windows and does not work on Windows 7
99+
100+
[ConditionalFact(typeof(ProcessModuleTests), nameof(Is_LongModuleFileNamesAreSupported_TestEnabled))]
98101
public void LongModuleFileNamesAreSupported()
99102
{
100-
// To be able to test Long Path support for ProcessModule.FileName we need a .dll that has a path >= 260 chars.
103+
// To be able to test Long Path support for ProcessModule.FileName we need a .dll that has a path > 260 chars.
101104
// Since Long Paths support can be disabled (see the ConditionalFact attribute usage above),
102105
// we just copy "LongName.dll" from bin to a temp directory with a long name and load it from there.
103106
// Loading from new path is possible because the type exposed by the assembly is not referenced in any explicit way.
104107
const string libraryName = "LongPath.dll";
108+
const int minPathLength = 261;
105109

106110
string testBinPath = Path.GetDirectoryName(typeof(ProcessModuleTests).Assembly.Location);
107111
string libraryToCopy = Path.Combine(testBinPath, libraryName);
108112
Assert.True(File.Exists(libraryToCopy), $"{libraryName} was not present in bin folder '{testBinPath}'");
109113

110-
string directoryWithLongName = Path.Combine(TestDirectory, new string('a', Math.Max(1, 261 - TestDirectory.Length)));
114+
string directoryWithLongName = Path.Combine(TestDirectory, new string('a', Math.Max(1, minPathLength - TestDirectory.Length)));
111115
Directory.CreateDirectory(directoryWithLongName);
112116

113117
string longNamePath = Path.Combine(directoryWithLongName, libraryName);
114-
Assert.True(longNamePath.Length > 260);
118+
Assert.True(longNamePath.Length > minPathLength);
115119

116120
File.Copy(libraryToCopy, longNamePath);
121+
Assert.True(File.Exists(longNamePath));
117122

118123
Assembly loaded = Assembly.LoadFile(longNamePath);
124+
Assert.Equal(longNamePath, loaded.Location);
119125

120-
Assert.Contains(Process.GetCurrentProcess().Modules.Cast<ProcessModule>(), module => module.FileName == longNamePath);
126+
string[] modulePaths = Process.GetCurrentProcess().Modules.Cast<ProcessModule>().Select(module => module.FileName).ToArray();
127+
Assert.Contains(longNamePath, modulePaths);
121128
}
122129
}
123130
}

0 commit comments

Comments
 (0)