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 use incompatible LOAD_LIBRARY_SEARCH flags when using LOAD_WITH_ALTERED_SEARCH_PATH #111990

Open
wants to merge 3 commits into
base: main
Choose a base branch
from

Conversation

elinor-fung
Copy link
Member

@elinor-fung elinor-fung commented Jan 30, 2025

LoadLibraryEx documents that the LOAD_WITH_ALTERED_SEARCH_PATH flag is incompatible with LOAD_LIBRARY_SEARCH flags. In cases where we explicitly add this flag (absolute path or looking in p/invoke assembly directory), we could end up specifying an incompatible combination if other DllImportSearchPath flags corresponding to the LOAD_LIBRARY_SEARCH flags were also specified. This change avoids using those flags in when the runtime specifies loading with an altered search path.

Fixes #111825

@elinor-fung elinor-fung force-pushed the alteredSearchPath-mask branch from b3cc511 to 5795fbd Compare January 30, 2025 04:44
@elinor-fung elinor-fung force-pushed the alteredSearchPath-mask branch from 5795fbd to e612989 Compare January 30, 2025 05:34
@AaronRobinsonMSFT
Copy link
Member

This change avoids using those flags in when the runtime specifies loading with an altered search path.

Should we reflect this in documentation somewhere? I'm also interested in if this is break change worthy? Was anyone being successful prior to this change?

@elinor-fung
Copy link
Member Author

Should we reflect this in documentation somewhere?

Where were you thinking? I see this as implementation detail of how the runtime uses the public/documented OS API LoadLibraryEx (and we were using it in a way that is documented as incorrect).

I'm also interested in if this is break change worthy? Was anyone being successful prior to this change?

I didn't find anything that would make this a breaking change. Without this change, if the library being loaded didn't need to load another native dependency, it could be loaded - that remains true with this change. If the library isn't in the assembly directory, we try to load with the other specified flags just as before.

@AaronRobinsonMSFT
Copy link
Member

Where were you thinking? I see this as implementation detail of how the runtime uses the public/documented OS API LoadLibraryEx (and we were using it in a way that is documented as incorrect).

That is a fair point and the question was sort of related to my second. I agree this is an incorrect usage so we fixed that, yay. It is really around is anyone going to be surprised that we are now filtering out these settings? Which really means was anyone previously being successful, which sounds like they weren't.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
2 participants