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

Avoid undefined behaviour of LoadLibrary() on Windows. #553

Merged
merged 1 commit into from
Jan 17, 2017

Conversation

larskanis
Copy link
Member

According to [1] the flag LOAD_WITH_ALTERED_SEARCH_PATH should be used with absolute paths only. It has the effect, that the path of the DLL to be loaded is temporary added to the search path, so that dependent DLLs in the same directory can be found and loaded implicit.

However for relative paths the standard LoadLibrary() search order should be used, because the behaviour of LOAD_WITH_ALTERED_SEARCH_PATH isn't defined for relative paths. In practice (on Windows 10) relative paths to Windows system DLLs do work, so that the library is loaded,
but DLLs in other search paths are not found, when this flag is set.

Ruby's fiddle library uses LoadLibrary without flags in both cases.

[1]
https://msdn.microsoft.com/en-us/library/windows/desktop/ms684179(v=vs.85).aspx

According to [1] the flag LOAD_WITH_ALTERED_SEARCH_PATH
should be used with absolute paths only. It has the effect,
that the path of the DLL to be loaded is temporary added
to the search path, so that dependent DLLs in the same
directory can be found and loaded implicit.

However for relative paths the standard LoadLibrary()
search order should be used, because the behaviour of
LOAD_WITH_ALTERED_SEARCH_PATH isn't defined for relative
paths. In practice (on Windows 10) relative paths to
Windows system DLLs do work, so that the library is loaded,
but DLLs in other search paths are not found, when this
flag is set.

Ruby's fiddle library uses LoadLibrary without flags in
both cases.

[1]
https://msdn.microsoft.com/en-us/library/windows/desktop/ms684179(v=vs.85).aspx
@larskanis
Copy link
Member Author

I've opened the same issue on jnr/jffi#45 for JRuby.

@larskanis
Copy link
Member Author

@headius Can you merge this PR into the release?

@larskanis
Copy link
Member Author

... It's a blocker for many FFI based gems in the new MSYS2 based RubyInstaller environment. See oneclick/rubyinstaller2#4

@tduehr tduehr merged commit cb65f1a into ffi:master Jan 17, 2017
@larskanis
Copy link
Member Author

Thanks @tduehr !

@daniel-rikowski
Copy link

I couldn't get ffi to compile with this change. It looks like an #include <shlwapi.h> is missing. Also the linker cannot resolve PathIsRelativeA (Missing shlwapi library reference?)

I wonder why the tests didn't catch this. Perhaps it is a problem on my machine...

@larskanis
Copy link
Member Author

@daniel-rikowski yes this include is missing. This is addressed in #556 but not yet merged.

@daniel-rikowski
Copy link

Ah, thank you, I didn't see that.

@headius
Copy link
Contributor

headius commented Mar 1, 2017

Hello again! I just merged and released jnr/jffi#45 and will look into this one now.

@headius
Copy link
Contributor

headius commented Mar 1, 2017

Ok I see this has been merged as well but presumably not released.

@headius
Copy link
Contributor

headius commented Mar 1, 2017

@tduehr @enebo @larskanis I don't know who did the win32 binary release last time...ping me and we'll coordinate on getting a new ffi gem out.

@tduehr
Copy link
Member

tduehr commented Mar 2, 2017

@enebo @larskanis @headius It was probably me. It uses a docker image to build it so it's quite easy. I just have to remember to do it. I'll cut a release later today.

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

Successfully merging this pull request may close these issues.

4 participants