Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

don't try to test module path if we can't load the module #57590

Merged
merged 1 commit into from
Aug 17, 2021

Conversation

adamsitnik
Copy link
Member

fixes #57452

@ghost
Copy link

ghost commented Aug 17, 2021

Tagging subscribers to this area: @dotnet/area-system-diagnostics-process
See info in area-owners.md if you want to be subscribed.

Issue Details

fixes #57452

Author: adamsitnik
Assignees: -
Labels:

area-System.Diagnostics.Process

Milestone: -

@@ -36,9 +36,7 @@ public void LongModuleFileNamesAreSupported()
IntPtr moduleHandle = Interop.Kernel32.LoadLibrary(longNamePath);
if (moduleHandle == IntPtr.Zero)
{
Assert.Equal(126, Marshal.GetLastWin32Error()); // ERROR_MOD_NOT_FOUND
Assert.Equal(Architecture.X86, RuntimeInformation.ProcessArchitecture);
Copy link
Member

Choose a reason for hiding this comment

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

Should we keep this assert or is it unstable as well?

Copy link
Member Author

Choose a reason for hiding this comment

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

I was hoping that 126 (ERROR_MOD_NOT_FOUND) is the only error we can get from LoadLibrary, but from the log that you have provided it seems that this syscall can fail with other errors as well.

FWIW in this particular case it failed with ERROR_INSUFFICIENT_BUFFER (The data area passed to a system call is too small) while we don't pass any buffers to the syscall (assuming that we don't threat the valid path as buffer)

Copy link
Member

Choose a reason for hiding this comment

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

Just to clarify, I was referring to the Assert from line 40

@@ -36,9 +36,7 @@ public void LongModuleFileNamesAreSupported()
IntPtr moduleHandle = Interop.Kernel32.LoadLibrary(longNamePath);
if (moduleHandle == IntPtr.Zero)
{
Assert.Equal(126, Marshal.GetLastWin32Error()); // ERROR_MOD_NOT_FOUND
Copy link
Member

Choose a reason for hiding this comment

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

could we bring back this assert in 7.0 and expand it to handle 122 (the error in helix)?

@lewing
Copy link
Member

lewing commented Aug 17, 2021

dotnet-linker-tests (Build Browser wasm release Runtime_Release) and runtime-dev-innerloop (Build Linux x64 release SourceBuild) are nuget restore issues.

@adamsitnik adamsitnik merged commit 24971b0 into dotnet:main Aug 17, 2021
@adamsitnik adamsitnik deleted the issue57452again branch August 17, 2021 20:48
@adamsitnik adamsitnik added this to the 6.0.0 milestone Aug 18, 2021
@ghost ghost locked as resolved and limited conversation to collaborators Sep 17, 2021
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Test failure in System.Diagnostics.Process.Tests
3 participants