From 1bffc9fea2888c7fcff42f346da38499a6132ad2 Mon Sep 17 00:00:00 2001 From: Mads Marquart Date: Sun, 31 Jul 2022 23:35:58 +0200 Subject: [PATCH] Test reference counting in NSValue, NSArray and NSMutableArray --- objc2/src/__macro_helpers.rs | 25 ++-- objc2/src/foundation/array.rs | 79 ++++++++++--- objc2/src/foundation/mutable_array.rs | 49 +++++--- objc2/src/foundation/value.rs | 24 +++- objc2/src/macros/declare_class.rs | 8 +- objc2/src/rc/test_object.rs | 160 ++++++++++++-------------- 6 files changed, 216 insertions(+), 129 deletions(-) diff --git a/objc2/src/__macro_helpers.rs b/objc2/src/__macro_helpers.rs index eb1081691..a2cb7bd69 100644 --- a/objc2/src/__macro_helpers.rs +++ b/objc2/src/__macro_helpers.rs @@ -274,15 +274,13 @@ mod tests { #[test] fn test_alloc_with_zone() { - let expected = ThreadTestData::current(); + let mut expected = ThreadTestData::current(); let cls = RcTestObject::class(); let zone: *const NSZone = ptr::null(); let _obj: Id, Owned> = unsafe { msg_send_id![cls, allocWithZone: zone].unwrap() }; - // `+[NSObject alloc]` delegates to `+[NSObject allocWithZone:]`, but - // `RcTestObject` only catches `alloc`. - // expected.alloc += 1; + expected.alloc += 1; expected.assert_current(); } @@ -325,9 +323,18 @@ mod tests { expected.init += 1; expected.assert_current(); - // TODO: - // let copy: Id = unsafe { msg_send_id![&obj, copy].unwrap() }; - // let mutable_copy: Id = unsafe { msg_send_id![&obj, mutableCopy].unwrap() }; + let _copy: Id = unsafe { msg_send_id![&obj, copy].unwrap() }; + expected.copy += 1; + expected.alloc += 1; + expected.init += 1; + expected.assert_current(); + + let _mutable_copy: Id = + unsafe { msg_send_id![&obj, mutableCopy].unwrap() }; + expected.mutable_copy += 1; + expected.alloc += 1; + expected.init += 1; + expected.assert_current(); let _self: Id = unsafe { msg_send_id![&obj, self].unwrap() }; expected.retain += 1; @@ -337,8 +344,8 @@ mod tests { unsafe { msg_send_id![&obj, description] }; expected.assert_current(); }); - expected.release += 3; - expected.dealloc += 2; + expected.release += 5; + expected.dealloc += 4; expected.assert_current(); } diff --git a/objc2/src/foundation/array.rs b/objc2/src/foundation/array.rs index 6ae44109c..d66491363 100644 --- a/objc2/src/foundation/array.rs +++ b/objc2/src/foundation/array.rs @@ -244,7 +244,7 @@ mod tests { use super::*; use crate::foundation::{NSNumber, NSString}; - use crate::rc::autoreleasepool; + use crate::rc::{RcTestObject, ThreadTestData}; fn sample_array(len: usize) -> Id, Owned> { let mut vec = Vec::with_capacity(len); @@ -262,10 +262,6 @@ mod tests { NSArray::from_vec(vec) } - fn retain_count(obj: &NSObject) -> usize { - unsafe { msg_send![obj, retainCount] } - } - #[test] fn test_two_empty() { let _empty_array1 = NSArray::::new(); @@ -317,25 +313,76 @@ mod tests { } #[test] - fn test_get_does_not_autorelease() { - let obj: Id<_, Shared> = NSObject::new().into(); + fn test_retains_stored() { + let obj = Id::from_owned(RcTestObject::new()); + let mut expected = ThreadTestData::current(); + + let input = [obj.clone(), obj.clone()]; + expected.retain += 2; + expected.assert_current(); - assert_eq!(retain_count(&obj), 1); + let array = NSArray::from_slice(&input); + expected.retain += 2; + expected.assert_current(); - let array = NSArray::from_slice(&[obj.clone()]); + let _obj = array.first().unwrap(); + expected.assert_current(); - assert_eq!(retain_count(&obj), 2); + drop(array); + expected.release += 2; + expected.assert_current(); - autoreleasepool(|_pool| { - let obj2 = array.first().unwrap(); - assert_eq!(retain_count(obj2), 2); - }); + let array = NSArray::from_vec(Vec::from(input)); + expected.retain += 2; + expected.release += 2; + expected.assert_current(); - assert_eq!(retain_count(&obj), 2); + let _obj = array.get(0).unwrap(); + let _obj = array.get(1).unwrap(); + assert!(array.get(2).is_none()); + expected.assert_current(); drop(array); + expected.release += 2; + expected.assert_current(); + + drop(obj); + expected.release += 1; + expected.dealloc += 1; + expected.assert_current(); + } - assert_eq!(retain_count(&obj), 1); + #[test] + fn test_nscopying_uses_retain() { + let obj = Id::from_owned(RcTestObject::new()); + let array = NSArray::from_slice(&[obj]); + let mut expected = ThreadTestData::current(); + + let _copy = array.copy(); + expected.assert_current(); + + let _copy = array.mutable_copy(); + expected.retain += 1; + expected.assert_current(); + } + + #[test] + fn test_iter_no_retain() { + let obj = Id::from_owned(RcTestObject::new()); + let array = NSArray::from_slice(&[obj]); + let mut expected = ThreadTestData::current(); + + let iter = array.iter(); + expected.retain += if cfg!(feature = "gnustep-1-7") { 0 } else { 1 }; + expected.assert_current(); + + assert_eq!(iter.count(), 1); + expected.autorelease += if cfg!(feature = "gnustep-1-7") { 0 } else { 1 }; + expected.assert_current(); + + let iter = array.iter_fast(); + assert_eq!(iter.count(), 1); + expected.assert_current(); } #[test] diff --git a/objc2/src/foundation/mutable_array.rs b/objc2/src/foundation/mutable_array.rs index 0fa392e74..edb919f5e 100644 --- a/objc2/src/foundation/mutable_array.rs +++ b/objc2/src/foundation/mutable_array.rs @@ -226,30 +226,41 @@ mod tests { use super::*; use crate::foundation::NSString; - use crate::rc::autoreleasepool; + use crate::rc::{autoreleasepool, RcTestObject, ThreadTestData}; #[test] fn test_adding() { let mut array = NSMutableArray::new(); - let obj = NSObject::new(); - array.push(obj); - + let obj1 = RcTestObject::new(); + let obj2 = RcTestObject::new(); + let mut expected = ThreadTestData::current(); + + array.push(obj1); + expected.retain += 1; + expected.release += 1; + expected.assert_current(); assert_eq!(array.len(), 1); assert_eq!(array.get(0), array.get(0)); - let obj = NSObject::new(); - array.insert(0, obj); + array.insert(0, obj2); + expected.retain += 1; + expected.release += 1; + expected.assert_current(); assert_eq!(array.len(), 2); } #[test] fn test_replace() { let mut array = NSMutableArray::new(); - let obj = NSObject::new(); - array.push(obj); - - let obj = NSObject::new(); - let old_obj = array.replace(0, obj); + let obj1 = RcTestObject::new(); + let obj2 = RcTestObject::new(); + array.push(obj1); + let mut expected = ThreadTestData::current(); + + let old_obj = array.replace(0, obj2); + expected.retain += 2; + expected.release += 2; + expected.assert_current(); assert_ne!(&*old_obj, array.get(0).unwrap()); } @@ -257,16 +268,26 @@ mod tests { fn test_remove() { let mut array = NSMutableArray::new(); for _ in 0..4 { - array.push(NSObject::new()); + array.push(RcTestObject::new()); } + let mut expected = ThreadTestData::current(); - let _ = array.remove(1); + let _obj = array.remove(1); + expected.retain += 1; + expected.release += 1; + expected.assert_current(); assert_eq!(array.len(), 3); - let _ = array.pop(); + let _obj = array.pop(); + expected.retain += 1; + expected.release += 1; + expected.assert_current(); assert_eq!(array.len(), 2); array.clear(); + expected.release += 2; + expected.dealloc += 2; + expected.assert_current(); assert_eq!(array.len(), 0); } diff --git a/objc2/src/foundation/value.rs b/objc2/src/foundation/value.rs index c081a1500..9df659e8f 100644 --- a/objc2/src/foundation/value.rs +++ b/objc2/src/foundation/value.rs @@ -227,9 +227,10 @@ impl fmt::Debug for NSValue { #[cfg(test)] mod tests { use alloc::format; - use core::slice; + use core::{ptr, slice}; use super::*; + use crate::rc::{RcTestObject, ThreadTestData}; #[test] fn basic() { @@ -237,6 +238,27 @@ mod tests { assert_eq!(unsafe { val.get::() }, 13); } + #[test] + fn does_not_retain() { + let obj = RcTestObject::new(); + let expected = ThreadTestData::current(); + + let val = NSValue::new::<*const RcTestObject>(&*obj); + expected.assert_current(); + + assert!(ptr::eq(unsafe { val.get::<*const RcTestObject>() }, &*obj)); + expected.assert_current(); + + let _clone = val.clone(); + expected.assert_current(); + + let _copy = val.copy(); + expected.assert_current(); + + drop(val); + expected.assert_current(); + } + #[test] fn test_equality() { let val1 = NSValue::new(123u32); diff --git a/objc2/src/macros/declare_class.rs b/objc2/src/macros/declare_class.rs index ac68bf9fa..ce1fcb7d3 100644 --- a/objc2/src/macros/declare_class.rs +++ b/objc2/src/macros/declare_class.rs @@ -516,8 +516,6 @@ macro_rules! declare_class { $v fn class() -> &'static $crate::runtime::Class { use $crate::__macro_helpers::Once; - use $crate::declare::ClassBuilder; - use $crate::runtime::{Class, Protocol}; static REGISTER_CLASS: Once = Once::new(); REGISTER_CLASS.call_once(|| { @@ -527,7 +525,7 @@ macro_rules! declare_class { stringify!($name), ". Perhaps a class with that name already exists?", ); - let mut builder = ClassBuilder::new(stringify!($name), superclass).expect(err_str); + let mut builder = $crate::declare::ClassBuilder::new(stringify!($name), superclass).expect(err_str); $( builder.add_ivar::<<$ivar as $crate::declare::IvarType>::Type>( @@ -539,7 +537,7 @@ macro_rules! declare_class { // Implement protocol if any specified $( let err_str = concat!("could not find protocol ", stringify!($protocol)); - builder.add_protocol(Protocol::get(stringify!($protocol)).expect(err_str)); + builder.add_protocol($crate::runtime::Protocol::get(stringify!($protocol)).expect(err_str)); )? // Implement methods @@ -558,7 +556,7 @@ macro_rules! declare_class { }); // We just registered the class, so it should be available - Class::get(stringify!($name)).unwrap() + $crate::runtime::Class::get(stringify!($name)).unwrap() } } diff --git a/objc2/src/rc/test_object.rs b/objc2/src/rc/test_object.rs index 2f37c1822..f9091c1cb 100644 --- a/objc2/src/rc/test_object.rs +++ b/objc2/src/rc/test_object.rs @@ -1,12 +1,11 @@ use core::cell::RefCell; -use core::ops::{Deref, DerefMut}; -use std::sync::Once; +use core::mem::ManuallyDrop; +use core::ptr; use super::{Id, Owned}; -use crate::declare::ClassBuilder; -use crate::runtime::{Bool, Class, Object, Sel}; -use crate::{class, msg_send, msg_send_bool, sel}; -use crate::{Encoding, Message, RefEncode}; +use crate::foundation::{NSObject, NSZone}; +use crate::runtime::Bool; +use crate::{declare_class, msg_send, msg_send_bool}; #[derive(Debug, Clone, Default, PartialEq, Eq)] pub(crate) struct ThreadTestData { @@ -14,6 +13,8 @@ pub(crate) struct ThreadTestData { pub(crate) dealloc: usize, pub(crate) init: usize, pub(crate) retain: usize, + pub(crate) copy: usize, + pub(crate) mutable_copy: usize, pub(crate) release: usize, pub(crate) autorelease: usize, pub(crate) try_retain: usize, @@ -47,96 +48,87 @@ std::thread_local! { pub(crate) static TEST_DATA: RefCell = RefCell::new(Default::default()); } -/// A helper object that counts how many times various reference-counting -/// primitives are called. -#[repr(C)] -pub(crate) struct RcTestObject { - inner: Object, -} +declare_class! { + /// A helper object that counts how many times various reference-counting + /// primitives are called. + #[derive(Debug, PartialEq)] + unsafe pub(crate) struct RcTestObject: NSObject {} + + unsafe impl { + #[sel(alloc)] + fn alloc() -> *mut Self { + TEST_DATA.with(|data| data.borrow_mut().alloc += 1); + let superclass = NSObject::class().metaclass(); + let zone: *const NSZone = ptr::null(); + unsafe { msg_send![super(Self::class(), superclass), allocWithZone: zone] } + } -unsafe impl RefEncode for RcTestObject { - const ENCODING_REF: Encoding<'static> = Object::ENCODING_REF; -} + #[sel(allocWithZone:)] + fn alloc_with_zone(zone: *const NSZone) -> *mut Self { + TEST_DATA.with(|data| data.borrow_mut().alloc += 1); + let superclass = NSObject::class().metaclass(); + unsafe { msg_send![super(Self::class(), superclass), allocWithZone: zone] } + } -unsafe impl Message for RcTestObject {} + #[sel(init)] + fn init(&mut self) -> *mut Self { + TEST_DATA.with(|data| data.borrow_mut().init += 1); + unsafe { msg_send![super(self, NSObject::class()), init] } + } -unsafe impl Send for RcTestObject {} -unsafe impl Sync for RcTestObject {} + #[sel(retain)] + fn retain(&self) -> *mut Self { + TEST_DATA.with(|data| data.borrow_mut().retain += 1); + unsafe { msg_send![super(self, NSObject::class()), retain] } + } -impl Deref for RcTestObject { - type Target = Object; - fn deref(&self) -> &Self::Target { - &self.inner - } -} + #[sel(release)] + fn release(&self) { + TEST_DATA.with(|data| data.borrow_mut().release += 1); + unsafe { msg_send![super(self, NSObject::class()), release] } + } -impl DerefMut for RcTestObject { - fn deref_mut(&mut self) -> &mut Self::Target { - &mut self.inner - } -} + #[sel(autorelease)] + fn autorelease(&self) -> *mut Self { + TEST_DATA.with(|data| data.borrow_mut().autorelease += 1); + unsafe { msg_send![super(self, NSObject::class()), autorelease] } + } -impl RcTestObject { - pub(crate) fn class() -> &'static Class { - static REGISTER_CLASS: Once = Once::new(); - - REGISTER_CLASS.call_once(|| { - extern "C" fn alloc(cls: &Class, _cmd: Sel) -> *mut RcTestObject { - TEST_DATA.with(|data| data.borrow_mut().alloc += 1); - let superclass = class!(NSObject).metaclass(); - unsafe { msg_send![super(cls, superclass), alloc] } - } - extern "C" fn init(this: &mut RcTestObject, _cmd: Sel) -> *mut RcTestObject { - TEST_DATA.with(|data| data.borrow_mut().init += 1); - unsafe { msg_send![super(this, class!(NSObject)), init] } - } - extern "C" fn retain(this: &RcTestObject, _cmd: Sel) -> *mut RcTestObject { - TEST_DATA.with(|data| data.borrow_mut().retain += 1); - unsafe { msg_send![super(this, class!(NSObject)), retain] } - } - extern "C" fn release(this: &RcTestObject, _cmd: Sel) { - TEST_DATA.with(|data| data.borrow_mut().release += 1); - unsafe { msg_send![super(this, class!(NSObject)), release] } - } - extern "C" fn autorelease(this: &RcTestObject, _cmd: Sel) -> *mut RcTestObject { - TEST_DATA.with(|data| data.borrow_mut().autorelease += 1); - unsafe { msg_send![super(this, class!(NSObject)), autorelease] } - } - unsafe extern "C" fn dealloc(_this: *mut RcTestObject, _cmd: Sel) { - TEST_DATA.with(|data| data.borrow_mut().dealloc += 1); - // Don't call superclass - } - unsafe extern "C" fn try_retain(this: &RcTestObject, _cmd: Sel) -> Bool { - TEST_DATA.with(|data| data.borrow_mut().try_retain += 1); - let res = unsafe { msg_send_bool![super(this, class!(NSObject)), _tryRetain] }; - if !res { - TEST_DATA.with(|data| data.borrow_mut().try_retain -= 1); - TEST_DATA.with(|data| data.borrow_mut().try_retain_fail += 1); - } - Bool::from(res) - } + #[sel(dealloc)] + unsafe fn dealloc(&mut self) { + TEST_DATA.with(|data| data.borrow_mut().dealloc += 1); + // Don't call superclass + } - let mut builder = ClassBuilder::new("RcTestObject", class!(NSObject)).unwrap(); - unsafe { - builder.add_class_method(sel!(alloc), alloc as extern "C" fn(_, _) -> _); - builder.add_method(sel!(init), init as extern "C" fn(_, _) -> _); - builder.add_method(sel!(retain), retain as extern "C" fn(_, _) -> _); - builder.add_method( - sel!(_tryRetain), - try_retain as unsafe extern "C" fn(_, _) -> _, - ); - builder.add_method(sel!(release), release as extern "C" fn(_, _)); - builder.add_method(sel!(autorelease), autorelease as extern "C" fn(_, _) -> _); - builder.add_method(sel!(dealloc), dealloc as unsafe extern "C" fn(_, _)); + #[sel(_tryRetain)] + unsafe fn try_retain(&self) -> Bool { + TEST_DATA.with(|data| data.borrow_mut().try_retain += 1); + let res = unsafe { msg_send_bool![super(self, NSObject::class()), _tryRetain] }; + if !res { + TEST_DATA.with(|data| data.borrow_mut().try_retain -= 1); + TEST_DATA.with(|data| data.borrow_mut().try_retain_fail += 1); } + Bool::from(res) + } - let _cls = builder.register(); - }); + #[sel(copyWithZone:)] + fn copy_with_zone(&self, _zone: *const NSZone) -> *const Self { + TEST_DATA.with(|data| data.borrow_mut().copy += 1); + Id::consume_as_ptr(ManuallyDrop::new(Self::new())) + } - // Can't use `class!` here since `RcTestObject` is dynamically created. - Class::get("RcTestObject").unwrap() + #[sel(mutableCopyWithZone:)] + fn mutable_copy_with_zone(&self, _zone: *const NSZone) -> *const Self { + TEST_DATA.with(|data| data.borrow_mut().mutable_copy += 1); + Id::consume_as_ptr(ManuallyDrop::new(Self::new())) + } } +} +unsafe impl Send for RcTestObject {} +unsafe impl Sync for RcTestObject {} + +impl RcTestObject { pub(crate) fn new() -> Id { // Use msg_send! - msg_send_id! is tested elsewhere! unsafe { Id::new(msg_send![Self::class(), new]) }.unwrap()