cleanup

crates/libs/core/src/com_object.rs

@@ -1,37 +1,54 @@
 use crate::{AsImpl, IUnknown, IUnknownImpl, Interface, InterfaceRef};
-use core::ffi::c_void;
-use core::mem::ManuallyDrop;
 use core::ptr::NonNull;
 use std::borrow::Borrow;
 
-// This is implemented on user types that are marked with #[implement].
+/// Identifies types that can be placed in `ComObject`.
+///
+/// The `#[implement]` macro generates implementations of this trait.
+///
+/// This trait is an implementation detail of the Windows crates.
+/// User code should not deal directly with this trait.
 #[allow(missing_docs)]
 pub unsafe trait ComImpl {
-    // The generated <foo>_Impl type (aka the "boxed" type)
-    type Impl: IUnknownImpl<Impl = Self>;
+    // The generated <foo>_Impl type (aka the "boxed" type or "outer" type)
+    type Outer: IUnknownImpl<Impl = Self>;
 }
 
 /// Describes the COM interfaces that a specific ComObject implements.
-/// This trait is implemented by ComObject implementation obejcts (e.g. `MyApp_Impl`).
+/// This trait is implemented by ComObject implementation object (e.g. `MyApp_Impl`).
+///
+/// The `#[implement]` macro generates implementations of this trait.
+///
+/// This trait is an implementation detail of the Windows crates.
+/// User code should not deal directly with this trait.
 pub trait ComObjectInterface<I: Interface> {
     /// Gets a borrowed interface on the ComObject.
-    fn get_interface(&self) -> InterfaceRef<'_, I>;
+    fn as_interface(&self) -> InterfaceRef<'_, I>;
 }
 
 /// A counted pointer to a type that implements COM interfaces, where the object has been
 /// placed in the heap (boxed).
 ///
 /// This type exists so that you can place an object into the heap and query for COM interfaces,
 /// without losing the safe reference to the implementation object.
