Receiving COM interface pointers as parameters

[Rantanen]

The following use case:

// C++ COM client

CComPtr<ILogic> logic = ...;
CComPtr<IData> data = ...;
logic.DoLogic(data);  // C++ holds reference count. This is essentially "a borrow".

// Rust COM server

impl ILogic for CoLogic {
    fn do_logic(&self, data: *mut c_void) {
        // Safety requirements for InterfacePtr::new.
        // - ptr is valid pointer of 'T'
        // - ptr must live as long as InterfacePtr
        let ptr = unsafe {
            InterfacePtr<IData>::new(data);
        }

        // Rust wants ownership and turns the 'ptr' into a Rc.

        // This should increment reference count (but doesn't currently).
        let rc = InterfaceRc::new(data);

        // This would increment reference count, but is not implemented for InterfacePtr.
        let rc = InterfacePtr::get_interface<IOtherData>(ptr);
    }
}

So couple of needs:

• Provide unsafe "InterfacePtr into InterfaceRc without add_ref" method.
  • Currently InterfaceRc::new does this, but isn't marked as unsafe, thus leading to easy double-release, if the user forgets to manually call add_ref.
• Provide safe "InterfacePtr into InterfaceRc with add_ref" method.
  • As long as InterfaceRc does add_ref, there should be no other safety concerns, assuming the pointer itself is valid (which InterfacePtr seems to imply based on the safety requirements on its new method).
• Implement get_interface on InterfacePtr.
  • Currently the user must turn any pointer into Rc type just to call get_interface

Maybe look into implementing a lifetime scoped variant of InterfacePtr:

#[repr(transparent)]
ScopedInterfacePtr<'a, TInterface> {
    ptr: *mut c_void,
    phantom: &'a PhantomData<TInterface>
}

This should prevent accidentally moving the pointer somewhere that would outlive the function call.

[Rantanen]

Also InterfacePtr probably shouldn't implement Clone, given turning &InterfacePtr into owned InterfacePtr is very unsafe thing to do if the user doesn't call add_ref.

Currently it's used to implement InterfaceRc::clone. One approach around this would be to implement fn as_rc(&self) -> InterfaceRc method on InterfacePtr directly. This way the unsafety of "remember to call add_ref" would be limited to InterfacePtr.

[rylev]

@Rantanen awesome!

I like all of your suggestions, although the only think I'm 100% convinced of is that we need better documentation.

• Provide unsafe "InterfacePtr into InterfaceRc without add_ref" method

  • Currently InterfaceRc::new accepts an InterfacePtr. I believe the assumption is that InterfacePtr points to a COM interface which has a reference count of at least 1. If this is the assumption than InterfaceRc::new as it currently stands is indeed safe and all we need to do is properly document this on the already unsafe InterfacePtr::new.

• Provide safe "InterfacePtr into InterfaceRc with add_ref" method

  • This is probably a good thing to have. Calling add_ref should always be safe (given that the pointer is valid and that add_ref simply adds to the reference count - which is already upheld by other unsafe verification).

• Implement get_interface on InterfacePtr

  • This sounds good, and I believe should always be safe.

I also like the idea of scoped interface ptr! I think such a construct could replace a large part of the uses for InteracePtr.

And lastly I agree that InterfacePtr should not implement Clone we should remove that.

I'd prefer if we either get all of these taken care of in one PR, or separate them out into different issues. Which would you prefer, @Rantanen?

[Rantanen]

It might be easiest to figure out the proper APIs in one go.

Just to clarify the different needs, I'll use Intercom's type system as an example (ignoring the cross-platform type system bits):

• intercom::InterfacePtr<I> is a raw #[repr(C)] COM pointer.
  • Very unsafe
  • Provide some documentative-benefits in method signatures
  • Gives some sort of "don't accidentally mix different interface pointers" safety from the Rust compiler.
• intercom::ComItfis a non-owned COM pointer.
  • This is what I imagined InterfacePtr to be.
  • Makes no assumptions about owning references (ie. it can be used when the caller has passed in a COM pointer that it owns, but the callee hasn't add_ref'd it).
  • Might turn into the Scoped...<'a> type in com-rs (Intercom uses &ComItf in its place, which it can do given it implements an extra conversion function layer)
• intercom::ComRc<I> is a COM interface reference that maintains its own reference count to keep the interface alive.
  • This is essentially exactly what InterfaceRc is.

It's important to note that there are use case for both

• Non-owning wrapper around the COM pointer that implements the interface but doesn't automatically add_ref/release
• Fully blown all-traits-usable COM pointer that does not own its pointer through add_ref/release.

Or well... Technically you can get away with "always add_ref/release" if you don't mind the performance cost of always invoking those functions (possibly over process boundaries or even network).

So based on this, my proposal would be:

• InterfacePtr - A non-owning interface pointer.
  • Assumes the underlying raw pointer is valid (has reference count of at least 1).
• InterfaceRc - An owning interface pointer.
  • Invokes release on drop.
  • Can be constructed from InterfacePtr in a safe manner (which invokes add_ref)
  • Can be constructed from InterfacePtr in an unsafe manner (which does not invoke add_ref)
    • This isn't safe even if InterfacePtr assumes the reference count is at least one. The reference might be held by the caller, which is prepared to release it. If such InterfacePtr is given to a InterfaceRc, the InterfaceRc will end up releasing the pointer even if it was never their pointer to release.

And now that I'm thinking back, given InterfacePtr is essentially just a new type around a pointer, I believe you could even make InterfacePtr into unsized type and always handle it by reference (something very similar to &str in stdlib).

#[repr(transparent)]
struct InterfacePtr<I>(c_void, PhantomData<I>);

impl<I> InterfacePtr<I> {
    fn new(source: *const c_void) -> &InterfacePtr<i> { ... }
}

Example in Playground

Though you might want to double check the soundness of this. I know similar bits are already used in std libs when turning *const c_char directly into &CStr.

Edit: The pointer type should be #[repr(transparent)] to make this safe