From 4d2591ee0094ae1031e17d45daaac5574f2b6a4f Mon Sep 17 00:00:00 2001 From: Mads Marquart Date: Fri, 28 May 2021 15:48:17 +0200 Subject: [PATCH 01/14] Add initial `Retained` pointer, the smart pointer version of StrongPtr --- src/rc/mod.rs | 21 +--- src/rc/retained.rs | 245 +++++++++++++++++++++++++++++++++++++++++++++ 2 files changed, 247 insertions(+), 19 deletions(-) create mode 100644 src/rc/retained.rs diff --git a/src/rc/mod.rs b/src/rc/mod.rs index c703f585d..a16a56731 100644 --- a/src/rc/mod.rs +++ b/src/rc/mod.rs @@ -40,10 +40,12 @@ assert!(weak.load().is_null()); ``` */ +mod retained; mod strong; mod weak; mod autorelease; +pub use self::retained::Retained; pub use self::strong::StrongPtr; pub use self::weak::WeakPtr; pub use self::autorelease::autoreleasepool; @@ -55,25 +57,6 @@ mod tests { use super::StrongPtr; use super::autoreleasepool; - #[test] - fn test_strong_clone() { - fn retain_count(obj: *mut Object) -> usize { - unsafe { msg_send![obj, retainCount] } - } - - let obj = unsafe { - StrongPtr::new(msg_send![class!(NSObject), new]) - }; - assert!(retain_count(*obj) == 1); - - let cloned = obj.clone(); - assert!(retain_count(*cloned) == 2); - assert!(retain_count(*obj) == 2); - - drop(obj); - assert!(retain_count(*cloned) == 1); - } - #[test] fn test_weak() { let obj = unsafe { diff --git a/src/rc/retained.rs b/src/rc/retained.rs new file mode 100644 index 000000000..f4e22c424 --- /dev/null +++ b/src/rc/retained.rs @@ -0,0 +1,245 @@ +use core::borrow; +use core::fmt; +use core::hash; +use core::marker::{PhantomData, Unpin}; +use core::mem; +use core::ops::Deref; +use core::ptr::NonNull; + +use crate::runtime::{self, Object}; + +/// A smart pointer that strongly references an object, ensuring it won't be +/// deallocated. +/// +/// This is guaranteed to have the same size as the underlying pointer. +/// +/// ## Caveats +/// +/// If the inner type implements [`Drop`], that implementation will not be +/// called, since there is no way to ensure that the Objective-C runtime will +/// do so. If you need to run some code when the object is destroyed, +/// implement the `dealloc` selector instead. +/// +/// TODO: Explain similarities with `Arc` and `RefCell`. +#[repr(transparent)] +pub struct Retained { + /// A pointer to the contained object. + /// + /// It is important that this is `NonNull`, since we want to dereference + /// it later. + /// + /// Usually the contained object would be an [extern type][extern-type-rfc] + /// (when that gets stabilized), or a type such as: + /// ``` + /// pub struct MyType { + /// _data: [u8; 0], // TODO: `UnsafeCell`? + /// } + /// ``` + /// + /// DSTs that carry metadata cannot be used here, so unsure if we should + /// have a `?Sized` bound? + /// + /// TODO: + /// https://doc.rust-lang.org/book/ch19-04-advanced-types.html#dynamically-sized-types-and-the-sized-trait + /// https://doc.rust-lang.org/nomicon/exotic-sizes.html + /// https://doc.rust-lang.org/core/ptr/trait.Pointee.html + /// https://doc.rust-lang.org/core/ptr/traitalias.Thin.html + /// + /// [extern-type-rfc]: https://github.com/rust-lang/rfcs/blob/master/text/1861-extern-types.md + ptr: NonNull, // Covariant + phantom: PhantomData, +} + +impl Retained { + /// Constructs a `Retained` to an object that already has a +1 retain + /// count. This will not retain the object. + /// + /// When dropped, the object will be released. + /// + /// # Safety + /// + /// The caller must ensure the given object pointer is valid, and has +1 + /// retain count. + /// + /// TODO: Something about there not being any mutable references. + #[inline] + pub const unsafe fn new(ptr: NonNull) -> Self { + Retained { + ptr, + phantom: PhantomData, + } + } + + #[inline] + pub const fn as_ptr(&self) -> *mut T { + self.ptr.as_ptr() + } + + /// Retains the given object pointer. + /// + /// When dropped, the object will be released. + /// + /// # Safety + /// + /// The caller must ensure the given object pointer is valid. + #[doc(alias = "objc_retain")] + #[inline] + pub unsafe fn retain(ptr: NonNull) -> Self { + // SAFETY: The caller upholds that the pointer is valid + let rtn = runtime::objc_retain(ptr.as_ptr() as *mut Object); + debug_assert_eq!(rtn, ptr.as_ptr() as *mut Object); + Retained { + ptr, + phantom: PhantomData, + } + } + + /// Autoreleases the retained pointer, meaning that the object is not + /// immediately released, but will be when the innermost / current + /// autorelease pool is drained. + /// + /// A pointer to the object is returned, but it's validity is only until + /// guaranteed until the innermost pool is drained. + #[doc(alias = "objc_autorelease")] + #[must_use = "If you don't intend to use the object any more, just drop it as usual"] + #[inline] + pub fn autorelease(self) -> NonNull { + let ptr = mem::ManuallyDrop::new(self).ptr; + // SAFETY: The `ptr` is guaranteed to be valid and have at least one + // retain count. + // And because of the ManuallyDrop, we don't call the Drop + // implementation, so the object won't also be released there. + unsafe { runtime::objc_autorelease(ptr.as_ptr() as *mut Object) }; + ptr + } + + #[cfg(test)] + #[doc(alias = "retainCount")] + pub fn retain_count(&self) -> usize { + unsafe { msg_send![self.ptr.as_ptr() as *mut Object, retainCount] } + } +} + +// TODO: #[may_dangle] +// https://doc.rust-lang.org/nightly/nomicon/dropck.html +impl Drop for Retained { + /// Releases the retained object + #[doc(alias = "objc_release")] + #[doc(alias = "release")] + #[inline] + fn drop(&mut self) { + // SAFETY: The `ptr` is guaranteed to be valid and have at least one + // retain count + unsafe { runtime::objc_release(self.ptr.as_ptr() as *mut Object) }; + } +} + +impl Clone for Retained { + /// Makes a clone of the `Retained` object. + /// + /// This increases the object's reference count. + #[doc(alias = "objc_retain")] + #[doc(alias = "retain")] + #[inline] + fn clone(&self) -> Self { + // SAFETY: The `ptr` is guaranteed to be valid + unsafe { Self::retain(self.ptr) } + } +} + +impl Deref for Retained { + type Target = T; + + #[inline] + fn deref(&self) -> &Self::Target { + // SAFETY: TODO + unsafe { self.ptr.as_ref() } + } +} + +impl PartialEq for Retained { + #[inline] + fn eq(&self, other: &Retained) -> bool { + &**self == &**other + } + + #[inline] + fn ne(&self, other: &Retained) -> bool { + &**self != &**other + } +} + +// TODO: impl PartialOrd, Ord and Eq + +impl fmt::Display for Retained { + fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { + fmt::Display::fmt(&**self, f) + } +} + +impl fmt::Debug for Retained { + fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { + fmt::Debug::fmt(&**self, f) + } +} + +impl fmt::Pointer for Retained { + fn fmt(&self, f: &mut fmt::Formatter) -> fmt::Result { + fmt::Pointer::fmt(&self.as_ptr(), f) + } +} + +impl hash::Hash for Retained { + fn hash(&self, state: &mut H) { + (&**self).hash(state) + } +} + +impl borrow::Borrow for Retained { + fn borrow(&self) -> &T { + &**self + } +} + +impl AsRef for Retained { + fn as_ref(&self) -> &T { + &**self + } +} + +impl Unpin for Retained {} + +#[cfg(test)] +mod tests { + use std::mem::size_of; + + use super::Retained; + use crate::runtime::Object; + + pub struct TestType { + _data: [u8; 0], // TODO: `UnsafeCell`? + } + + #[test] + fn test_size_of() { + assert_eq!(size_of::>(), size_of::<&TestType>()); + assert_eq!( + size_of::>>(), + size_of::<&TestType>() + ); + } + + #[cfg(any(target_os = "macos", target_os = "ios"))] + #[test] + fn test_clone() { + let obj: Retained = unsafe { Retained::new(msg_send![class!(NSObject), new]) }; + assert!(obj.retain_count() == 1); + + let cloned = obj.clone(); + assert!(cloned.retain_count() == 2); + assert!(obj.retain_count() == 2); + + drop(obj); + assert!(cloned.retain_count() == 1); + } +} From b9f18496afd66d9fc14460bb84362bc8f227f43c Mon Sep 17 00:00:00 2001 From: Mads Marquart Date: Fri, 28 May 2021 17:33:28 +0200 Subject: [PATCH 02/14] Fix creation of Retained in test when verify_message feature is enabled --- src/rc/retained.rs | 7 +++++-- 1 file changed, 5 insertions(+), 2 deletions(-) diff --git a/src/rc/retained.rs b/src/rc/retained.rs index f4e22c424..a25f2cade 100644 --- a/src/rc/retained.rs +++ b/src/rc/retained.rs @@ -211,7 +211,8 @@ impl Unpin for Retained {} #[cfg(test)] mod tests { - use std::mem::size_of; + use core::mem::size_of; + use core::ptr::NonNull; use super::Retained; use crate::runtime::Object; @@ -232,7 +233,9 @@ mod tests { #[cfg(any(target_os = "macos", target_os = "ios"))] #[test] fn test_clone() { - let obj: Retained = unsafe { Retained::new(msg_send![class!(NSObject), new]) }; + // TODO: Maybe make a way to return `Retained` directly? + let obj: *mut Object = unsafe { msg_send![class!(NSObject), new] }; + let obj: Retained = unsafe { Retained::new(NonNull::new(obj).unwrap()) }; assert!(obj.retain_count() == 1); let cloned = obj.clone(); From 2cebfce66573155339a3fb741a390cd78eab5e63 Mon Sep 17 00:00:00 2001 From: Mads Marquart Date: Sat, 29 May 2021 02:02:40 +0200 Subject: [PATCH 03/14] Add initial `Owned` pointer, the mutable version of Retained --- src/rc/mod.rs | 2 + src/rc/owned.rs | 160 +++++++++++++++++++++++++++++++++++++++++++++ src/rc/retained.rs | 8 +++ 3 files changed, 170 insertions(+) create mode 100644 src/rc/owned.rs diff --git a/src/rc/mod.rs b/src/rc/mod.rs index a16a56731..57d81ef50 100644 --- a/src/rc/mod.rs +++ b/src/rc/mod.rs @@ -40,12 +40,14 @@ assert!(weak.load().is_null()); ``` */ +mod owned; mod retained; mod strong; mod weak; mod autorelease; pub use self::retained::Retained; +pub use self::owned::Owned; pub use self::strong::StrongPtr; pub use self::weak::WeakPtr; pub use self::autorelease::autoreleasepool; diff --git a/src/rc/owned.rs b/src/rc/owned.rs new file mode 100644 index 000000000..e3cffe8fa --- /dev/null +++ b/src/rc/owned.rs @@ -0,0 +1,160 @@ +use core::marker::PhantomData; +use core::ptr::{NonNull, drop_in_place}; +use core::mem; +use core::ops::{DerefMut, Deref}; +use core::fmt; +use core::hash; +use core::borrow; + +use super::Retained; +use crate::runtime::{self, Object}; + +/// A smart pointer that strongly references and owns an Objective-C object. +/// +/// The fact that we own the pointer means that we're safe to mutate it, hence +/// why this implements [`DerefMut`]. +/// +/// This is guaranteed to have the same size as the underlying pointer. +/// +/// TODO: Explain similarities to [`Box`]. +/// +/// TODO: Explain this vs. [`Retained`] +#[repr(transparent)] +pub struct Owned { + /// The pointer is always retained. + pub(super) ptr: NonNull, // Covariant + phantom: PhantomData, // Necessary for dropcheck +} + +// SAFETY: TODO +unsafe impl Send for Owned {} + +// SAFETY: TODO +unsafe impl Sync for Owned {} + +impl Owned { + #[inline] + pub unsafe fn new(ptr: NonNull) -> Self { + Self { + ptr, + phantom: PhantomData, + } + } + + // TODO: Unsure how the API should look... + #[inline] + pub unsafe fn retain(ptr: NonNull) -> Self { + Self::from_retained(Retained::retain(ptr)) + } + + /// TODO + /// + /// # Safety + /// + /// The given [`Retained`] must be the only reference to the object + /// anywhere in the program - even in other Objective-C code. + #[inline] + pub unsafe fn from_retained(obj: Retained) -> Self { + let ptr = mem::ManuallyDrop::new(obj).ptr; + Self { + ptr, + phantom: PhantomData, + } + } +} + +// TODO: #[may_dangle] +// https://doc.rust-lang.org/nightly/nomicon/dropck.html +impl Drop for Owned { + /// Releases the retained object + /// + /// This is guaranteed to be the last destructor that runs, in contrast to + /// [`Retained`], which means that we can run the [`Drop`] implementation + /// on the contained object as well. + #[inline] + fn drop(&mut self) { + let ptr = self.ptr; + unsafe { + drop_in_place(ptr.as_ptr()); + // Construct a new `Retained`, which will be dropped immediately + Retained::new(ptr); + }; + } +} + +// Note: `Clone` is not implemented for this! + +impl Deref for Owned { + type Target = T; + + #[inline] + fn deref(&self) -> &Self::Target { + // SAFETY: TODO + unsafe { self.ptr.as_ref() } + } +} + +impl DerefMut for Owned { + #[inline] + fn deref_mut(&mut self) -> &mut Self::Target { + // SAFETY: TODO + unsafe { self.ptr.as_mut() } + } +} + +// TODO: impl PartialEq, PartialOrd, Ord and Eq + +impl fmt::Display for Owned { + fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { + fmt::Display::fmt(&**self, f) + } +} + +impl fmt::Debug for Owned { + fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { + fmt::Debug::fmt(&**self, f) + } +} + +impl fmt::Pointer for Owned { + fn fmt(&self, f: &mut fmt::Formatter) -> fmt::Result { + fmt::Pointer::fmt(&self.ptr.as_ptr(), f) + } +} + +impl hash::Hash for Owned { + fn hash(&self, state: &mut H) { + (&**self).hash(state) + } +} + +// TODO: impl Fn traits? See `boxed_closure_impls` + +// TODO: CoerceUnsized + +impl borrow::Borrow for Owned { + fn borrow(&self) -> &T { + &**self + } +} + +impl borrow::BorrowMut for Owned { + fn borrow_mut(&mut self) -> &mut T { + &mut **self + } +} + +impl AsRef for Owned { + fn as_ref(&self) -> &T { + &**self + } +} + +impl AsMut for Owned { + fn as_mut(&mut self) -> &mut T { + &mut **self + } +} + +// TODO: Comment on impl Unpin for Box +impl Unpin for Owned {} diff --git a/src/rc/retained.rs b/src/rc/retained.rs index a25f2cade..d1bda9986 100644 --- a/src/rc/retained.rs +++ b/src/rc/retained.rs @@ -7,6 +7,7 @@ use core::ops::Deref; use core::ptr::NonNull; use crate::runtime::{self, Object}; +use super::Owned; /// A smart pointer that strongly references an object, ensuring it won't be /// deallocated. @@ -209,6 +210,13 @@ impl AsRef for Retained { impl Unpin for Retained {} +impl From> for Retained { + fn from(obj: Owned) -> Self { + // SAFETY: TODO + unsafe { Self::new(obj.ptr) } + } +} + #[cfg(test)] mod tests { use core::mem::size_of; From a0760018ccefc73f459e7f8167a149983dcce51a Mon Sep 17 00:00:00 2001 From: Mads Marquart Date: Sat, 29 May 2021 02:03:02 +0200 Subject: [PATCH 04/14] Add Send and Sync impls for Retained --- src/rc/retained.rs | 7 +++++++ 1 file changed, 7 insertions(+) diff --git a/src/rc/retained.rs b/src/rc/retained.rs index d1bda9986..ef61775bb 100644 --- a/src/rc/retained.rs +++ b/src/rc/retained.rs @@ -51,6 +51,13 @@ pub struct Retained { phantom: PhantomData, } +// SAFETY: TODO +unsafe impl Send for Retained {} + +// SAFETY: TODO +unsafe impl Sync for Retained {} + + impl Retained { /// Constructs a `Retained` to an object that already has a +1 retain /// count. This will not retain the object. From 869449dc4aae0a2d08adb04d579852613a049aa2 Mon Sep 17 00:00:00 2001 From: Mads Marquart Date: Sat, 29 May 2021 02:03:41 +0200 Subject: [PATCH 05/14] Clean up Retained code a bit --- src/rc/retained.rs | 14 +++++++------- 1 file changed, 7 insertions(+), 7 deletions(-) diff --git a/src/rc/retained.rs b/src/rc/retained.rs index ef61775bb..e8ad25b09 100644 --- a/src/rc/retained.rs +++ b/src/rc/retained.rs @@ -71,15 +71,15 @@ impl Retained { /// /// TODO: Something about there not being any mutable references. #[inline] - pub const unsafe fn new(ptr: NonNull) -> Self { - Retained { + pub unsafe fn new(ptr: NonNull) -> Self { + Self { ptr, phantom: PhantomData, } } #[inline] - pub const fn as_ptr(&self) -> *mut T { + pub fn as_ptr(&self) -> *mut T { self.ptr.as_ptr() } @@ -96,7 +96,7 @@ impl Retained { // SAFETY: The caller upholds that the pointer is valid let rtn = runtime::objc_retain(ptr.as_ptr() as *mut Object); debug_assert_eq!(rtn, ptr.as_ptr() as *mut Object); - Retained { + Self { ptr, phantom: PhantomData, } @@ -167,12 +167,12 @@ impl Deref for Retained { impl PartialEq for Retained { #[inline] - fn eq(&self, other: &Retained) -> bool { + fn eq(&self, other: &Self) -> bool { &**self == &**other } #[inline] - fn ne(&self, other: &Retained) -> bool { + fn ne(&self, other: &Self) -> bool { &**self != &**other } } @@ -193,7 +193,7 @@ impl fmt::Debug for Retained { impl fmt::Pointer for Retained { fn fmt(&self, f: &mut fmt::Formatter) -> fmt::Result { - fmt::Pointer::fmt(&self.as_ptr(), f) + fmt::Pointer::fmt(&self.ptr.as_ptr(), f) } } From 684159dd8fd7f67d8d2e3217035894822e8a09ba Mon Sep 17 00:00:00 2001 From: Mads Marquart Date: Sat, 29 May 2021 02:04:10 +0200 Subject: [PATCH 06/14] Add some extra TODOs to Retained --- src/rc/retained.rs | 12 +++++++++++- 1 file changed, 11 insertions(+), 1 deletion(-) diff --git a/src/rc/retained.rs b/src/rc/retained.rs index e8ad25b09..5dcdcf925 100644 --- a/src/rc/retained.rs +++ b/src/rc/retained.rs @@ -14,6 +14,9 @@ use super::Owned; /// /// This is guaranteed to have the same size as the underlying pointer. /// +/// TODO: Something about the fact that we haven't made the methods associated +/// for [reasons]??? +/// /// ## Caveats /// /// If the inner type implements [`Drop`], that implementation will not be @@ -21,6 +24,8 @@ use super::Owned; /// do so. If you need to run some code when the object is destroyed, /// implement the `dealloc` selector instead. /// +/// TODO: Restrict the possible types with some kind of unsafe marker trait? +/// /// TODO: Explain similarities with `Arc` and `RefCell`. #[repr(transparent)] pub struct Retained { @@ -47,7 +52,9 @@ pub struct Retained { /// https://doc.rust-lang.org/core/ptr/traitalias.Thin.html /// /// [extern-type-rfc]: https://github.com/rust-lang/rfcs/blob/master/text/1861-extern-types.md - ptr: NonNull, // Covariant + pub(super) ptr: NonNull, // Covariant - but should probably be invariant? + /// TODO: + /// https://github.com/rust-lang/rfcs/blob/master/text/0769-sound-generic-drop.md#phantom-data phantom: PhantomData, } @@ -92,6 +99,7 @@ impl Retained { /// The caller must ensure the given object pointer is valid. #[doc(alias = "objc_retain")] #[inline] + // TODO: Maybe just take a normal reference, and then this can be safe? pub unsafe fn retain(ptr: NonNull) -> Self { // SAFETY: The caller upholds that the pointer is valid let rtn = runtime::objc_retain(ptr.as_ptr() as *mut Object); @@ -215,6 +223,8 @@ impl AsRef for Retained { } } +// TODO: CoerceUnsized? + impl Unpin for Retained {} impl From> for Retained { From f52bc56fd6e36ef54dbf50d39e664d8b5e7d6038 Mon Sep 17 00:00:00 2001 From: Mads Marquart Date: Sat, 29 May 2021 03:13:06 +0200 Subject: [PATCH 07/14] Create Owned and Retained using references instead of NonNull pointers Makes the API easier to use, and means there are less safety requirements that we have to document. --- src/rc/owned.rs | 55 ++++++++++++++++++++++++++++++---------------- src/rc/retained.rs | 54 ++++++++++++++++++++++++++++++--------------- 2 files changed, 72 insertions(+), 37 deletions(-) diff --git a/src/rc/owned.rs b/src/rc/owned.rs index e3cffe8fa..009168398 100644 --- a/src/rc/owned.rs +++ b/src/rc/owned.rs @@ -1,18 +1,18 @@ -use core::marker::PhantomData; -use core::ptr::{NonNull, drop_in_place}; -use core::mem; -use core::ops::{DerefMut, Deref}; +use core::borrow; use core::fmt; use core::hash; -use core::borrow; +use core::marker::PhantomData; +use core::mem; +use core::ops::{Deref, DerefMut}; +use core::ptr::{drop_in_place, NonNull}; use super::Retained; use crate::runtime::{self, Object}; /// A smart pointer that strongly references and owns an Objective-C object. /// -/// The fact that we own the pointer means that we're safe to mutate it, hence -/// why this implements [`DerefMut`]. +/// The fact that we own the pointer means that it's safe to mutate it. As +/// such, this implements [`DerefMut`]. /// /// This is guaranteed to have the same size as the underlying pointer. /// @@ -32,27 +32,44 @@ unsafe impl Send for Owned {} // SAFETY: TODO unsafe impl Sync for Owned {} +// TODO: Unsure how the API should look... impl Owned { + /// TODO + /// + /// # Safety + /// + /// The caller must ensure the given object reference has exactly 1 retain + /// count (that is, a retain count that has been handed off from somewhere + /// else, usually Objective-C methods like `init`, `alloc`, `new`, or + /// `copy`). + /// + /// Additionally, there must be no other pointers to the same object. + /// + /// # Example + /// + /// ```rust + /// let obj: &mut Object = unsafe { msg_send![cls, alloc] }; + /// let obj: Owned = unsafe { Owned::new(msg_send![obj, init]) }; + /// // Or in this case simply just: + /// let obj: Owned = unsafe { Owned::new(msg_send![cls, new]) }; + /// ``` + /// + /// TODO: Something about there not being other references. #[inline] - pub unsafe fn new(ptr: NonNull) -> Self { + pub unsafe fn new(obj: &mut T) -> Self { Self { - ptr, + ptr: obj.into(), phantom: PhantomData, } } - // TODO: Unsure how the API should look... - #[inline] - pub unsafe fn retain(ptr: NonNull) -> Self { - Self::from_retained(Retained::retain(ptr)) - } - - /// TODO + /// Construct an `Owned` pointer /// /// # Safety /// - /// The given [`Retained`] must be the only reference to the object - /// anywhere in the program - even in other Objective-C code. + /// The caller must ensure that there are no other pointers to the same + /// object (which also means that the given [`Retained`] should have a + /// retain count of exactly 1). #[inline] pub unsafe fn from_retained(obj: Retained) -> Self { let ptr = mem::ManuallyDrop::new(obj).ptr; @@ -77,7 +94,7 @@ impl Drop for Owned { unsafe { drop_in_place(ptr.as_ptr()); // Construct a new `Retained`, which will be dropped immediately - Retained::new(ptr); + Retained::new(ptr.as_ref()); }; } } diff --git a/src/rc/retained.rs b/src/rc/retained.rs index 5dcdcf925..acd58df3f 100644 --- a/src/rc/retained.rs +++ b/src/rc/retained.rs @@ -6,8 +6,8 @@ use core::mem; use core::ops::Deref; use core::ptr::NonNull; -use crate::runtime::{self, Object}; use super::Owned; +use crate::runtime::{self, Object}; /// A smart pointer that strongly references an object, ensuring it won't be /// deallocated. @@ -64,29 +64,33 @@ unsafe impl Send for Retained {} // SAFETY: TODO unsafe impl Sync for Retained {} - impl Retained { /// Constructs a `Retained` to an object that already has a +1 retain /// count. This will not retain the object. /// /// When dropped, the object will be released. /// + /// See also [`Owned::new`] for the common case of creating objects. + /// /// # Safety /// - /// The caller must ensure the given object pointer is valid, and has +1 - /// retain count. + /// The caller must ensure the given object reference has +1 retain count + /// (that is, a retain count that has been handed off from somewhere else, + /// usually Objective-C methods with the `ns_returns_retained` attribute). + /// + /// Additionally, there must be no [`Owned`] pointers to the same object. /// /// TODO: Something about there not being any mutable references. #[inline] - pub unsafe fn new(ptr: NonNull) -> Self { + pub unsafe fn new(obj: &T) -> Self { Self { - ptr, + ptr: obj.into(), phantom: PhantomData, } } #[inline] - pub fn as_ptr(&self) -> *mut T { + pub fn as_ptr(&self) -> *const T { self.ptr.as_ptr() } @@ -96,16 +100,28 @@ impl Retained { /// /// # Safety /// - /// The caller must ensure the given object pointer is valid. + /// The caller must ensure that there are no [`Owned`] pointers to the + /// same object. + // + // So this would be illegal: + // ```rust + // let owned: Owned = ...; + // // Lifetime information is discarded + // let retained = Retained::retain(&*owned); + // // Which means we can still mutate `Owned`: + // let x: &mut T = &mut *owned; + // // While we have an immutable reference + // let y: &T = &*retained; + // ``` #[doc(alias = "objc_retain")] - #[inline] - // TODO: Maybe just take a normal reference, and then this can be safe? - pub unsafe fn retain(ptr: NonNull) -> Self { + // Inlined since it's `objc_retain` that does the work. + #[cfg_attr(debug_assertions, inline)] + pub unsafe fn retain(obj: &T) -> Self { // SAFETY: The caller upholds that the pointer is valid - let rtn = runtime::objc_retain(ptr.as_ptr() as *mut Object); - debug_assert_eq!(rtn, ptr.as_ptr() as *mut Object); + let rtn = runtime::objc_retain(obj as *const T as *mut Object); + debug_assert_eq!(rtn, obj as *const T as *mut Object); Self { - ptr, + ptr: obj.into(), phantom: PhantomData, } } @@ -119,6 +135,8 @@ impl Retained { #[doc(alias = "objc_autorelease")] #[must_use = "If you don't intend to use the object any more, just drop it as usual"] #[inline] + // TODO: Get a lifetime relating to the pool, so that we can return a + // reference instead of a pointer. pub fn autorelease(self) -> NonNull { let ptr = mem::ManuallyDrop::new(self).ptr; // SAFETY: The `ptr` is guaranteed to be valid and have at least one @@ -159,7 +177,7 @@ impl Clone for Retained { #[inline] fn clone(&self) -> Self { // SAFETY: The `ptr` is guaranteed to be valid - unsafe { Self::retain(self.ptr) } + unsafe { Self::retain(&*self) } } } @@ -230,7 +248,7 @@ impl Unpin for Retained {} impl From> for Retained { fn from(obj: Owned) -> Self { // SAFETY: TODO - unsafe { Self::new(obj.ptr) } + unsafe { Self::new(&*obj) } } } @@ -259,8 +277,8 @@ mod tests { #[test] fn test_clone() { // TODO: Maybe make a way to return `Retained` directly? - let obj: *mut Object = unsafe { msg_send![class!(NSObject), new] }; - let obj: Retained = unsafe { Retained::new(NonNull::new(obj).unwrap()) }; + let obj: &Object = unsafe { msg_send![class!(NSObject), new] }; + let obj: Retained = unsafe { Retained::new(obj) }; assert!(obj.retain_count() == 1); let cloned = obj.clone(); From fecb2e7ced6c2203d6472e58ef15d7d4df0b0d46 Mon Sep 17 00:00:00 2001 From: Mads Marquart Date: Sat, 29 May 2021 03:28:29 +0200 Subject: [PATCH 08/14] Add some more documentation to Owned and Retained --- src/rc/owned.rs | 17 ++++++++++++++++- src/rc/retained.rs | 5 ++++- 2 files changed, 20 insertions(+), 2 deletions(-) diff --git a/src/rc/owned.rs b/src/rc/owned.rs index 009168398..ac6c60a51 100644 --- a/src/rc/owned.rs +++ b/src/rc/owned.rs @@ -16,6 +16,18 @@ use crate::runtime::{self, Object}; /// /// This is guaranteed to have the same size as the underlying pointer. /// +/// # Cloning and [`Retained`] +/// +/// This does not implement [`Clone`], but [`Retained`] has a [`From`] +/// implementation to convert from this, so you can easily reliquish ownership +/// and work with a normal [`Retained`] pointer. +/// +/// ```no_run +/// let obj: Owned = ...; +/// let retained: Retained = obj.into(); +/// let cloned: Retained = retained.clone(); +/// ``` +/// /// TODO: Explain similarities to [`Box`]. /// /// TODO: Explain this vs. [`Retained`] @@ -43,7 +55,8 @@ impl Owned { /// else, usually Objective-C methods like `init`, `alloc`, `new`, or /// `copy`). /// - /// Additionally, there must be no other pointers to the same object. + /// Additionally, there must be no other pointers or references to the same + /// object, and the given reference must not be used afterwards. /// /// # Example /// @@ -55,6 +68,8 @@ impl Owned { /// ``` /// /// TODO: Something about there not being other references. + // Note: The fact that we take a `&mut` here is more of a lint; the lifetime + // information is lost, so whatever produced the reference can still be #[inline] pub unsafe fn new(obj: &mut T) -> Self { Self { diff --git a/src/rc/retained.rs b/src/rc/retained.rs index acd58df3f..2b9917841 100644 --- a/src/rc/retained.rs +++ b/src/rc/retained.rs @@ -9,9 +9,12 @@ use core::ptr::NonNull; use super::Owned; use crate::runtime::{self, Object}; -/// A smart pointer that strongly references an object, ensuring it won't be +/// An smart pointer that strongly references an object, ensuring it won't be /// deallocated. /// +/// This doesn't own the object, so it is not safe to obtain a mutable +/// reference from this. For that, see [`Owned`]. +/// /// This is guaranteed to have the same size as the underlying pointer. /// /// TODO: Something about the fact that we haven't made the methods associated From 2ec0185ba1395ec7413b1a2ebedd57728b15f678 Mon Sep 17 00:00:00 2001 From: Mads Marquart Date: Sat, 29 May 2021 03:47:27 +0200 Subject: [PATCH 09/14] Add stubs for a few extra objc reference counting methods --- src/rc/retained.rs | 41 ++++++++++++++++++++++++++++++++++++++++- 1 file changed, 40 insertions(+), 1 deletion(-) diff --git a/src/rc/retained.rs b/src/rc/retained.rs index 2b9917841..ebf6574f1 100644 --- a/src/rc/retained.rs +++ b/src/rc/retained.rs @@ -129,6 +129,12 @@ impl Retained { } } + /// TODO + #[doc(alias = "objc_retainAutoreleasedReturnValue")] + pub unsafe fn retain_autoreleased_return(obj: &T) -> Self { + todo!() + } + /// Autoreleases the retained pointer, meaning that the object is not /// immediately released, but will be when the innermost / current /// autorelease pool is drained. @@ -150,13 +156,46 @@ impl Retained { ptr } - #[cfg(test)] + /// TODO + #[doc(alias = "objc_autoreleaseReturnValue")] + pub fn autorelease_return(self) -> *const T { + todo!() + } + + /// TODO + /// + /// Equivalent to `Retained::retain(&obj).autorelease()`, but slightly + /// more efficient. + #[doc(alias = "objc_retainAutorelease")] + pub unsafe fn retain_and_autorelease(obj: &T) -> *const T { + todo!() + } + + /// TODO + /// + /// Equivalent to `Retained::retain(&obj).autorelease_return()`, but + /// slightly more efficient. + #[doc(alias = "objc_retainAutoreleaseReturnValue")] + pub unsafe fn retain_and_autorelease_return(obj: &T) -> *const T { + todo!() + } + + #[cfg(test)] // TODO #[doc(alias = "retainCount")] pub fn retain_count(&self) -> usize { unsafe { msg_send![self.ptr.as_ptr() as *mut Object, retainCount] } } } +// TODO: Consider something like this +// #[cfg(block)] +// impl Retained { +// #[doc(alias = "objc_retainBlock")] +// pub unsafe fn retain_block(block: &T) -> Self { +// todo!() +// } +// } + // TODO: #[may_dangle] // https://doc.rust-lang.org/nightly/nomicon/dropck.html impl Drop for Retained { From 7147f9f4321f0b5683a370b2d2c1d3847f926efa Mon Sep 17 00:00:00 2001 From: Mads Marquart Date: Mon, 31 May 2021 08:33:36 +0200 Subject: [PATCH 10/14] Add comments on why the Sync, Send and Drop implementations are safe Also clean up other comments --- src/rc/owned.rs | 56 ++++++++++++++++++++++++++++------------------ src/rc/retained.rs | 33 +++++++++++++++++++-------- 2 files changed, 58 insertions(+), 31 deletions(-) diff --git a/src/rc/owned.rs b/src/rc/owned.rs index ac6c60a51..09fcc8402 100644 --- a/src/rc/owned.rs +++ b/src/rc/owned.rs @@ -9,10 +9,11 @@ use core::ptr::{drop_in_place, NonNull}; use super::Retained; use crate::runtime::{self, Object}; -/// A smart pointer that strongly references and owns an Objective-C object. +/// A smart pointer that strongly references and uniquely owns an Objective-C +/// object. /// -/// The fact that we own the pointer means that it's safe to mutate it. As -/// such, this implements [`DerefMut`]. +/// The fact that we uniquely own the pointer means that it's safe to mutate +/// it. As such, this implements [`DerefMut`]. /// /// This is guaranteed to have the same size as the underlying pointer. /// @@ -34,29 +35,31 @@ use crate::runtime::{self, Object}; #[repr(transparent)] pub struct Owned { /// The pointer is always retained. - pub(super) ptr: NonNull, // Covariant - phantom: PhantomData, // Necessary for dropcheck + ptr: NonNull, // We are the unique owner of T, so covariance is correct + phantom: PhantomData, // Necessary for dropck } -// SAFETY: TODO +/// `Owned` pointers are `Send` if `T` is `Send` because they give the same +/// access as having a T directly. unsafe impl Send for Owned {} -// SAFETY: TODO +/// `Owned` pointers are `Sync` if `T` is `Sync` because they give the same +/// access as having a `T` directly. unsafe impl Sync for Owned {} // TODO: Unsure how the API should look... impl Owned { - /// TODO + /// Create a new `Owned` pointer to the object. + /// + /// Uses a retain count that has been handed off from somewhere else, + /// usually Objective-C methods like `init`, `alloc`, `new`, or `copy`. /// /// # Safety /// - /// The caller must ensure the given object reference has exactly 1 retain - /// count (that is, a retain count that has been handed off from somewhere - /// else, usually Objective-C methods like `init`, `alloc`, `new`, or - /// `copy`). + /// The caller must ensure that there are no other pointers or references + /// to the same object, and the given reference is not be used afterwards. /// - /// Additionally, there must be no other pointers or references to the same - /// object, and the given reference must not be used afterwards. + /// Additionally, the given object reference must have +1 retain count. /// /// # Example /// @@ -70,6 +73,7 @@ impl Owned { /// TODO: Something about there not being other references. // Note: The fact that we take a `&mut` here is more of a lint; the lifetime // information is lost, so whatever produced the reference can still be + // mutated or aliased afterwards. #[inline] pub unsafe fn new(obj: &mut T) -> Self { Self { @@ -78,16 +82,23 @@ impl Owned { } } - /// Construct an `Owned` pointer + /// Acquires a `*mut` pointer to the object. + #[inline] + pub fn as_ptr(&self) -> *mut T { + self.ptr.as_ptr() + } + + /// Construct an `Owned` pointer from a `Retained` pointer. /// /// # Safety /// /// The caller must ensure that there are no other pointers to the same /// object (which also means that the given [`Retained`] should have a - /// retain count of exactly 1). + /// retain count of exactly 1 in almost all cases). #[inline] pub unsafe fn from_retained(obj: Retained) -> Self { - let ptr = mem::ManuallyDrop::new(obj).ptr; + // SAFETY: The pointer is guaranteed by `Retained` to be NonNull + let ptr = NonNull::new_unchecked(mem::ManuallyDrop::new(obj).as_ptr() as *mut T); Self { ptr, phantom: PhantomData, @@ -95,10 +106,13 @@ impl Owned { } } -// TODO: #[may_dangle] -// https://doc.rust-lang.org/nightly/nomicon/dropck.html +/// `#[may_dangle]` (see [this][dropck_eyepatch]) would not be safe here, +/// since we cannot verify that a `dealloc` method doesn't access borrowed +/// data. +/// +/// [dropck_eyepatch]: https://doc.rust-lang.org/nightly/nomicon/dropck.html#an-escape-hatch impl Drop for Owned { - /// Releases the retained object + /// Releases the retained object. /// /// This is guaranteed to be the last destructor that runs, in contrast to /// [`Retained`], which means that we can run the [`Drop`] implementation @@ -114,8 +128,6 @@ impl Drop for Owned { } } -// Note: `Clone` is not implemented for this! - impl Deref for Owned { type Target = T; diff --git a/src/rc/retained.rs b/src/rc/retained.rs index ebf6574f1..89521f112 100644 --- a/src/rc/retained.rs +++ b/src/rc/retained.rs @@ -55,16 +55,26 @@ pub struct Retained { /// https://doc.rust-lang.org/core/ptr/traitalias.Thin.html /// /// [extern-type-rfc]: https://github.com/rust-lang/rfcs/blob/master/text/1861-extern-types.md - pub(super) ptr: NonNull, // Covariant - but should probably be invariant? + ptr: NonNull, // T is immutable, so covariance is correct /// TODO: /// https://github.com/rust-lang/rfcs/blob/master/text/0769-sound-generic-drop.md#phantom-data phantom: PhantomData, } -// SAFETY: TODO +/// The `Send` implementation requires `T: Sync` because `Retained` gives +/// access to `&T`. +/// +/// Additiontally, it requires `T: Send` because if `T: !Send`, you could +/// clone a `Retained`, send it to another thread, and drop the clone last, +/// making `dealloc` get called on the other thread, violating `T: !Send`. unsafe impl Send for Retained {} -// SAFETY: TODO +/// The `Sync` implementation requires `T: Sync` because `&Retained` gives +/// access to `&T`. +/// +/// Additiontally, it requires `T: Send`, because if `T: !Send`, you could +/// clone a `&Retained` from another thread, and drop the clone last, making +/// `dealloc` get called on the other thread, violating `T: !Send`. unsafe impl Sync for Retained {} impl Retained { @@ -73,13 +83,14 @@ impl Retained { /// /// When dropped, the object will be released. /// - /// See also [`Owned::new`] for the common case of creating objects. + /// This is used when you have a retain count that has been handed off + /// from somewhere else, usually Objective-C methods with the + /// `ns_returns_retained` attribute. See [`Owned::new`] for the more + /// common case when creating objects. /// /// # Safety /// - /// The caller must ensure the given object reference has +1 retain count - /// (that is, a retain count that has been handed off from somewhere else, - /// usually Objective-C methods with the `ns_returns_retained` attribute). + /// The caller must ensure the given object reference has +1 retain count. /// /// Additionally, there must be no [`Owned`] pointers to the same object. /// @@ -92,6 +103,7 @@ impl Retained { } } + /// Acquires a `*const` pointer to the object. #[inline] pub fn as_ptr(&self) -> *const T { self.ptr.as_ptr() @@ -196,8 +208,11 @@ impl Retained { // } // } -// TODO: #[may_dangle] -// https://doc.rust-lang.org/nightly/nomicon/dropck.html +/// `#[may_dangle]` (see [this][dropck_eyepatch]) doesn't really make sense +/// here, since we actually want to disallow creating `Retained` pointers to +/// objects that have a `Drop` implementation. +/// +/// [dropck_eyepatch]: https://doc.rust-lang.org/nightly/nomicon/dropck.html#an-escape-hatch impl Drop for Retained { /// Releases the retained object #[doc(alias = "objc_release")] From 69b5047090bc7288883f1d45897598f96add7e2f Mon Sep 17 00:00:00 2001 From: Mads Marquart Date: Mon, 31 May 2021 08:56:24 +0200 Subject: [PATCH 11/14] Undo making Owned and Retained take references. See f52bc56fd, but that change made it very easy to create unbounded references, or create two aliasing mutable references. --- src/rc/owned.rs | 22 ++++++++++++---------- src/rc/retained.rs | 29 +++++++++++++++++++---------- 2 files changed, 31 insertions(+), 20 deletions(-) diff --git a/src/rc/owned.rs b/src/rc/owned.rs index 09fcc8402..f9dc7f578 100644 --- a/src/rc/owned.rs +++ b/src/rc/owned.rs @@ -21,7 +21,7 @@ use crate::runtime::{self, Object}; /// /// This does not implement [`Clone`], but [`Retained`] has a [`From`] /// implementation to convert from this, so you can easily reliquish ownership -/// and work with a normal [`Retained`] pointer. +/// and work with a clonable [`Retained`] pointer. /// /// ```no_run /// let obj: Owned = ...; @@ -57,9 +57,13 @@ impl Owned { /// # Safety /// /// The caller must ensure that there are no other pointers or references - /// to the same object, and the given reference is not be used afterwards. + /// to the same object, and the given pointer is not be used afterwards. /// - /// Additionally, the given object reference must have +1 retain count. + /// Additionally, the given object pointer must have +1 retain count. + /// + /// And lastly, the object pointer must be valid as a mutable reference + /// (non-null, aligned, dereferencable, initialized and upholds aliasing + /// rules, see the [`std::ptr`] module for more information). /// /// # Example /// @@ -69,15 +73,13 @@ impl Owned { /// // Or in this case simply just: /// let obj: Owned = unsafe { Owned::new(msg_send![cls, new]) }; /// ``` - /// - /// TODO: Something about there not being other references. - // Note: The fact that we take a `&mut` here is more of a lint; the lifetime - // information is lost, so whatever produced the reference can still be - // mutated or aliased afterwards. #[inline] - pub unsafe fn new(obj: &mut T) -> Self { + // Note: We don't take a mutable reference as a parameter since it would + // be too easy to accidentally create two aliasing mutable references. + pub unsafe fn new(ptr: *mut T) -> Self { Self { - ptr: obj.into(), + // SAFETY: Upheld by the caller + ptr: NonNull::new_unchecked(ptr), phantom: PhantomData, } } diff --git a/src/rc/retained.rs b/src/rc/retained.rs index 89521f112..6bce56425 100644 --- a/src/rc/retained.rs +++ b/src/rc/retained.rs @@ -92,13 +92,17 @@ impl Retained { /// /// The caller must ensure the given object reference has +1 retain count. /// - /// Additionally, there must be no [`Owned`] pointers to the same object. + /// Additionally, there must be no [`Owned`] pointers or mutable + /// references to the same object. /// - /// TODO: Something about there not being any mutable references. + /// And lastly, the object pointer must be valid as a reference (non-null, + /// aligned, dereferencable, initialized and upholds aliasing rules, see + /// the [`std::ptr`] module for more information). #[inline] - pub unsafe fn new(obj: &T) -> Self { + pub unsafe fn new(ptr: *const T) -> Self { Self { - ptr: obj.into(), + // SAFETY: Upheld by the caller + ptr: NonNull::new_unchecked(ptr as *mut T), phantom: PhantomData, } } @@ -117,6 +121,10 @@ impl Retained { /// /// The caller must ensure that there are no [`Owned`] pointers to the /// same object. + /// + /// Additionally, the object pointer must be valid as a reference + /// (non-null, aligned, dereferencable, initialized and upholds aliasing + /// rules, see the [`std::ptr`] module for more information). // // So this would be illegal: // ```rust @@ -131,12 +139,14 @@ impl Retained { #[doc(alias = "objc_retain")] // Inlined since it's `objc_retain` that does the work. #[cfg_attr(debug_assertions, inline)] - pub unsafe fn retain(obj: &T) -> Self { + pub unsafe fn retain(ptr: *const T) -> Self { // SAFETY: The caller upholds that the pointer is valid - let rtn = runtime::objc_retain(obj as *const T as *mut Object); - debug_assert_eq!(rtn, obj as *const T as *mut Object); + let rtn = runtime::objc_retain(ptr as *mut Object) as *const T; + debug_assert_eq!(rtn, ptr); Self { - ptr: obj.into(), + // SAFETY: Non-null upheld by the caller and `objc_retain` always + // returns the same pointer. + ptr: NonNull::new_unchecked(rtn as *mut T), phantom: PhantomData, } } @@ -234,7 +244,7 @@ impl Clone for Retained { #[inline] fn clone(&self) -> Self { // SAFETY: The `ptr` is guaranteed to be valid - unsafe { Self::retain(&*self) } + unsafe { Self::retain(self.as_ptr()) } } } @@ -312,7 +322,6 @@ impl From> for Retained { #[cfg(test)] mod tests { use core::mem::size_of; - use core::ptr::NonNull; use super::Retained; use crate::runtime::Object; From 5846dd583920186988c763e291cf9393f60d05de Mon Sep 17 00:00:00 2001 From: Mads Marquart Date: Mon, 31 May 2021 10:05:06 +0200 Subject: [PATCH 12/14] Make autoreleasepool take the pool as a parameter The lifetime of the parameter is the lifetime of references in the pool. This allows us to bound lifetimes on autoreleased objects, thereby making them safe to return. --- src/rc/autorelease.rs | 119 +++++++++++++++++++++++++++++++++++++----- src/rc/mod.rs | 12 ++--- src/rc/owned.rs | 15 +++++- src/rc/retained.rs | 28 +++++----- 4 files changed, 139 insertions(+), 35 deletions(-) diff --git a/src/rc/autorelease.rs b/src/rc/autorelease.rs index 17df8af39..c8931a4de 100644 --- a/src/rc/autorelease.rs +++ b/src/rc/autorelease.rs @@ -1,30 +1,121 @@ +use crate::runtime::{objc_autoreleasePoolPop, objc_autoreleasePoolPush}; use std::os::raw::c_void; -use crate::runtime::{objc_autoreleasePoolPush, objc_autoreleasePoolPop}; -// we use a struct to ensure that objc_autoreleasePoolPop during unwinding. -struct AutoReleaseHelper { +/// An Objective-C autorelease pool. +/// +/// The pool is drained when dropped. +/// +/// This is not `Send`, since `objc_autoreleasePoolPop` must be called on the +/// same thread. +/// +/// And this is not `Sync`, since you can only autorelease a reference to a +/// pool on the current thread. +/// +/// See [the clang documentation][clang-arc] and +/// [this apple article][memory-mgmt] for more information on automatic +/// reference counting. +/// +/// [clang-arc]: https://clang.llvm.org/docs/AutomaticReferenceCounting.html +/// [memory-mgmt]: https://developer.apple.com/library/archive/documentation/Cocoa/Conceptual/MemoryMgmt/Articles/MemoryMgmt.html +pub struct AutoreleasePool { context: *mut c_void, } -impl AutoReleaseHelper { +impl AutoreleasePool { + /// Construct a new autoreleasepool. + /// + /// Use the [`autoreleasepool`] block for a safe alternative. + /// + /// # Safety + /// + /// The caller must ensure that when handing out `&'p AutoreleasePool` to + /// functions that this is the innermost pool. + /// + /// Additionally, the pools must be dropped in the same order they were + /// created. + #[doc(alias = "objc_autoreleasePoolPush")] unsafe fn new() -> Self { - AutoReleaseHelper { context: objc_autoreleasePoolPush() } + AutoreleasePool { + context: objc_autoreleasePoolPush(), + } } + + // TODO: Add helper functions to ensure (with debug_assertions) that the + // pool is innermost when its lifetime is tied to a reference. } -impl Drop for AutoReleaseHelper { +impl Drop for AutoreleasePool { + /// Drains the autoreleasepool. + #[doc(alias = "objc_autoreleasePoolPop")] fn drop(&mut self) { unsafe { objc_autoreleasePoolPop(self.context) } } } -/** -Execute `f` in the context of a new autorelease pool. The pool is drained -after the execution of `f` completes. +// TODO: +// #![feature(negative_impls)] +// #![feature(auto_traits)] +// /// A trait for the sole purpose of ensuring we can't pass an `&AutoreleasePool` +// /// through to the closure inside `autoreleasepool` +// pub unsafe auto trait AutoreleaseSafe {} +// // TODO: Unsure how negative impls work exactly +// unsafe impl !AutoreleaseSafe for AutoreleasePool {} +// unsafe impl !AutoreleaseSafe for &AutoreleasePool {} +// unsafe impl !AutoreleaseSafe for &mut AutoreleasePool {} -This corresponds to `@autoreleasepool` blocks in Objective-C and Swift. -*/ -pub fn autoreleasepool T>(f: F) -> T { - let _context = unsafe { AutoReleaseHelper::new() }; - f() +/// Execute `f` in the context of a new autorelease pool. The pool is drained +/// after the execution of `f` completes. +/// +/// This corresponds to `@autoreleasepool` blocks in Objective-C and Swift. +/// +/// The pool is passed as a reference to the enclosing function to give it a +/// lifetime parameter that autoreleased objects can refer to. +/// +/// # Examples +/// +/// ```rust +/// use objc::{class, msg_send}; +/// use objc::rc::{autoreleasepool, AutoreleasePool, Owned}; +/// use objc::runtime::Object; +/// +/// fn needs_lifetime_from_pool<'p>(pool: &'p AutoreleasePool) -> &'p mut Object { +/// let obj: Owned = unsafe { Owned::new(msg_send![class!(NSObject), new]) }; +/// obj.autorelease(pool) +/// } +/// +/// autoreleasepool(|pool| { +/// let obj = needs_lifetime_from_pool(pool); +/// // Use `obj` +/// }); +/// +/// // `obj` is deallocated when the pool ends +/// ``` +/// +/// ```rust,compile_fail +/// # use objc::{class, msg_send}; +/// # use objc::rc::{autoreleasepool, AutoreleasePool, Owned}; +/// # use objc::runtime::Object; +/// # +/// # fn needs_lifetime_from_pool<'p>(pool: &'p AutoreleasePool) -> &'p mut Object { +/// # let obj: Owned = unsafe { Owned::new(msg_send![class!(NSObject), new]) }; +/// # obj.autorelease(pool) +/// # } +/// # +/// // Fails to compile because `obj` does not live long enough for us to +/// // safely take it out of the pool. +/// +/// let obj = autoreleasepool(|pool| { +/// let obj = needs_lifetime_from_pool(pool); +/// // Use `obj` +/// obj +/// }); +/// ``` +/// +/// TODO: More examples. +pub fn autoreleasepool(f: F) -> T +where + for<'p> F: FnOnce(&'p AutoreleasePool) -> T, // + AutoreleaseSafe, +{ + let pool = unsafe { AutoreleasePool::new() }; + f(&pool) } diff --git a/src/rc/mod.rs b/src/rc/mod.rs index 57d81ef50..599bb67a1 100644 --- a/src/rc/mod.rs +++ b/src/rc/mod.rs @@ -40,17 +40,17 @@ assert!(weak.load().is_null()); ``` */ +mod autorelease; mod owned; mod retained; mod strong; mod weak; -mod autorelease; -pub use self::retained::Retained; +pub use self::autorelease::{autoreleasepool, AutoreleasePool}; pub use self::owned::Owned; +pub use self::retained::Retained; pub use self::strong::StrongPtr; pub use self::weak::WeakPtr; -pub use self::autorelease::autoreleasepool; // These tests use NSObject, which isn't present for GNUstep #[cfg(all(test, any(target_os = "macos", target_os = "ios")))] @@ -97,9 +97,9 @@ mod tests { } let cloned = obj.clone(); - autoreleasepool(|| { - obj.autorelease(); - assert!(retain_count(*cloned) == 2); + autoreleasepool(|_| { + obj.autorelease(); + assert!(retain_count(*cloned) == 2); }); // make sure that the autoreleased value has been released diff --git a/src/rc/owned.rs b/src/rc/owned.rs index f9dc7f578..cfdb2f741 100644 --- a/src/rc/owned.rs +++ b/src/rc/owned.rs @@ -6,8 +6,8 @@ use core::mem; use core::ops::{Deref, DerefMut}; use core::ptr::{drop_in_place, NonNull}; +use super::AutoreleasePool; use super::Retained; -use crate::runtime::{self, Object}; /// A smart pointer that strongly references and uniquely owns an Objective-C /// object. @@ -106,6 +106,19 @@ impl Owned { phantom: PhantomData, } } + + /// Autoreleases the retained pointer, meaning that the object is not + /// immediately released, but will be when the innermost / current + /// autorelease pool is drained. + #[doc(alias = "objc_autorelease")] + #[must_use = "If you don't intend to use the object any more, just drop it as usual"] + #[inline] + pub fn autorelease<'p>(self, pool: &'p AutoreleasePool) -> &'p mut T { + let retained: Retained = self.into(); + let ptr = retained.autorelease(pool) as *const T as *mut T; + // SAFETY: The pointer was previously `Owned`, so is safe to be mutable + unsafe { &mut *ptr } + } } /// `#[may_dangle]` (see [this][dropck_eyepatch]) would not be safe here, diff --git a/src/rc/retained.rs b/src/rc/retained.rs index 6bce56425..406b41257 100644 --- a/src/rc/retained.rs +++ b/src/rc/retained.rs @@ -6,6 +6,7 @@ use core::mem; use core::ops::Deref; use core::ptr::NonNull; +use super::AutoreleasePool; use super::Owned; use crate::runtime::{self, Object}; @@ -153,59 +154,58 @@ impl Retained { /// TODO #[doc(alias = "objc_retainAutoreleasedReturnValue")] - pub unsafe fn retain_autoreleased_return(obj: &T) -> Self { + pub unsafe fn retain_autoreleased_return(_obj: *const T) -> Self { todo!() } /// Autoreleases the retained pointer, meaning that the object is not /// immediately released, but will be when the innermost / current /// autorelease pool is drained. - /// - /// A pointer to the object is returned, but it's validity is only until - /// guaranteed until the innermost pool is drained. #[doc(alias = "objc_autorelease")] #[must_use = "If you don't intend to use the object any more, just drop it as usual"] #[inline] - // TODO: Get a lifetime relating to the pool, so that we can return a - // reference instead of a pointer. - pub fn autorelease(self) -> NonNull { + pub fn autorelease<'p>(self, _pool: &'p AutoreleasePool) -> &'p T { let ptr = mem::ManuallyDrop::new(self).ptr; // SAFETY: The `ptr` is guaranteed to be valid and have at least one // retain count. // And because of the ManuallyDrop, we don't call the Drop // implementation, so the object won't also be released there. unsafe { runtime::objc_autorelease(ptr.as_ptr() as *mut Object) }; - ptr + // SAFETY: The lifetime is bounded by the type function signature + unsafe { &*ptr.as_ptr() } } /// TODO #[doc(alias = "objc_autoreleaseReturnValue")] - pub fn autorelease_return(self) -> *const T { + pub fn autorelease_return<'p>(self, _pool: &'p AutoreleasePool) -> &'p T { todo!() } /// TODO /// - /// Equivalent to `Retained::retain(&obj).autorelease()`, but slightly + /// Equivalent to `Retained::retain(&obj).autorelease(pool)`, but slightly /// more efficient. #[doc(alias = "objc_retainAutorelease")] - pub unsafe fn retain_and_autorelease(obj: &T) -> *const T { + pub unsafe fn retain_and_autorelease<'p>(_obj: *const T, _pool: &'p AutoreleasePool) -> &'p T { todo!() } /// TODO /// - /// Equivalent to `Retained::retain(&obj).autorelease_return()`, but + /// Equivalent to `Retained::retain(&obj).autorelease_return(pool)`, but /// slightly more efficient. #[doc(alias = "objc_retainAutoreleaseReturnValue")] - pub unsafe fn retain_and_autorelease_return(obj: &T) -> *const T { + pub unsafe fn retain_and_autorelease_return<'p>( + _obj: *const T, + _pool: &'p AutoreleasePool, + ) -> &'p T { todo!() } #[cfg(test)] // TODO #[doc(alias = "retainCount")] pub fn retain_count(&self) -> usize { - unsafe { msg_send![self.ptr.as_ptr() as *mut Object, retainCount] } + unsafe { msg_send![self.as_ptr() as *mut Object, retainCount] } } } From 4421537efe2191c11d8928e63dfed1b369e1f042 Mon Sep 17 00:00:00 2001 From: Mads Marquart Date: Mon, 31 May 2021 15:55:20 +0200 Subject: [PATCH 13/14] Fix double free when creating Retained from Owned --- src/rc/owned.rs | 8 ++++---- src/rc/retained.rs | 13 +++++++------ 2 files changed, 11 insertions(+), 10 deletions(-) diff --git a/src/rc/owned.rs b/src/rc/owned.rs index cfdb2f741..37117da86 100644 --- a/src/rc/owned.rs +++ b/src/rc/owned.rs @@ -134,11 +134,11 @@ impl Drop for Owned { /// on the contained object as well. #[inline] fn drop(&mut self) { - let ptr = self.ptr; + let ptr = self.as_ptr(); unsafe { - drop_in_place(ptr.as_ptr()); + drop_in_place(ptr); // Construct a new `Retained`, which will be dropped immediately - Retained::new(ptr.as_ref()); + Retained::new(ptr); }; } } @@ -177,7 +177,7 @@ impl fmt::Debug for Owned { impl fmt::Pointer for Owned { fn fmt(&self, f: &mut fmt::Formatter) -> fmt::Result { - fmt::Pointer::fmt(&self.ptr.as_ptr(), f) + fmt::Pointer::fmt(&self.as_ptr(), f) } } diff --git a/src/rc/retained.rs b/src/rc/retained.rs index 406b41257..11cf5107e 100644 --- a/src/rc/retained.rs +++ b/src/rc/retained.rs @@ -165,14 +165,14 @@ impl Retained { #[must_use = "If you don't intend to use the object any more, just drop it as usual"] #[inline] pub fn autorelease<'p>(self, _pool: &'p AutoreleasePool) -> &'p T { - let ptr = mem::ManuallyDrop::new(self).ptr; + let ptr = mem::ManuallyDrop::new(self).as_ptr(); // SAFETY: The `ptr` is guaranteed to be valid and have at least one // retain count. // And because of the ManuallyDrop, we don't call the Drop // implementation, so the object won't also be released there. - unsafe { runtime::objc_autorelease(ptr.as_ptr() as *mut Object) }; + unsafe { runtime::objc_autorelease(ptr as *mut Object) }; // SAFETY: The lifetime is bounded by the type function signature - unsafe { &*ptr.as_ptr() } + unsafe { &*ptr } } /// TODO @@ -231,7 +231,7 @@ impl Drop for Retained { fn drop(&mut self) { // SAFETY: The `ptr` is guaranteed to be valid and have at least one // retain count - unsafe { runtime::objc_release(self.ptr.as_ptr() as *mut Object) }; + unsafe { runtime::objc_release(self.as_ptr() as *mut Object) }; } } @@ -286,7 +286,7 @@ impl fmt::Debug for Retained { impl fmt::Pointer for Retained { fn fmt(&self, f: &mut fmt::Formatter) -> fmt::Result { - fmt::Pointer::fmt(&self.ptr.as_ptr(), f) + fmt::Pointer::fmt(&self.as_ptr(), f) } } @@ -314,8 +314,9 @@ impl Unpin for Retained {} impl From> for Retained { fn from(obj: Owned) -> Self { + let ptr = mem::ManuallyDrop::new(obj).as_ptr(); // SAFETY: TODO - unsafe { Self::new(&*obj) } + unsafe { Self::new(ptr) } } } From d067bbd8d6da8a0ff56b3819dd11dcd10c15148a Mon Sep 17 00:00:00 2001 From: Mads Marquart Date: Mon, 31 May 2021 16:27:02 +0200 Subject: [PATCH 14/14] Inline when creating Retained from Owned --- src/rc/retained.rs | 1 + 1 file changed, 1 insertion(+) diff --git a/src/rc/retained.rs b/src/rc/retained.rs index 11cf5107e..1db7258d6 100644 --- a/src/rc/retained.rs +++ b/src/rc/retained.rs @@ -313,6 +313,7 @@ impl AsRef for Retained { impl Unpin for Retained {} impl From> for Retained { + #[inline] fn from(obj: Owned) -> Self { let ptr = mem::ManuallyDrop::new(obj).as_ptr(); // SAFETY: TODO