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

Support loading dynamic libraries from multiple paths and try more paths in default loader #1864

Open
wants to merge 1 commit into
base: master
Choose a base branch
from
Open
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
83 changes: 55 additions & 28 deletions vulkano/src/instance/loader.rs
Original file line number Diff line number Diff line change
Expand Up @@ -79,31 +79,49 @@ pub struct DynamicLibraryLoader {
}

impl DynamicLibraryLoader {
/// Tries to load the dynamic library at the given path, and tries to
/// load `vkGetInstanceProcAddr` in it.
/// Tries to load the dynamic library at the given paths, using the first successfully loaded,
/// and tries to load `vkGetInstanceProcAddr` in it.
///
/// # Safety
///
/// - The dynamic library must be a valid Vulkan implementation.
///
pub unsafe fn new<P>(path: P) -> Result<DynamicLibraryLoader, LoadingError>
pub unsafe fn new<P>(paths: &[P]) -> Result<DynamicLibraryLoader, LoadingError>
where
P: AsRef<Path>,
{
let vk_lib = shared_library::dynamic_library::DynamicLibrary::open(Some(path.as_ref()))
.map_err(LoadingError::LibraryLoadFailure)?;

let get_proc_addr = {
let ptr: *mut c_void = vk_lib
.symbol("vkGetInstanceProcAddr")
.map_err(|_| LoadingError::MissingEntryPoint("vkGetInstanceProcAddr".to_owned()))?;
mem::transmute(ptr)
};

Ok(DynamicLibraryLoader {
vk_lib,
get_proc_addr,
})
let mut errors = Vec::new();
let vk_lib = paths.iter().find_map(|path| {
shared_library::dynamic_library::DynamicLibrary::open(Some(path.as_ref()))
.or_else(|e| {
errors.push(e.clone());
Err(e)
})
.ok()
});

match vk_lib {
Some(dl) => {
let get_proc_addr = {
let ptr: *mut c_void = dl
.symbol("vkGetInstanceProcAddr")
// Arguably we could continue trying subsequent paths here, but we don't
// expect to find a matching filename without the right contents, so better
// to fail here, especially since an unknown library is now loaded into the
// process.
.map_err(|_| {
LoadingError::MissingEntryPoint("vkGetInstanceProcAddr".to_owned())
})?;
mem::transmute(ptr)
};
Comment on lines +105 to +116
Copy link
Member

Choose a reason for hiding this comment

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

DynamicLibrary seems to implement Drop, so would it be safe to continue attempting paths?
https://docs.rs/shared_library/0.1.9/shared_library/dynamic_library/struct.DynamicLibrary.html#impl-Drop

I don't know much about loading library, so I don't know what is safe and what isn't.


Ok(DynamicLibraryLoader {
vk_lib: dl,
get_proc_addr,
})
}
None => Err(LoadingError::LibraryLoadFailure(errors.join(" "))),
Copy link
Member

Choose a reason for hiding this comment

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

I not sure I am a huge fan of the space separated errors. shared_library isn't very helpful with just returning a String. Perhaps the errors should be a Vec<LoadingError { path: Path, error: String }>?

}
}
}

Expand Down Expand Up @@ -237,23 +255,29 @@ pub fn auto_loader() -> Result<&'static FunctionPointers<Box<dyn Loader>>, Loadi
#[cfg(not(target_os = "ios"))]
fn def_loader_impl() -> Result<Box<dyn Loader>, LoadingError> {
#[cfg(windows)]
fn get_path() -> &'static Path {
Path::new("vulkan-1.dll")
fn get_paths() -> [&'static Path; 1] {
[&Path::new("vulkan-1.dll")]
}
#[cfg(all(unix, not(target_os = "android"), not(target_os = "macos")))]
fn get_path() -> &'static Path {
Path::new("libvulkan.so.1")
fn get_paths() -> [&'static Path; 1] {
[&Path::new("libvulkan.so.1")]
}
#[cfg(target_os = "macos")]
fn get_path() -> &'static Path {
Path::new("libvulkan.1.dylib")
fn get_paths() -> [&'static Path; 5] {
[
&Path::new("libvulkan.1.dylib"),
&Path::new("libvulkan.dylib"),
&Path::new("vulkan.framework/vulkan"),
&Path::new("MoltenVK.framework/MoltenVK"),
&Path::new("libMoltenVK.dylib"),
]
}
#[cfg(target_os = "android")]
fn get_path() -> &'static Path {
Path::new("libvulkan.so")
fn get_paths() -> [&'static Path; 1] {
[&Path::new("libvulkan.so")]
}

let loader = unsafe { DynamicLibraryLoader::new(get_path())? };
let loader = unsafe { DynamicLibraryLoader::new(&get_paths())? };

Ok(Box::new(loader))
}
Expand Down Expand Up @@ -299,7 +323,7 @@ impl fmt::Display for LoadingError {
LoadingError::LibraryLoadFailure(_) => "failed to load the Vulkan shared library",
LoadingError::MissingEntryPoint(_) => {
"one of the entry points required to be supported by the Vulkan implementation \
is missing"
is missing"
}
}
)
Expand All @@ -314,7 +338,10 @@ mod tests {
#[test]
fn dl_open_error() {
unsafe {
match DynamicLibraryLoader::new("_non_existing_library.void") {
match DynamicLibraryLoader::new(&[
"_non_existing_library.void",
"_another_non_existing_library.void",
]) {
Err(LoadingError::LibraryLoadFailure(_)) => (),
_ => panic!(),
}
Expand Down