Skip to content

Commit

Permalink
Use objc's autoreleasepool instead of manually with NSAutoreleasePool (
Browse files Browse the repository at this point in the history
…rust-windowing#1936)

Ensures the pools are released even if we panic
  • Loading branch information
madsmtm committed May 27, 2021
1 parent 657b4fd commit 982ad46
Show file tree
Hide file tree
Showing 9 changed files with 47 additions and 58 deletions.
1 change: 0 additions & 1 deletion Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -48,7 +48,6 @@ cocoa = "0.24"
core-foundation = "0.9"
core-graphics = "0.22"
dispatch = "0.2.0"
scopeguard = "1.1"

[target.'cfg(target_os = "macos")'.dependencies.core-video-sys]
version = "0.1.4"
Expand Down
29 changes: 15 additions & 14 deletions src/platform_impl/macos/app_state.rs
Original file line number Diff line number Diff line change
Expand Up @@ -15,11 +15,12 @@ use std::{
use cocoa::{
appkit::{NSApp, NSApplication, NSWindow},
base::{id, nil},
foundation::{NSAutoreleasePool, NSSize},
foundation::NSSize,
};
use objc::{
rc::autoreleasepool,
runtime::{Object, YES},
};
use objc::runtime::YES;

use objc::runtime::Object;

use crate::{
dpi::LogicalSize,
Expand Down Expand Up @@ -403,16 +404,16 @@ impl AppState {
};

let dialog_is_closing = HANDLER.dialog_is_closing.load(Ordering::SeqCst);
let pool = NSAutoreleasePool::new(nil);
if !INTERRUPT_EVENT_LOOP_EXIT.load(Ordering::SeqCst)
&& !dialog_open
&& !dialog_is_closing
{
let () = msg_send![app, stop: nil];
// To stop event loop immediately, we need to post some event here.
post_dummy_event(app);
}
pool.drain();
autoreleasepool(|| {
if !INTERRUPT_EVENT_LOOP_EXIT.load(Ordering::SeqCst)
&& !dialog_open
&& !dialog_is_closing
{
let () = msg_send![app, stop: nil];
// To stop event loop immediately, we need to post some event here.
post_dummy_event(app);
}
});

if window_count > 0 {
let window: id = msg_send![windows, objectAtIndex:0];
Expand Down
17 changes: 7 additions & 10 deletions src/platform_impl/macos/event_loop.rs
Original file line number Diff line number Diff line change
Expand Up @@ -14,10 +14,9 @@ use std::{
use cocoa::{
appkit::{NSApp, NSEventType::NSApplicationDefined},
base::{id, nil, YES},
foundation::{NSAutoreleasePool, NSPoint},
foundation::NSPoint,
};

use scopeguard::defer;
use objc::rc::autoreleasepool;

use crate::{
event::Event,
Expand Down Expand Up @@ -115,9 +114,9 @@ impl<T> EventLoop<T> {
let app: id = msg_send![APP_CLASS.0, sharedApplication];

let delegate = IdRef::new(msg_send![APP_DELEGATE_CLASS.0, new]);
let pool = NSAutoreleasePool::new(nil);
let _: () = msg_send![app, setDelegate:*delegate];
let _: () = msg_send![pool, drain];
autoreleasepool(|| {
let _: () = msg_send![app, setDelegate:*delegate];
});
delegate
};
let panic_info: Rc<PanicInfo> = Default::default();
Expand Down Expand Up @@ -162,9 +161,7 @@ impl<T> EventLoop<T> {

self._callback = Some(Rc::clone(&callback));

unsafe {
let pool = NSAutoreleasePool::new(nil);
defer!(pool.drain());
autoreleasepool(|| unsafe {
let app = NSApp();
assert_ne!(app, nil);

Expand All @@ -180,7 +177,7 @@ impl<T> EventLoop<T> {
resume_unwind(panic);
}
AppState::exit();
}
});
}

pub fn create_proxy(&self) -> Proxy<T> {
Expand Down
11 changes: 6 additions & 5 deletions src/platform_impl/macos/menu.rs
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
use super::util::IdRef;
use cocoa::appkit::{NSApp, NSApplication, NSEventModifierFlags, NSMenu, NSMenuItem};
use cocoa::base::{nil, selector};
use cocoa::foundation::{NSAutoreleasePool, NSProcessInfo, NSString};
use cocoa::foundation::{NSProcessInfo, NSString};
use objc::{
rc::autoreleasepool,
runtime::{Object, Sel},
Expand All @@ -13,11 +14,11 @@ struct KeyEquivalent<'a> {

pub fn initialize() {
autoreleasepool(|| unsafe {
let menubar = NSMenu::new(nil).autorelease();
let app_menu_item = NSMenuItem::new(nil).autorelease();
menubar.addItem_(app_menu_item);
let menubar = IdRef::new(NSMenu::new(nil));
let app_menu_item = IdRef::new(NSMenuItem::new(nil));
menubar.addItem_(*app_menu_item);
let app = NSApp();
app.setMainMenu_(menubar);
app.setMainMenu_(*menubar);

let app_menu = NSMenu::new(nil);
let process_name = NSProcessInfo::processInfo(nil).processName();
Expand Down
3 changes: 2 additions & 1 deletion src/platform_impl/macos/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,7 @@ pub use self::{
use crate::{
error::OsError as RootOsError, event::DeviceId as RootDeviceId, window::WindowAttributes,
};
use objc::rc::autoreleasepool;

pub(crate) use crate::icon::NoIcon as PlatformIcon;

Expand Down Expand Up @@ -69,7 +70,7 @@ impl Window {
attributes: WindowAttributes,
pl_attribs: PlatformSpecificWindowBuilderAttributes,
) -> Result<Self, RootOsError> {
let (window, _delegate) = UnownedWindow::new(attributes, pl_attribs)?;
let (window, _delegate) = autoreleasepool(|| UnownedWindow::new(attributes, pl_attribs))?;
Ok(Window { window, _delegate })
}
}
Expand Down
4 changes: 1 addition & 3 deletions src/platform_impl/macos/util/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,7 @@ use std::ops::{BitAnd, Deref};
use cocoa::{
appkit::{NSApp, NSWindowStyleMask},
base::{id, nil},
foundation::{NSAutoreleasePool, NSPoint, NSRect, NSString, NSUInteger},
foundation::{NSPoint, NSRect, NSString, NSUInteger},
};
use core_graphics::display::CGDisplay;
use objc::runtime::{Class, Object, Sel, BOOL, YES};
Expand Down Expand Up @@ -61,9 +61,7 @@ impl Drop for IdRef {
fn drop(&mut self) {
if self.0 != nil {
unsafe {
let pool = NSAutoreleasePool::new(nil);
let () = msg_send![self.0, release];
pool.drain();
};
}
}
Expand Down
2 changes: 1 addition & 1 deletion src/platform_impl/macos/view.rs
Original file line number Diff line number Diff line change
Expand Up @@ -289,7 +289,7 @@ extern "C" fn init_with_winit(this: &Object, _sel: Sel, state: *mut c_void) -> i
let notification_center: &Object =
msg_send![class!(NSNotificationCenter), defaultCenter];
let notification_name =
NSString::alloc(nil).init_str("NSViewFrameDidChangeNotification");
IdRef::new(NSString::alloc(nil).init_str("NSViewFrameDidChangeNotification"));
let _: () = msg_send![
notification_center,
addObserver: this
Expand Down
25 changes: 8 additions & 17 deletions src/platform_impl/macos/window.rs
Original file line number Diff line number Diff line change
Expand Up @@ -38,11 +38,12 @@ use cocoa::{
NSRequestUserAttentionType, NSScreen, NSView, NSWindow, NSWindowButton, NSWindowStyleMask,
},
base::{id, nil},
foundation::{NSAutoreleasePool, NSDictionary, NSPoint, NSRect, NSSize},
foundation::{NSDictionary, NSPoint, NSRect, NSSize},
};
use core_graphics::display::{CGDisplay, CGDisplayMode};
use objc::{
declare::ClassDecl,
rc::autoreleasepool,
runtime::{Class, Object, Sel, BOOL, NO, YES},
};

Expand Down Expand Up @@ -118,8 +119,7 @@ fn create_window(
attrs: &WindowAttributes,
pl_attrs: &PlatformSpecificWindowBuilderAttributes,
) -> Option<IdRef> {
unsafe {
let pool = NSAutoreleasePool::new(nil);
autoreleasepool(|| unsafe {
let screen = match attrs.fullscreen {
Some(Fullscreen::Borderless(Some(RootMonitorHandle { inner: ref monitor })))
| Some(Fullscreen::Exclusive(RootVideoMode {
Expand Down Expand Up @@ -241,9 +241,8 @@ fn create_window(
}
ns_window
});
pool.drain();
res
}
})
}

struct WindowClass(*const Class);
Expand Down Expand Up @@ -336,17 +335,11 @@ impl UnownedWindow {
}
trace!("Creating new window");

let pool = unsafe { NSAutoreleasePool::new(nil) };
let ns_window = create_window(&win_attribs, &pl_attribs).ok_or_else(|| {
unsafe { pool.drain() };
os_error!(OsError::CreationError("Couldn't create `NSWindow`"))
})?;
let ns_window = create_window(&win_attribs, &pl_attribs)
.ok_or_else(|| os_error!(OsError::CreationError("Couldn't create `NSWindow`")))?;

let (ns_view, cursor_state) =
unsafe { create_view(*ns_window, &pl_attribs) }.ok_or_else(|| {
unsafe { pool.drain() };
os_error!(OsError::CreationError("Couldn't create `NSView`"))
})?;
let (ns_view, cursor_state) = unsafe { create_view(*ns_window, &pl_attribs) }
.ok_or_else(|| os_error!(OsError::CreationError("Couldn't create `NSView`")))?;

// Configure the new view as the "key view" for the window
unsafe {
Expand Down Expand Up @@ -422,8 +415,6 @@ impl UnownedWindow {
window.set_maximized(maximized);
}

unsafe { pool.drain() };

Ok((window, delegate))
}

Expand Down
13 changes: 7 additions & 6 deletions src/platform_impl/macos/window_delegate.rs
Original file line number Diff line number Diff line change
Expand Up @@ -7,10 +7,11 @@ use std::{
use cocoa::{
appkit::{self, NSApplicationPresentationOptions, NSView, NSWindow},
base::{id, nil},
foundation::{NSAutoreleasePool, NSUInteger},
foundation::NSUInteger,
};
use objc::{
declare::ClassDecl,
rc::autoreleasepool,
runtime::{Class, Object, Sel, BOOL, NO, YES},
};

Expand Down Expand Up @@ -274,11 +275,11 @@ extern "C" fn window_will_close(this: &Object, _: Sel, _: id) {
trace!("Triggered `windowWillClose:`");
with_state(this, |state| unsafe {
// `setDelegate:` retains the previous value and then autoreleases it
let pool = NSAutoreleasePool::new(nil);
// Since El Capitan, we need to be careful that delegate methods can't
// be called after the window closes.
let () = msg_send![*state.ns_window, setDelegate: nil];
pool.drain();
autoreleasepool(|| {
// Since El Capitan, we need to be careful that delegate methods can't
// be called after the window closes.
let () = msg_send![*state.ns_window, setDelegate: nil];
});
state.emit_event(WindowEvent::Destroyed);
});
trace!("Completed `windowWillClose:`");
Expand Down

0 comments on commit 982ad46

Please sign in to comment.