Skip to content
Merged
Show file tree
Hide file tree
Changes from 13 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -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++)
Expand All @@ -151,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;
}

Expand All @@ -162,25 +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<char>.Shared.Return(chars);
chars = ArrayPool<char>.Shared.Rent(length * 2);
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why is it better to rent a new array first before returning the old one?

Copy link
Member

@stephentoub stephentoub Aug 15, 2021

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The order of renting and returning doesn't matter. But what does matter is the array being returned isn't referenced by the variable used for returning in the finally block. If it is, and the Return call throws after having stored the array, it could double return.

}

if (length == 0)
{
HandleLastWin32Error();
HandleLastWin32Error(module);
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)
{
ArrayPool<char>.Shared.Return(chars);
chars = ArrayPool<char>.Shared.Rent(length * 2);
}

if (length == 0)
{
HandleLastWin32Error();
HandleLastWin32Error(module);
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);
}
Expand Down Expand Up @@ -223,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)
Expand All @@ -235,6 +256,7 @@ private static void HandleLastWin32Error()
// move on.
break;
default:
processModule?.Dispose();
throw new Win32Exception(lastError);
}
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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);
}
}
}