From 982ad46c83ea7c4dfcc5dc164d026f6ece3611a9 Mon Sep 17 00:00:00 2001 From: Mads Marquart Date: Thu, 27 May 2021 17:38:41 +0200 Subject: [PATCH] Use objc's autoreleasepool instead of manually with NSAutoreleasePool (#1936) Ensures the pools are released even if we panic --- Cargo.toml | 1 - src/platform_impl/macos/app_state.rs | 29 +++++++++++----------- src/platform_impl/macos/event_loop.rs | 17 ++++++------- src/platform_impl/macos/menu.rs | 11 ++++---- src/platform_impl/macos/mod.rs | 3 ++- src/platform_impl/macos/util/mod.rs | 4 +-- src/platform_impl/macos/view.rs | 2 +- src/platform_impl/macos/window.rs | 25 ++++++------------- src/platform_impl/macos/window_delegate.rs | 13 +++++----- 9 files changed, 47 insertions(+), 58 deletions(-) diff --git a/Cargo.toml b/Cargo.toml index ce4a2579..88ac1cc6 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -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" diff --git a/src/platform_impl/macos/app_state.rs b/src/platform_impl/macos/app_state.rs index 35c6d584..5f148333 100644 --- a/src/platform_impl/macos/app_state.rs +++ b/src/platform_impl/macos/app_state.rs @@ -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, @@ -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]; diff --git a/src/platform_impl/macos/event_loop.rs b/src/platform_impl/macos/event_loop.rs index 2142c285..45a32306 100644 --- a/src/platform_impl/macos/event_loop.rs +++ b/src/platform_impl/macos/event_loop.rs @@ -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, @@ -115,9 +114,9 @@ impl EventLoop { 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 = Default::default(); @@ -162,9 +161,7 @@ impl EventLoop { self._callback = Some(Rc::clone(&callback)); - unsafe { - let pool = NSAutoreleasePool::new(nil); - defer!(pool.drain()); + autoreleasepool(|| unsafe { let app = NSApp(); assert_ne!(app, nil); @@ -180,7 +177,7 @@ impl EventLoop { resume_unwind(panic); } AppState::exit(); - } + }); } pub fn create_proxy(&self) -> Proxy { diff --git a/src/platform_impl/macos/menu.rs b/src/platform_impl/macos/menu.rs index 1a8cd2ee..089874ef 100644 --- a/src/platform_impl/macos/menu.rs +++ b/src/platform_impl/macos/menu.rs @@ -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}, @@ -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(); diff --git a/src/platform_impl/macos/mod.rs b/src/platform_impl/macos/mod.rs index 5254a993..a7c29027 100644 --- a/src/platform_impl/macos/mod.rs +++ b/src/platform_impl/macos/mod.rs @@ -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; @@ -69,7 +70,7 @@ impl Window { attributes: WindowAttributes, pl_attribs: PlatformSpecificWindowBuilderAttributes, ) -> Result { - let (window, _delegate) = UnownedWindow::new(attributes, pl_attribs)?; + let (window, _delegate) = autoreleasepool(|| UnownedWindow::new(attributes, pl_attribs))?; Ok(Window { window, _delegate }) } } diff --git a/src/platform_impl/macos/util/mod.rs b/src/platform_impl/macos/util/mod.rs index 2ebfe56c..93931d2c 100644 --- a/src/platform_impl/macos/util/mod.rs +++ b/src/platform_impl/macos/util/mod.rs @@ -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}; @@ -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(); }; } } diff --git a/src/platform_impl/macos/view.rs b/src/platform_impl/macos/view.rs index 5da5b536..360f289a 100644 --- a/src/platform_impl/macos/view.rs +++ b/src/platform_impl/macos/view.rs @@ -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 diff --git a/src/platform_impl/macos/window.rs b/src/platform_impl/macos/window.rs index 51a37a33..68ef526d 100644 --- a/src/platform_impl/macos/window.rs +++ b/src/platform_impl/macos/window.rs @@ -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}, }; @@ -118,8 +119,7 @@ fn create_window( attrs: &WindowAttributes, pl_attrs: &PlatformSpecificWindowBuilderAttributes, ) -> Option { - 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 { @@ -241,9 +241,8 @@ fn create_window( } ns_window }); - pool.drain(); res - } + }) } struct WindowClass(*const Class); @@ -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 { @@ -422,8 +415,6 @@ impl UnownedWindow { window.set_maximized(maximized); } - unsafe { pool.drain() }; - Ok((window, delegate)) } diff --git a/src/platform_impl/macos/window_delegate.rs b/src/platform_impl/macos/window_delegate.rs index c226a66d..d77c8957 100644 --- a/src/platform_impl/macos/window_delegate.rs +++ b/src/platform_impl/macos/window_delegate.rs @@ -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}, }; @@ -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:`");