Skip to content

Conversation

@MichalStrehovsky
Copy link
Member

@MichalStrehovsky MichalStrehovsky commented May 26, 2022

Finishes the LoadLibraryErrorTracker port from CoreCLR VM to C#.

This allows us to report more details about the reason of DllImport resolution failures at runtime.

The logic matches CoreCLR. There might be a slight behavior difference for empty string literal DllImport/NativeLibrary.Load on Unix-like systems. Not sure we care enough (I couldn't discern what the CoreCLR behavior is - I think we report an arbitrary old message from the last load attempt since we never even try to dlopen the empty string).

Instead of:

System.DllNotFoundException: Unable to load native library 'bruh' or one of its dependencies.

We can now do:

System.DllNotFoundException: Unable to load DLL 'bruh' or one of its dependencies: The process cannot access the file because it is being used by another process.

Contributes to #69743.

Cc @dotnet/ilc-contrib

@ghost
Copy link

ghost commented May 26, 2022

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

Issue Details

Finishes the LoadLibraryErrorTracker port from CoreCLR VM to C#.

This allows us to report more details about the reason of DllImport resolution failures at runtime.

The logic matches CoreCLR. There might be a slight behavior difference for empty string literal DllImport/NativeLibrary.Load. Not sure we care enough (I couldn't discern what the CoreCLR behavior is - I think we report an arbitrary old message from the last load attempt since we never even try to dlopen the empty string).

Instead of:

System.DllNotFoundException: Unable to load native library 'bruh' or one of its dependencies.

We can now do:

System.DllNotFoundException: Unable to load DLL 'bruh' or one of its dependencies: The process cannot access the file because it is being used by another process.

Contributes to #69743.

Cc @dotnet/ilc-contrib

Author: MichalStrehovsky
Assignees: -
Labels:

area-System.Runtime.InteropServices

Milestone: -

Finishes the LoadLibraryErrorTracker port from CoreCLR VM to C#.

This allows us to report more details about the reason of `DllImport` resolution failures at runtime.

The logic matches CoreCLR. There might be a slight behavior difference for empty string literal `DllImport`/`NativeLibrary.Load`. Not sure we care enough (I couldn't discern what the CoreCLR behavior is - I think we report an arbitrary old message from the last load attempt since we never even try to `dlopen` the empty string).

Instead of:

```
System.DllNotFoundException: Unable to load native library 'bruh' or one of its dependencies.
```

We can now do:

```
System.DllNotFoundException: Unable to load DLL 'bruh' or one of its dependencies: The process cannot access the file because it is being used by another process.
```
Copy link
Member

@jkotas jkotas left a comment

Choose a reason for hiding this comment

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

LGTM otherwise. Thank you!

<data name="Arg_DllNotFoundExceptionParameterized" xml:space="preserve">
<value>Unable to load native library '{0}' or one of its dependencies.</value>
<data name="DllNotFound_Windows" xml:space="preserve">
<value>Unable to load DLL '{0}' or one of its dependencies: {1}</value>
Copy link
Member

Choose a reason for hiding this comment

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

Since {1} seems likely to be a sentence, I think it reads better (and perhaps it's more conventional) to separate with a period instead of a colon. For example we do this in the console output for FailFast. Etc

Copy link
Member Author

Choose a reason for hiding this comment

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

This is the existing string from mscorrc:

IDS_EE_NDIRECT_LOADLIB_WIN "Unable to load DLL '%1' or one of its dependencies: %2"
that dates back to .NET Framework. I'll keep it for consistency.

@MichalStrehovsky MichalStrehovsky merged commit e297470 into dotnet:main May 28, 2022
@MichalStrehovsky MichalStrehovsky deleted the errorTracker branch May 28, 2022 05:31
@ghost ghost locked as resolved and limited conversation to collaborators Jun 27, 2022
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.

4 participants