Skip to content

Commit

Permalink
Merge pull request #274 from madsmtm/fix-leaks
Browse files Browse the repository at this point in the history
Run AddressSanitizer and fix leak
  • Loading branch information
madsmtm authored Oct 5, 2022
2 parents bb1871f + 1fc1b39 commit 077a964
Show file tree
Hide file tree
Showing 6 changed files with 26 additions and 9 deletions.
1 change: 1 addition & 0 deletions objc2/src/declare/ivar.rs
Original file line number Diff line number Diff line change
Expand Up @@ -410,6 +410,7 @@ mod tests {
#[sel(dealloc)]
fn dealloc(&mut self) {
HAS_RUN_DEALLOC.store(true, Ordering::SeqCst);
unsafe { msg_send![super(self), dealloc] }
}
}
);
Expand Down
1 change: 1 addition & 0 deletions objc2/src/foundation/error.rs
Original file line number Diff line number Diff line change
Expand Up @@ -75,6 +75,7 @@ extern_methods!(
}

pub fn localized_description(&self) -> Id<NSString, Shared> {
// TODO: For some reason this leaks a lot?
let obj: Option<_> = unsafe { msg_send_id![self, localizedDescription] };
obj.expect(
"unexpected NULL localized description; a default should have been generated!",
Expand Down
18 changes: 11 additions & 7 deletions objc2/src/foundation/thread.rs
Original file line number Diff line number Diff line change
Expand Up @@ -214,14 +214,18 @@ mod tests {
let thread = NSThread::main();

let actual = format!("{:?}", thread);
let expected_macos_11 = format!("<NSThread: {:p}>{{number = 1, name = (null)}}", thread);
let expected_macos_12 =
format!("<_NSMainThread: {:p}>{{number = 1, name = (null)}}", thread);
let expected = [
// macOS 11
format!("<NSThread: {:p}>{{number = 1, name = (null)}}", thread),
format!("<NSThread: {:p}>{{number = 1, name = main}}", thread),
// macOS 12
format!("<_NSMainThread: {:p}>{{number = 1, name = (null)}}", thread),
format!("<_NSMainThread: {:p}>{{number = 1, name = main}}", thread),
];
assert!(
actual == expected_macos_11 || actual == expected_macos_12,
"Expected one of {:?} or {:?}, got {:?}",
expected_macos_11,
expected_macos_12,
expected.contains(&actual),
"Expected one of {:?}, got {:?}",
expected,
actual,
);

Expand Down
11 changes: 11 additions & 0 deletions objc2/src/macros/declare_class.rs
Original file line number Diff line number Diff line change
Expand Up @@ -410,6 +410,10 @@ macro_rules! declare_class {
}

impl $for {
// See the following links for more details:
// - <https://clang.llvm.org/docs/AutomaticReferenceCounting.html#dealloc>
// - <https://developer.apple.com/documentation/objectivec/nsobject/1571947-dealloc>
// - <https://developer.apple.com/library/archive/documentation/Cocoa/Conceptual/MemoryMgmt/Articles/mmRules.html#//apple_ref/doc/uid/20000994-SW2>
unsafe extern "C" fn __objc2_dealloc(&mut self, _cmd: $crate::runtime::Sel) {
$(
let ptr: *mut $crate::declare::Ivar<$ivar> = &mut self.$ivar;
Expand All @@ -418,6 +422,13 @@ macro_rules! declare_class {
// be touched again.
unsafe { $crate::__macro_helpers::drop_in_place(ptr) };
)*

// Invoke the super class' `dealloc` method.
//
// Note: ARC does this automatically, so most Objective-C code
// in the wild don't contain this; but we don't have ARC, so
// we must do this.
unsafe { msg_send![super(self), dealloc] }
}
}

Expand Down
3 changes: 2 additions & 1 deletion objc2/src/rc/test_object.rs
Original file line number Diff line number Diff line change
Expand Up @@ -101,6 +101,7 @@ declare_class!(

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

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

#[sel(_tryRetain)]
Expand Down
1 change: 0 additions & 1 deletion objc2/src/rc/weak_id.rs
Original file line number Diff line number Diff line change
Expand Up @@ -175,7 +175,6 @@ mod tests {
if cfg!(not(feature = "gnustep-1-7")) {
// This loads the object on GNUStep for some reason??
assert!(weak.load().is_none());
expected.try_retain_fail += 1;
expected.assert_current();
}

Expand Down

0 comments on commit 077a964

Please sign in to comment.