nagisa / rust_libloading

Bindings around the platform's dynamic library loading primitives with greatly improved memory safety.
https://docs.rs/libloading
ISC License
1.24k stars 102 forks source link

OwnedSymbol #104

Open constvoidptr opened 2 years ago

constvoidptr commented 2 years ago

I needed the ability the cache multiple Symbols in a struct across FFI calls.

I didn't want to use a self referental struct, so instead I created a OwnedSymbol type that keeps the library alive as long as needed.

pub struct OwnedSymbol<T> {
    inner: imp::Symbol<T>,
    library: Arc<Library>,
}

// Obtain a OwnedSymbol
impl Library {
  pub unsafe fn get_owned<T>(self: Arc<Self>, symbol: &[u8]) -> Result<OwnedSymbol<T>, Error> {
      self.0
          .get(symbol)
          .map(|from| OwnedSymbol::from_raw(from, self))
  }
}

https://github.com/constvoidptr/rust_libloading/blob/768dc11b4fa3d48527a6406d56e3100c7f49a376/src/safe.rs#L308

Is that something you'd like to see upstream? I've seen a few issues/pr's that reference something similar. If so, I would document the new type and functions, write the tests and get a pull request going.

nagisa commented 2 years ago

I have seen multiple different use-cases asking for symbols associated with a library instance, but nothing quite like this. Most of the use-cases involve collecting the Library together with a number of Symbols that come from it. As an example, this is something bindgen can generate code for.

As it pertains to this specific proposal, it isn’t clear to me if Arc is the appropriate reference counting type in all instances, nor that it is the right way to associate the library and the loaded symbol in 100% of the instances. It might well be the case that people might want to use Rc instead, or it might be the case that leaving the reference counting up to the system/loader is more efficient or appropriate.

With that in mind, I’m not sure if libloading is the right place for this sort of type, at least not without an extensive analysis of the problem and solution space. In the meantime, libloading does provide the tools to build structs like these outside of libloading via libloading::os::*::{Library,Symbol}. The os-specific Symbols don’t relate to the containing Library via lifetime, which should help constructing such types much easier.

As an alternative to adding this type to libloading proper, what I could see is making this repository a workspace and start adding various libloading-* crates implementing experiments such as the type proposed in this issue.