1
0
Fork 0

NSView lifetime fixes (#104)

* drain autorelease pools in Window::open_* methods

* fixes to NSView lifetime logic:

- in open_parented and open_blocking, release NSView after adding
  it as a subview of the parent
- in open_blocking, don't call autorelease on NSWindow. previously
  it was a no-op, but now that we are actually draining our
  autorelease pools, it ends up prematurely releasing the window.

* fixes to NSView cleanup logic:

- Move retainCount check to before calling [super release].
  If [super release] happens first, then in the final call to
  release, [super release] deallocates the object and the call to
  retainCount results in a segfault.

- Move objc_disposeClassPair to dealloc. Previously we were
  calling it when retainCount == 1, but that's exactly when
  dealloc is called, so this is cleaner. Also, we need to call
  objc_disposeClassPair after [super dealloc].

NOTE: The circular-reference-breaking logic in release is
definitely broken. It's easy to thwart it by e.g. creating a
wgpu surface at some point after build() or dropping one at some
point before drop(). Need to come up with a better solution.
This commit is contained in:
glowcoil 2021-11-07 16:57:12 -06:00 committed by GitHub
parent 6172090be3
commit 9fbfe18f9a
No known key found for this signature in database
GPG key ID: 4AEE18F83AFDEB23
2 changed files with 46 additions and 17 deletions

View file

@ -119,6 +119,10 @@ unsafe fn create_view_class() -> &'static Class {
sel!(release), sel!(release),
release as extern "C" fn(&mut Object, Sel) release as extern "C" fn(&mut Object, Sel)
); );
class.add_method(
sel!(dealloc),
dealloc as extern "C" fn(&mut Object, Sel)
);
class.add_method( class.add_method(
sel!(viewWillMoveToWindow:), sel!(viewWillMoveToWindow:),
view_will_move_to_window as extern "C" fn(&Object, Sel, id) view_will_move_to_window as extern "C" fn(&Object, Sel, id)
@ -229,11 +233,17 @@ extern "C" fn accepts_first_mouse(
extern "C" fn release(this: &mut Object, _sel: Sel) { extern "C" fn release(this: &mut Object, _sel: Sel) {
unsafe { // Hack for breaking circular references. We store the value of retainCount
let superclass = msg_send![this, superclass]; // after build(), and then when retainCount drops back to that value, we
// drop the WindowState, hoping that any circular references it holds back
let () = msg_send![super(this, superclass), release]; // to the NSView (e.g. wgpu surfaces) get released.
} //
// This is definitely broken, since it can be thwarted by e.g. creating a
// wgpu surface at some point after build() (which will mean the NSView
// never gets dealloced) or dropping a wgpu surface at some point before
// drop() (which will mean the WindowState gets dropped early).
//
// TODO: Find a better solution for circular references.
unsafe { unsafe {
let retain_count: usize = msg_send![this, retainCount]; let retain_count: usize = msg_send![this, retainCount];
@ -257,11 +267,23 @@ extern "C" fn release(this: &mut Object, _sel: Sel) {
} }
} }
if retain_count == 1 { }
// Delete class
let class = msg_send![this, class]; unsafe {
::objc::runtime::objc_disposeClassPair(class); let superclass = msg_send![this, superclass];
} let () = msg_send![super(this, superclass), release];
}
}
extern "C" fn dealloc(this: &mut Object, _sel: Sel) {
unsafe {
let class = msg_send![this, class];
let superclass = msg_send![this, superclass];
let () = msg_send![super(this, superclass), dealloc];
// Delete class
::objc::runtime::objc_disposeClassPair(class);
} }
} }

View file

@ -41,7 +41,7 @@ impl Window {
B: FnOnce(&mut crate::Window) -> H, B: FnOnce(&mut crate::Window) -> H,
B: Send + 'static, B: Send + 'static,
{ {
let _pool = unsafe { NSAutoreleasePool::new(nil) }; let pool = unsafe { NSAutoreleasePool::new(nil) };
let handle = if let RawWindowHandle::MacOS(handle) = parent.raw_window_handle() { let handle = if let RawWindowHandle::MacOS(handle) = parent.raw_window_handle() {
handle handle
@ -60,6 +60,9 @@ impl Window {
unsafe { unsafe {
let _: id = msg_send![handle.ns_view as *mut Object, addSubview: ns_view]; let _: id = msg_send![handle.ns_view as *mut Object, addSubview: ns_view];
let () = msg_send![ns_view as id, release];
let () = msg_send![pool, drain];
} }
} }
@ -69,7 +72,7 @@ impl Window {
B: FnOnce(&mut crate::Window) -> H, B: FnOnce(&mut crate::Window) -> H,
B: Send + 'static, B: Send + 'static,
{ {
let _pool = unsafe { NSAutoreleasePool::new(nil) }; let pool = unsafe { NSAutoreleasePool::new(nil) };
let ns_view = unsafe { create_view(&options) }; let ns_view = unsafe { create_view(&options) };
@ -82,6 +85,10 @@ impl Window {
Self::init(window, build); Self::init(window, build);
unsafe {
let () = msg_send![pool, drain];
}
raw_window_handle raw_window_handle
} }
@ -91,7 +98,7 @@ impl Window {
B: FnOnce(&mut crate::Window) -> H, B: FnOnce(&mut crate::Window) -> H,
B: Send + 'static, B: Send + 'static,
{ {
let _pool = unsafe { NSAutoreleasePool::new(nil) }; let pool = unsafe { NSAutoreleasePool::new(nil) };
// It seems prudent to run NSApp() here before doing other // It seems prudent to run NSApp() here before doing other
// work. It runs [NSApplication sharedApplication], which is // work. It runs [NSApplication sharedApplication], which is
@ -131,8 +138,7 @@ impl Window {
NSWindowStyleMask::NSTitledWindowMask, NSWindowStyleMask::NSTitledWindowMask,
NSBackingStoreBuffered, NSBackingStoreBuffered,
NO, NO,
) );
.autorelease();
ns_window.center(); ns_window.center();
let title = NSString::alloc(nil) let title = NSString::alloc(nil)
@ -156,9 +162,10 @@ impl Window {
unsafe { unsafe {
ns_window.setContentView_(ns_view); ns_window.setContentView_(ns_view);
} let () = msg_send![ns_view as id, release];
let () = msg_send![pool, drain];
unsafe {
app.run(); app.run();
} }
} }