diff --git a/src/lib.rs b/src/lib.rs index f6fc58d..21912b8 100644 --- a/src/lib.rs +++ b/src/lib.rs @@ -199,6 +199,8 @@ impl Library { /// You only need to call this if you are interested in handling any errors that may arise when /// library is unloaded. Otherwise the implementation of `Drop` for `Library` will close the /// library and ignore the errors were they arise. + /// + /// The underlying data structures may still get leaked if an error does occur. pub fn close(self) -> Result<(), Error> { self.0.close() } diff --git a/src/os/unix/mod.rs b/src/os/unix/mod.rs index 71d2c5f..83657a9 100644 --- a/src/os/unix/mod.rs +++ b/src/os/unix/mod.rs @@ -301,6 +301,8 @@ impl Library { /// /// You only need to call this if you are interested in handling any errors that may arise when /// library is unloaded. Otherwise this will be done when `Library` is dropped. + /// + /// The underlying data structures may still get leaked if an error does occur. pub fn close(self) -> Result<(), crate::Error> { let result = with_dlerror(|desc| crate::Error::DlClose { desc }, || { if unsafe { dlclose(self.handle) } == 0 { @@ -309,6 +311,9 @@ impl Library { None } }).map_err(|e| e.unwrap_or(crate::Error::DlCloseUnknown)); + // While the library is not free'd yet in case of an error, there is no reason to try + // dropping it again, because all that will do is try calling `FreeLibrary` again. only + // this time it would ignore the return result, which we already seen failing… std::mem::forget(self); result } diff --git a/src/os/windows/mod.rs b/src/os/windows/mod.rs index a106234..da1b5c9 100644 --- a/src/os/windows/mod.rs +++ b/src/os/windows/mod.rs @@ -217,14 +217,24 @@ impl Library { } /// Unload the library. + /// + /// You only need to call this if you are interested in handling any errors that may arise when + /// library is unloaded. Otherwise this will be done when `Library` is dropped. + /// + /// The underlying data structures may still get leaked if an error does occur. pub fn close(self) -> Result<(), crate::Error> { - with_get_last_error(|source| crate::Error::FreeLibrary { source }, || { + let result = 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)) + }).map_err(|e| e.unwrap_or(crate::Error::FreeLibraryUnknown)); + // While the library is not free'd yet in case of an error, there is no reason to try + // dropping it again, because all that will do is try calling `FreeLibrary` again. only + // this time it would ignore the return result, which we already seen failing… + std::mem::forget(self); + result } } diff --git a/tests/functions.rs b/tests/functions.rs index 20ac3c1..e899344 100644 --- a/tests/functions.rs +++ b/tests/functions.rs @@ -145,6 +145,30 @@ fn test_static_ptr() { } } +#[test] +// Something about i686-pc-windows-gnu, makes dll initialization code call abort when it is loaded +// and unloaded many times. So far it seems like an issue with mingw, not libloading, so ignoring +// the target. Especially since it is very unlikely to be fixed given the state of support its +// support. +#[cfg(not(all(target_arch="x86", target_os="windows", target_env="gnu")))] +fn manual_close_many_times() { + make_helpers(); + let join_handles: Vec<_> = (0..16).map(|_| { + std::thread::spawn(|| unsafe { + for _ in 0..10000 { + let lib = Library::new(LIBPATH).expect("open library"); + let _: Symbol u32> = + lib.get(b"test_identity_u32").expect("get fn"); + lib.close().expect("close is successful"); + } + }) + }).collect(); + for handle in join_handles { + handle.join().expect("thread should succeed"); + } +} + + #[cfg(unix)] #[test] fn library_this_get() {