Hi!
I've been working together with someone on a PR to an FFI wrapper crate I maintain to make use of the new bindgen feature that generates libloading
boilerplate, enabling us to load the library dynamically at runtime.
Everything is working well on both Windows and Linux except for the following:
error: test failed, to rerun pass '--lib'
Caused by:
process didn't exit successfully: `C:\Users\cldfi\Desktop\programming\nvml-wrapper\target\debug\deps\nvml_wrapper-f40d6d390aa3bfdd.exe` (exit code: 0xc0000005, STATUS_ACCESS_VIOLATION)
I'm getting a STATUS_ACCESS_VIOLATION
on Windows when running the test suite locally in parallel (via cargo test --features test-local
). I found that if I commented out this test function the test suite ran successfully. I also found that running the test suite with --test-threads 1
fixed the issue.
That test function is calling NVML::shutdown
, defined as follows (with my local changes, this isn't in the repo atm):
pub fn shutdown(mut self) -> Result<(), NvmlError> {
let sym = nvml_sym(self.lib.nvmlShutdown.as_ref())?;
unsafe {
nvml_try(sym())?;
}
// SAFETY: we `mem::forget(self)` after this, so `self.lib` won't get
// touched by our `Drop` impl
let lib = unsafe { ManuallyDrop::take(&mut self.lib) };
lib.__library.close()?;
mem::forget(self);
Ok(())
}
Where NVML
(self
) is defined as:
pub struct NVML {
lib: ManuallyDrop<NvmlLib>,
}
impl Drop for NVML {
fn drop(&mut self) {
unsafe {
self.lib.nvmlShutdown();
// SAFETY: called after the last usage of `self.lib`
ManuallyDrop::drop(&mut self.lib);
}
}
}
and NvmlLib
is the wrapper type bindgen has generated over top of libloading::Library
.
I double-checked my implementation of NVML::shutdown
, and as far as I can tell it looks good. So, I went and took a look at libloading::Library::close
, which on Windows calls this function:
pub fn close(self) -> Result<(), crate::Error> {
with_get_last_error(|source| crate::Error::FreeLibrary { source }, || {
if unsafe { libloaderapi::FreeLibrary(self.0) == 0 } {
None
} else {
Some(())
}
}).map_err(|e| e.unwrap_or(crate::Error::FreeLibraryUnknown))
}
immediately under which is this Drop
impl:
impl Drop for Library {
fn drop(&mut self) {
unsafe { libloaderapi::FreeLibrary(self.0); }
}
}
It looks to me as if there's a double free going on here; libloaderapi::FreeLibrary
is going to get called twice when calling os::windows::Library::close
because there's no mem::forget(self)
involved. The equivalent function on the linux side does do a mem::forget(self)
(and I didn't experience this issue on linux).
I tried putting together a small reproduction:
fn main() {
for _ in 0..100 {
std::thread::spawn(|| {
let lib = libloading::Library::new("nvml.dll").unwrap();
let _: libloading::Symbol<unsafe extern "C" fn() -> u32> = unsafe { lib.get(b"nvmlInit").unwrap() };
lib.close().unwrap();
});
}
}
I wasn't able to get a STATUS_ACCESS_VIOLATION
out of this, but it does return random quantities of errors when running it:
PS C:\Users\cldfi\Desktop\programming\rust_libloading> cargo run --release --example quick
Finished release [optimized] target(s) in 0.01s
Running `target\release\examples\quick.exe`
thread '<unnamed>' panicked at 'called `Result::unwrap()` on an `Err` value: GetProcAddress { source: Os { code: 87, kind: InvalidInput, message: "The parameter is incorrect." } }', examples\quick.rs:5:102
note: run with `RUST_BACKTRACE=1` environment variable to display a backtrace
thread '<unnamed>' panicked at 'called `Result::unwrap()` on an `Err` value: GetProcAddress { source: Os { code: 87, kind: InvalidInput, message: "The parameter is incorrect." } }', examples\quick.rs:5:102
thread 'thread 'thread 'thread '<unnamed><unnamed><unnamed><unnamed>' panicked at '' panicked at '' panicked at '' panicked at 'called `Result::unwrap()` on an `Err` value: GetProcAddress { source: Os { code: 87, kind: InvalidInput, message: "The parameter is incorrect."
} }called `Result::unwrap()` on an `Err` value: GetProcAddress { source: Os { code: 87, kind: InvalidInput, message: "The parameter is incorrect." } }called `Result::unwrap()` on an `Err` value: GetProcAddress { source: Os { code: 87, kind: InvalidInput, message: "The parameter is incorrect." } }called `Result::unwrap()` on an `Err` value: GetProcAddress { source: Os { code: 87, kind: InvalidInput, message: "The parameter is incorrect." } }', ', ', ', examples\quick.rsexamples\quick.rsexamples\quick.rsexamples\quick.rs::::5555::::102102102
PS C:\Users\cldfi\Desktop\programming\rust_libloading>
Adding a mem::forget(self)
to the end of os::windows::Library::close
seems to eliminate those errors for me:
PS C:\Users\cldfi\Desktop\programming\rust_libloading> cargo run --release --example quick
Finished release [optimized] target(s) in 0.01s
Running `target\release\examples\quick.exe`
PS C:\Users\cldfi\Desktop\programming\rust_libloading>
and it also fixed the STATUS_ACCESS_VIOLATION
when running my crate's test suite in parallel.
tl;dr, am I doing something wrong or is there a missing mem::forget(self)
in os::windows::Library::close
?