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 local (not/temporarily heap allocated) class instances #236

Open
MarijnS95 opened this issue Feb 1, 2022 · 0 comments
Open

Support local (not/temporarily heap allocated) class instances #236

MarijnS95 opened this issue Feb 1, 2022 · 0 comments

Comments

@MarijnS95
Copy link
Contributor

For Traverse-Research/hassle-rs#35

Over at hassle-rs we recently switched our callback handler to be a borrow rather than owned, to convey that it will only be borrowed and used for the duration of the call (ie. not cloned and stored elsewhere). Storing this borrow in a Box was easy to do on the older com-rs library, but is understandably tricky to implement with this crate which forces every class object to live on the heap and outlive the ClassAllocation handle for it.

We realistically want to be able to enforce the same local lifetime our COM object, in addition to storing these borrows in it (which is currently not allowed by the class! macro). If the underlying library (libdxcompiler) that it is passed to copies or otherwise keeps a reference to it, that's an error. As such I've implemented a LocalClassAllocation that pulls the class out of a ClassAllocation and enforces the refcount to be decremented to zero on drop:

https://github.com/Traverse-Research/hassle-rs/blob/5d88ab3f300d5bc947840b3f7d6db588eab01e00/src/wrapper.rs#L167-L215

Perhaps this is something that can be added to com-rs. This solution still requires transmutes to a 'static or unbounded lifetime for its members, but the class! macro could support/detect this and automatically implement a different allocate method (different name, too) that only provides a localized class allocation with the same drop guarantees.

The only downside to this method is that leaking memory - ie ManuallyDrop - is considered safe so the local allocation could be kept alive. Then again, using the raw pointer from the Box<> that would carry the lifetime annotation or shoving it into a C++ function is already unsafe :)

Finally query_interface() should probably return an interface that borrows the local class, making it impossible for the caller to accidentally drop the local allocation (and panic) when it is moved into query_interface(). However, it seems Interface is not implemented for classes, or any other trait that provides these methods? Is there perhaps an option for unification here?

@MarijnS95 MarijnS95 changed the title Support local (not heap allocated) class instances Support local (not/temporarily heap allocated) class instances Feb 1, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

1 participant