Skip to content

Commit 1fc1b39

Browse files
committed
Run AddressSanitizer and fix leaks
Notably, I incorrectly assumed that the super class' `dealloc` method is called in custom `dealloc` methods, but this is only true in Objective-C with ARC enabled!
1 parent bb1871f commit 1fc1b39

File tree

6 files changed

+26
-9
lines changed

6 files changed

+26
-9
lines changed

objc2/src/declare/ivar.rs

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -410,6 +410,7 @@ mod tests {
410410
#[sel(dealloc)]
411411
fn dealloc(&mut self) {
412412
HAS_RUN_DEALLOC.store(true, Ordering::SeqCst);
413+
unsafe { msg_send![super(self), dealloc] }
413414
}
414415
}
415416
);

objc2/src/foundation/error.rs

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -75,6 +75,7 @@ extern_methods!(
7575
}
7676

7777
pub fn localized_description(&self) -> Id<NSString, Shared> {
78+
// TODO: For some reason this leaks a lot?
7879
let obj: Option<_> = unsafe { msg_send_id![self, localizedDescription] };
7980
obj.expect(
8081
"unexpected NULL localized description; a default should have been generated!",

objc2/src/foundation/thread.rs

Lines changed: 11 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -214,14 +214,18 @@ mod tests {
214214
let thread = NSThread::main();
215215

216216
let actual = format!("{:?}", thread);
217-
let expected_macos_11 = format!("<NSThread: {:p}>{{number = 1, name = (null)}}", thread);
218-
let expected_macos_12 =
219-
format!("<_NSMainThread: {:p}>{{number = 1, name = (null)}}", thread);
217+
let expected = [
218+
// macOS 11
219+
format!("<NSThread: {:p}>{{number = 1, name = (null)}}", thread),
220+
format!("<NSThread: {:p}>{{number = 1, name = main}}", thread),
221+
// macOS 12
222+
format!("<_NSMainThread: {:p}>{{number = 1, name = (null)}}", thread),
223+
format!("<_NSMainThread: {:p}>{{number = 1, name = main}}", thread),
224+
];
220225
assert!(
221-
actual == expected_macos_11 || actual == expected_macos_12,
222-
"Expected one of {:?} or {:?}, got {:?}",
223-
expected_macos_11,
224-
expected_macos_12,
226+
expected.contains(&actual),
227+
"Expected one of {:?}, got {:?}",
228+
expected,
225229
actual,
226230
);
227231

objc2/src/macros/declare_class.rs

Lines changed: 11 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -410,6 +410,10 @@ macro_rules! declare_class {
410410
}
411411

412412
impl $for {
413+
// See the following links for more details:
414+
// - <https://clang.llvm.org/docs/AutomaticReferenceCounting.html#dealloc>
415+
// - <https://developer.apple.com/documentation/objectivec/nsobject/1571947-dealloc>
416+
// - <https://developer.apple.com/library/archive/documentation/Cocoa/Conceptual/MemoryMgmt/Articles/mmRules.html#//apple_ref/doc/uid/20000994-SW2>
413417
unsafe extern "C" fn __objc2_dealloc(&mut self, _cmd: $crate::runtime::Sel) {
414418
$(
415419
let ptr: *mut $crate::declare::Ivar<$ivar> = &mut self.$ivar;
@@ -418,6 +422,13 @@ macro_rules! declare_class {
418422
// be touched again.
419423
unsafe { $crate::__macro_helpers::drop_in_place(ptr) };
420424
)*
425+
426+
// Invoke the super class' `dealloc` method.
427+
//
428+
// Note: ARC does this automatically, so most Objective-C code
429+
// in the wild don't contain this; but we don't have ARC, so
430+
// we must do this.
431+
unsafe { msg_send![super(self), dealloc] }
421432
}
422433
}
423434

objc2/src/rc/test_object.rs

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -101,6 +101,7 @@ declare_class!(
101101

102102
#[sel(initReturningNull)]
103103
fn init_returning_null(&mut self) -> *mut Self {
104+
let _: () = unsafe { msg_send![self, release] };
104105
ptr::null_mut()
105106
}
106107

@@ -125,7 +126,7 @@ declare_class!(
125126
#[sel(dealloc)]
126127
unsafe fn dealloc(&mut self) {
127128
TEST_DATA.with(|data| data.borrow_mut().dealloc += 1);
128-
// Don't call superclass
129+
unsafe { msg_send![super(self), dealloc] }
129130
}
130131

131132
#[sel(_tryRetain)]

objc2/src/rc/weak_id.rs

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -175,7 +175,6 @@ mod tests {
175175
if cfg!(not(feature = "gnustep-1-7")) {
176176
// This loads the object on GNUStep for some reason??
177177
assert!(weak.load().is_none());
178-
expected.try_retain_fail += 1;
179178
expected.assert_current();
180179
}
181180

0 commit comments

Comments
 (0)