+///
+/// Because the pointer inside this type is known to be non-null, `Option<ComObject<T>>` should
+/// always have the same size as a single pointer.
+///
+/// # Safety
+///
+/// The contained `ptr` field is an owned, reference-counted pointer to a _pinned_ Pin<Box<T::Outer>>.
+/// Although this code does not currently use `Pin<T>`,
+#[repr(transparent)]
 pub struct ComObject<T: ComImpl> {
-    ptr: NonNull<T::Impl>,
+    ptr: NonNull<T::Outer>,
 }
 
 impl<T: ComImpl> ComObject<T> {
     /// Allocates a heap cell (box) and moves `obj` into it. Returns a counted pointer to `obj`.
     pub fn new(value: T) -> Self {
         unsafe {
-            let box_ = T::Impl::new_box(value);
+            let box_ = T::Outer::new_box(value);
             Self { ptr: NonNull::new_unchecked(Box::into_raw(box_)) }
         }
     }
@@ -48,85 +65,96 @@ impl<T: ComImpl> ComObject<T> {
 
     /// Gets a reference to the shared object's heap box.
     #[inline(always)]
-    pub fn get_box(&self) -> &T::Impl {
+    pub fn get_box(&self) -> &T::Outer {
         unsafe { self.ptr.as_ref() }
     }
 
     // Note that we _do not_ provide a way to get a mutable reference to the outer box.
-    // It's ok to return &mut T, but not &mut T::Impl. That would allow someone to replace the
+    // It's ok to return &mut T, but not &mut T::Outer. That would allow someone to replace the
     // contents of the entire object (box and reference count), which could lead to UB.
-    // This could maybe be solved by returning Pin<&mut T::Impl>, but that requires some
+    // This could maybe be solved by returning Pin<&mut T::Outer>, but that requires some
     // additional thinking.
 
     /// Gets a mutable reference to the object stored in the box, if the reference count
     /// is exactly 1. If there are multiple references to this object then this returns `None`.
     #[inline(always)]
     pub fn get_mut(&mut self) -> Option<&mut T> {
-        unsafe {
-            let count = self.ptr.as_ref().count();
-            if count.is_one() {
-                Some(self.ptr.as_mut().get_impl_mut())
-            } else {
-                None
-            }
+        if self.is_reference_count_one() {
+            // SAFETY: We must only return &mut T, *NOT* &mut T::Outer.
+            // Returning T::Outer would allow swapping the contents of the object, which would
+            // allow (incorrectly) modifying the reference count.
+            unsafe { Some(self.ptr.as_mut().get_impl_mut()) }
+        } else {
+            None
         }
     }
 
+    /// Returns `true` if this reference is the only reference to the `ComObject`.
+    #[inline(always)]
+    pub fn is_exclusive_reference(&self) -> bool {
+        self.get_box().is_reference_count_one()
+    }
+
     /// If this object has only a single object reference (i.e. this `ComObject` is the only
     /// reference to the heap allocation), then this method will extract the inner `T`
     /// (and return it in an `Ok`) and then free the heap allocation.
     ///
     /// If there is more than one reference to this object, then this returns `Err(self)`.
     #[inline(always)]
     pub fn try_take(self) -> Result<T, Self> {
-        unsafe {
-            let count = self.ptr.as_ref().count();
-            if count.is_one() {
-                let ptr = self.ptr;
-                core::mem::forget(self);
-                Ok(T::Impl::extract_inner(ptr))
-            } else {
-                Err(self)
-            }
+        if self.is_exclusive_reference() {
+            let outer_box: Box<T::Outer> = unsafe { core::mem::transmute(self) };
+            Ok(outer_box.into_inner())
+        } else {
+            Err(self)
         }
     }
 
     /// Casts to a given interface type.
+    ///
+    /// This always performs a `QueryInterface`, even if `T` is known to implement `I`.
+    /// If you know that `T` implements `I`, then use `as_interface` or `to_interface` because
+    /// those functions do not require a dynamic `QueryInterface` call.
     #[inline(always)]
-    pub fn cast<J: Interface>(&self) -> windows_core::Result<J> {
-        unsafe {
-            let unknown_ptr = self.ptr.as_ref().iunknown_ptr();
-            let unknown: ManuallyDrop<IUnknown> = core::mem::transmute(unknown_ptr);
-            unknown.cast()
-        }
+    pub fn cast<I: Interface>(&self) -> windows_core::Result<I>
+    where
+        T::Outer: ComObjectInterface<IUnknown>,
+    {
+        let unknown = self.as_interface::<IUnknown>();
+        unknown.cast()
     }
 
     /// Gets a borrowed reference to an interface that is implemented by this ComObject.
     ///
     /// The returned reference does not have an additional reference count.
     /// You can AddRef it by calling to_owned().
+    #[inline(always)]
     pub fn as_interface<I: Interface>(&self) -> InterfaceRef<'_, I>
     where
-        T::Impl: ComObjectInterface<I>,
+        T::Outer: ComObjectInterface<I>,
     {
-        self.get_box().get_interface()
+        self.get_box().as_interface()
     }
 
     /// Gets an owned (counted) reference to an interface that is implemented by this ComObject.
+    #[inline(always)]
     pub fn to_interface<I: Interface>(&self) -> I
     where
-        T::Impl: ComObjectInterface<I>,
+        T::Outer: ComObjectInterface<I>,
     {
         self.as_interface::<I>().to_owned()
     }
 
     /// Converts this `ComObject` into an interface that it implements.
+    ///
+    /// This does not need to adjust reference counts because `self` is consumed.
+    #[inline(always)]
     pub fn into_interface<I: Interface>(self) -> I
     where
-        T::Impl: ComObjectInterface<I>,
+        T::Outer: ComObjectInterface<I>,
     {
         unsafe {
-            let raw: *mut c_void = self.get_box().get_interface().as_raw();
+            let raw = self.get_box().as_interface().as_raw();
             core::mem::forget(self);
             I::from_raw(raw)
         }
@@ -142,7 +170,7 @@ impl<T: ComImpl + Default> Default for ComObject<T> {
 impl<T: ComImpl> Drop for ComObject<T> {
     fn drop(&mut self) {
         unsafe {
-            T::Impl::Release(self.ptr.as_ptr());
+            T::Outer::Release(self.ptr.as_ptr());
         }
     }
 }
@@ -168,7 +196,7 @@ where
 }
 
 impl<T: ComImpl> core::ops::Deref for ComObject<T> {
-    type Target = T::Impl;
+    type Target = T::Outer;
 
     #[inline(always)]
     fn deref(&self) -> &Self::Target {
@@ -196,7 +224,7 @@ impl<T: ComImpl + core::hash::Hash> core::hash::Hash for ComObject<T> {
 
 // If T is Send (or Sync) then the ComObject<T> is also Send (or Sync).
 // Since the actual object storage is in the heap, the object is never moved.
-unsafe impl<T: ComImpl + Sync> Send for ComObject<T> {}
+unsafe impl<T: ComImpl + Send> Send for ComObject<T> {}
 unsafe impl<T: ComImpl + Sync> Sync for ComObject<T> {}
 
 impl<T: ComImpl + PartialEq> PartialEq for ComObject<T> {

crates/libs/core/src/imp/weak_ref_count.rs

@@ -16,13 +16,9 @@ impl WeakRefCount {
         self.0.fetch_update(Ordering::Relaxed, Ordering::Relaxed, |count_or_pointer| bool::then_some(!is_weak_ref(count_or_pointer), count_or_pointer + 1)).map(|u| u as u32 + 1).unwrap_or_else(|pointer| unsafe { TearOff::decode(pointer).strong_count.add_ref() })
     }
 
+    #[inline(always)]
     pub fn is_one(&self) -> bool {
-        let count_or_pointer = self.0.load(Ordering::Acquire);
-        count_or_pointer == 1
-    }
-
-    pub fn poll(&self) -> isize {
-        self.0.load(Ordering::Acquire)
+        self.0.load(Ordering::Acquire) == 1
     }
 
     pub fn release(&self) -> u32 {

crates/libs/core/src/unknown.rs

@@ -1,5 +1,3 @@
-use self::imp::WeakRefCount;
-
 use super::*;
 use core::ffi::c_void;
 use core::ptr::NonNull;
@@ -63,23 +61,24 @@ impl std::fmt::Debug for IUnknown {
 
 /// The `#[implement]` macro generates implementations of this trait for the types
 /// that it generates, e.g. `MyApp_Impl`,
+///
+/// `ComObject` uses this trait to interact with boxed COM objects.
 #[doc(hidden)]
 pub trait IUnknownImpl {
-    /// The user type, e.g. `MyApp`.
+    /// The contained user type, e.g. `MyApp`. Also known as the "inner" type.
     type Impl;
 
     /// Initializes a new heap box and returns it.
     fn new_box(value: Self::Impl) -> Box<Self>;
 
     /// Get a reference to the backing implementation.
     fn get_impl(&self) -> &Self::Impl;
-    fn get_impl_mut(&mut self) -> &mut Self::Impl;
-    fn count(&self) -> &WeakRefCount;
 
-    unsafe fn extract_inner(ptr: NonNull<Self>) -> Self::Impl;
+    /// Get a mutable reference to the contained (inner) object.
+    fn get_impl_mut(&mut self) -> &mut Self::Impl;
 
-    /// Gets the pointer to IUnknown.  This _does not_ increase the reference count.
-    unsafe fn iunknown_ptr(&self) -> NonNull<c_void>;
+    /// Consumes the box and returns the contained (inner) object. This is the opposite of `new_box`.
+    fn into_inner(self) -> Self::Impl;
 
     /// The classic `QueryInterface` method from COM.
     ///
@@ -98,8 +97,16 @@ pub trait IUnknownImpl {
     ///
     /// This function should only be called when the interface pointer is no longer used as calling `Release`
     /// on a non-aliased interface pointer and then using that interface pointer may result in use after free.
+    ///
+    /// This function takes `*mut Self` because the object may be freed by the time this method returns.
+    /// Taking `&self` would violate Rust's rules on reference lifetime.
     unsafe fn Release(self_: *mut Self) -> u32;
 
+    unsafe fn destroy(self_: *mut Self);
+
+    /// Returns `true` if the reference count of the box is equal to 1.
+    fn is_reference_count_one(&self) -> bool;
+
     /// Gets the trust level of the current object.
     unsafe fn GetTrustLevel(&self, value: *mut i32) -> HRESULT;
 }