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

Load Using GetModuleHandle on win32 and RTLD_NOLOAD on Unix #128

Closed
cwfitzgerald opened this issue Jun 20, 2022 · 7 comments · Fixed by #140
Closed

Load Using GetModuleHandle on win32 and RTLD_NOLOAD on Unix #128

cwfitzgerald opened this issue Jun 20, 2022 · 7 comments · Fixed by #140
Assignees
Labels

Comments

@cwfitzgerald
Copy link

cwfitzgerald commented Jun 20, 2022

Related issue gfx-rs/wgpu#2793

Renderdoc is very particular about how it is loaded - particularly it cannot be loaded if there is no renderdoc client running in the process. This hasn't been a problem for the most part as librenderdoc is not available in the dynamic library search path on most systems, but gfx-rs/wgpu#2789 is an example where it is. I asked baldurk about this and he pointed out we must use the RTLD_NOLOAD flag to dlopen on unix and GetModuleHandle on windows.

See the documentation here for more info: https://renderdoc.org/docs/in_application_api.html

The important bit:

To do this you’ll use your platforms dynamic library functions to see if the library is open already - e.g. GetModuleHandle on Windows, or dlopen with the RTLD_NOW | RTLD_NOLOAD flags if available on *nix systems. On most platforms you can just search for the module name - renderdoc.dll on Windows, or librenderdoc.so on Linux, or libVkLayer_GLES_RenderDoc.so on Android should be sufficient here, so you don’t need to know the path to where RenderDoc is running from.

In libloading terms:

On windows this should call https://docs.rs/libloading/latest/libloading/os/windows/struct.Library.html#method.open_already_loaded which is GetModuleHandle.

On linux this should call https://docs.rs/libloading/latest/libloading/os/unix/struct.Library.html#method.open with RTLD_NOW | RTLD_NOLOAD - the latter needs to be added to lib loading, but it's just an integer so we can just specify the value until lib loading adds it.

@Jengamon
Copy link

So I looked, and as of 0.7.3, libloading only supports the RTLD_NOW flag, and doesn't expose the RTLD_NOLOAD flag, so it might need to be filed there too.

For future peeps:
Relevant line of code is here:
https://github.com/ebkalderon/renderdoc-rs/blob/master/src/version.rs#L80

Instead of using the cross platform libloading::Library::new, one should maybe delegate to a conditionally compiled function that returns libloading::os::<insert os>::Library or an error, and using the functions from the first comment (once RTLD_NOLOAD is supported by libloading ofc). Maybe, I dunno, haven't tested this design...

@cwfitzgerald
Copy link
Author

Yeah we should file an issue there, but in the mean time the things in libloading are just constants of type c_int, so we can add whatever flag we want there by just finding the right number.

@Jengamon
Copy link

Jengamon commented Jun 22, 2022

Found it?
https://docs.rs/libc/latest/libc/constant.RTLD_NOLOAD.html

Apparently it's 0x4, so either hardcode that, or add a dep on libc as we wait for libloading to get this constant (which is a bit much for such a small change).

@Jengamon
Copy link

Please test my fork at https://github.com/Jengamon/renderdoc-rs, as I think this should properly load the library, but I can't test it rn as I'm currently on a Mac.

@cwfitzgerald
Copy link
Author

I can likely test sometime this week, I'm also currently on a Mac lol

@hw762
Copy link

hw762 commented Jul 7, 2022

Is there any instructions how to test this?

@ebkalderon
Copy link
Owner

I'm very sorry for the long-time radio silence on this! Thanks for reporting this issue. Somehow, I missed this when reading through the documentation initially. I'm glad you've managed to work around this upstream in wgpu, though. I've picked this project back up and am slowly addressing all the major bugs and shortcomings in the library, including this bug. Expect a pull request to be up shortly!

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

Successfully merging a pull request may close this issue.

4 participants