From c8f17bbcfb9ab88c7844cd1541de77b423faa401 Mon Sep 17 00:00:00 2001
From: Mads Marquart <mads@marquart.dk>
Date: Sun, 29 Sep 2024 11:50:05 +0200
Subject: [PATCH] Avoid using ClassType in class_with_lifetime example

E.g. `as_super` is unsound, since it allows retaining the returned
object past the lifetime of the original.
---
 crates/objc2/examples/class_with_lifetime.rs | 60 +++++++++-----------
 crates/objc2/src/rc/retained.rs              |  3 +-
 crates/objc2/src/top_level_traits.rs         | 12 ++--
 3 files changed, 33 insertions(+), 42 deletions(-)

diff --git a/crates/objc2/examples/class_with_lifetime.rs b/crates/objc2/examples/class_with_lifetime.rs
index 5f4e19479..dfc83d90b 100644
--- a/crates/objc2/examples/class_with_lifetime.rs
+++ b/crates/objc2/examples/class_with_lifetime.rs
@@ -7,18 +7,21 @@ use std::sync::Once;
 
 use objc2::msg_send_id;
 use objc2::rc::Retained;
-use objc2::runtime::{AnyClass, ClassBuilder, NSObject};
+use objc2::runtime::{AnyClass, AnyObject, ClassBuilder, NSObject};
 use objc2::{ClassType, Encoding, Message, RefEncode};
 
-// The type of the instance variable that we want to store
+/// The type of the instance variable that we want to store
 type Ivar<'a> = &'a Cell<u8>;
 
 /// Struct that represents our custom object.
 #[repr(C)]
 struct MyObject<'a> {
-    // Required to give MyObject the proper layout
-    superclass: NSObject,
-    // For auto traits and variance
+    /// Required to give MyObject the proper layout
+    ///
+    /// Beware: `retain`-ing this field directly is dangerous, since it'd make
+    /// it possible to extend the lifetime of the object beyond lifetime `'a`.
+    inner: AnyObject,
+    /// For auto traits and variance
     p: PhantomData<Ivar<'a>>,
 }
 
@@ -29,6 +32,21 @@ unsafe impl RefEncode for MyObject<'_> {
 unsafe impl Message for MyObject<'_> {}
 
 impl<'a> MyObject<'a> {
+    fn class() -> &'static AnyClass {
+        // NOTE: Use std::lazy::LazyCell if in MSRV
+        static REGISTER_CLASS: Once = Once::new();
+
+        REGISTER_CLASS.call_once(|| {
+            let superclass = NSObject::class();
+            let mut builder = ClassBuilder::new(c"MyObject", superclass).unwrap();
+
+            builder.add_ivar::<Ivar<'_>>(c"number");
+
+            let _cls = builder.register();
+        });
+        AnyClass::get(c"MyObject").unwrap()
+    }
+
     fn new(number: &'a mut u8) -> Retained<Self> {
         // SAFETY: The instance variable is initialized below.
         let this: Retained<Self> = unsafe { msg_send_id![Self::class(), new] };
@@ -40,46 +58,20 @@ impl<'a> MyObject<'a> {
         let ivar = Self::class().instance_variable(c"number").unwrap();
         // SAFETY: The ivar is added with the same type below, and the
         // lifetime of the reference is properly bound to the class.
-        unsafe { ivar.load_ptr::<Ivar<'_>>(&this.superclass).write(number) };
+        unsafe { ivar.load_ptr::<Ivar<'_>>(&this.inner).write(number) };
         this
     }
 
     fn get(&self) -> u8 {
         let ivar = Self::class().instance_variable(c"number").unwrap();
         // SAFETY: The ivar is added with the same type below, and is initialized in `new`
-        unsafe { ivar.load::<Ivar<'_>>(&self.superclass).get() }
+        unsafe { ivar.load::<Ivar<'_>>(&self.inner).get() }
     }
 
     fn set(&self, number: u8) {
         let ivar = Self::class().instance_variable(c"number").unwrap();
         // SAFETY: The ivar is added with the same type below, and is initialized in `new`
-        unsafe { ivar.load::<Ivar<'_>>(&self.superclass).set(number) };
-    }
-}
-
-unsafe impl<'a> ClassType for MyObject<'a> {
-    type Super = NSObject;
-    type ThreadKind = <Self::Super as ClassType>::ThreadKind;
-    const NAME: &'static str = "MyObject";
-
-    fn class() -> &'static AnyClass {
-        // NOTE: Use std::lazy::LazyCell if in MSRV
-        static REGISTER_CLASS: Once = Once::new();
-
-        REGISTER_CLASS.call_once(|| {
-            let superclass = NSObject::class();
-            let mut builder = ClassBuilder::new(c"MyObject", superclass).unwrap();
-
-            builder.add_ivar::<Ivar<'_>>(c"number");
-
-            let _cls = builder.register();
-        });
-
-        AnyClass::get(c"MyObject").unwrap()
-    }
-
-    fn as_super(&self) -> &Self::Super {
-        &self.superclass
+        unsafe { ivar.load::<Ivar<'_>>(&self.inner).set(number) };
     }
 }
 
diff --git a/crates/objc2/src/rc/retained.rs b/crates/objc2/src/rc/retained.rs
index b0a49e018..bb3ba78ab 100644
--- a/crates/objc2/src/rc/retained.rs
+++ b/crates/objc2/src/rc/retained.rs
@@ -983,8 +983,7 @@ mod tests {
         let obj: Retained<RcTestObject> = RcTestObject::new();
         let expected = ThreadTestData::current();
 
-        // SAFETY: Any object can be cast to `AnyObject`
-        let obj: Retained<AnyObject> = unsafe { Retained::cast_unchecked(obj) };
+        let obj: Retained<AnyObject> = obj.into();
         expected.assert_current();
 
         let _obj: Retained<RcTestObject> = Retained::downcast(obj).unwrap();
diff --git a/crates/objc2/src/top_level_traits.rs b/crates/objc2/src/top_level_traits.rs
index 50f611154..53b34f3af 100644
--- a/crates/objc2/src/top_level_traits.rs
+++ b/crates/objc2/src/top_level_traits.rs
@@ -80,6 +80,12 @@ use crate::{msg_send_id, MainThreadMarker};
 /// //
 /// // And `Retained<MyObject>` can now be constructed.
 /// ```
+///
+/// Implement the trait manually for a class with a lifetime parameter.
+///
+/// ```
+#[doc = include_str!("../examples/class_with_lifetime.rs")]
+/// ```
 pub unsafe trait Message: RefEncode {
     /// Increment the reference count of the receiver.
     ///
@@ -200,12 +206,6 @@ pub unsafe trait Message: RefEncode {
 /// let cls = MyClass::class();
 /// let obj = MyClass::alloc();
 /// ```
-///
-/// Implement the trait manually for a class with a lifetime parameter.
-///
-/// ```
-#[doc = include_str!("../examples/class_with_lifetime.rs")]
-/// ```
 pub unsafe trait ClassType: Message {
     /// The superclass of this class.
     ///