Drop application handler on run loop exit (#4149)

Calling the `Drop` impl of the user's `ApplicationHandler` is important on
iOS and Web, since they don't return from `EventLoop::run_app`.

And now that we reliably call `Drop`, the `ApplicationHandler::exited`
event/callback is unnecessary; using `Drop` composes much better (open files
etc. stored in the app state will be automatically flushed), and prevents
weirdness like attempting to create a new window while exiting.

src/platform_impl/android/mod.rs

@@ -546,8 +546,6 @@ impl EventLoop {
         if self.exiting() {
             self.loop_running = false;
 
-            app.exiting(&self.window_target);
-
             PumpStatus::Exit(0)
         } else {
             PumpStatus::Continue

src/platform_impl/apple/appkit/app_state.rs

@@ -157,6 +157,7 @@ impl AppState {
     pub fn will_terminate(self: &Rc<Self>, _notification: &NSNotification) {
         trace_scope!("NSApplicationWillTerminateNotification");
         // TODO: Notify every window that it will be destroyed, like done in iOS?
+        self.event_handler.terminate();
         self.internal_exit();
     }
 
@@ -164,10 +165,10 @@ impl AppState {
     /// of the given closure.
     pub fn set_event_handler<R>(
         &self,
-        handler: &mut dyn ApplicationHandler,
+        handler: impl ApplicationHandler,
         closure: impl FnOnce() -> R,
     ) -> R {
-        self.event_handler.set(handler, closure)
+        self.event_handler.set(Box::new(handler), closure)
     }
 
     pub fn event_loop_proxy(&self) -> &Arc<EventLoopProxy> {
@@ -202,10 +203,6 @@ impl AppState {
     /// NOTE: that if the `NSApplication` has been launched then that state is preserved,
     /// and we won't need to re-launch the app if subsequent EventLoops are run.
     pub fn internal_exit(self: &Rc<Self>) {
-        self.with_handler(|app, event_loop| {
-            app.exiting(event_loop);
-        });
-
         self.set_is_running(false);
         self.set_stop_on_redraw(false);
         self.set_stop_before_wait(false);

src/platform_impl/apple/appkit/event_loop.rs

@@ -285,10 +285,10 @@ impl EventLoop {
     // redundant wake ups.
     pub fn run_app_on_demand<A: ApplicationHandler>(
         &mut self,
-        mut app: A,
+        app: A,
     ) -> Result<(), EventLoopError> {
         self.app_state.clear_exit();
-        self.app_state.set_event_handler(&mut app, || {
+        self.app_state.set_event_handler(app, || {
             autoreleasepool(|_| {
                 // clear / normalize pump_events state
                 self.app_state.set_wait_timeout(None);
@@ -324,9 +324,9 @@ impl EventLoop {
     pub fn pump_app_events<A: ApplicationHandler>(
         &mut self,
         timeout: Option<Duration>,
-        mut app: A,
+        app: A,
     ) -> PumpStatus {
-        self.app_state.set_event_handler(&mut app, || {
+        self.app_state.set_event_handler(app, || {
             autoreleasepool(|_| {
                 // As a special case, if the application hasn't been launched yet then we at least
                 // run the loop until it has fully launched.

src/platform_impl/apple/event_handler.rs

@@ -8,10 +8,10 @@ use crate::application::ApplicationHandler;
 #[derive(Default)]
 pub(crate) struct EventHandler {
     /// This can be in the following states:
-    /// - Not registered by the event loop (None).
+    /// - Not registered by the event loop, or terminated (None).
     /// - Present (Some(handler)).
     /// - Currently executing the handler / in use (RefCell borrowed).
-    inner: RefCell<Option<&'static mut dyn ApplicationHandler>>,
+    inner: RefCell<Option<Box<dyn ApplicationHandler + 'static>>>,
 }
 
 impl fmt::Debug for EventHandler {
@@ -37,7 +37,7 @@ impl EventHandler {
     /// from within the closure.
     pub(crate) fn set<'handler, R>(
         &self,
-        app: &'handler mut dyn ApplicationHandler,
+        app: Box<dyn ApplicationHandler + 'handler>,
         closure: impl FnOnce() -> R,
     ) -> R {
         // SAFETY: We extend the lifetime of the handler here so that we can
@@ -48,8 +48,8 @@ impl EventHandler {
         // extended beyond `'handler`.
         let handler = unsafe {
             mem::transmute::<
-                &'handler mut dyn ApplicationHandler,
-                &'static mut dyn ApplicationHandler,
+                Box<dyn ApplicationHandler + 'handler>,
+                Box<dyn ApplicationHandler + 'static>,
             >(app)
         };
 
@@ -71,10 +71,13 @@ impl EventHandler {
             fn drop(&mut self) {
                 match self.0.inner.try_borrow_mut().as_deref_mut() {
                     Ok(data @ Some(_)) => {
-                        *data = None;
+                        let handler = data.take();
+                        // Explicitly `Drop` the application handler.
+                        drop(handler);
                     },
                     Ok(None) => {
-                        tracing::error!("tried to clear handler, but no handler was set");
+                        // Allowed, happens if the handler was cleared manually
+                        // elsewhere (such as in `applicationWillTerminate:`).
                     },
                     Err(_) => {
                         // Note: This is not expected to ever happen, this
@@ -110,16 +113,16 @@ impl EventHandler {
         matches!(self.inner.try_borrow().as_deref(), Ok(Some(_)))
     }
 
-    pub(crate) fn handle(&self, callback: impl FnOnce(&mut dyn ApplicationHandler)) {
+    pub(crate) fn handle(&self, callback: impl FnOnce(&mut (dyn ApplicationHandler + '_))) {
         match self.inner.try_borrow_mut().as_deref_mut() {
-            Ok(Some(user_app)) => {
+            Ok(Some(ref mut user_app)) => {
                 // It is important that we keep the reference borrowed here,
                 // so that `in_use` can properly detect that the handler is
                 // still in use.
                 //
                 // If the handler unwinds, the `RefMut` will ensure that the
                 // handler is no longer borrowed.
-                callback(*user_app);
+                callback(&mut **user_app);
             },
             Ok(None) => {
                 // `NSApplication`, our app state and this handler are all
@@ -133,4 +136,21 @@ impl EventHandler {
             },
         }
     }
+
+    pub(crate) fn terminate(&self) {
+        match self.inner.try_borrow_mut().as_deref_mut() {
+            Ok(data @ Some(_)) => {
+                let handler = data.take();
+                // Explicitly `Drop` the application handler.
+                drop(handler);
+            },
+            Ok(None) => {
+                // When terminating, we expect the application handler to still be registered.
+                tracing::error!("tried to clear handler, but no handler was set");
+            },
+            Err(_) => {
+                panic!("tried to clear handler while an event is currently being handled");
+            },
+        }
+    }
 }

src/platform_impl/apple/uikit/app_state.rs

@@ -305,8 +305,8 @@ pub(crate) fn queue_gl_or_metal_redraw(mtm: MainThreadMarker, window: Retained<W
     }
 }
 
-pub(crate) fn launch(mtm: MainThreadMarker, app: &mut dyn ApplicationHandler, run: impl FnOnce()) {
-    get_handler(mtm).set(app, run)
+pub(crate) fn launch(mtm: MainThreadMarker, app: impl ApplicationHandler, run: impl FnOnce()) {
+    get_handler(mtm).set(Box::new(app), run)
 }
 
 pub fn did_finish_launching(mtm: MainThreadMarker) {
@@ -495,13 +495,11 @@ pub(crate) fn terminated(application: &UIApplication) {
 
     let mut this = AppState::get_mut(mtm);
     this.terminated_transition();
-    drop(this);
-
-    get_handler(mtm).handle(|app| app.exiting(&ActiveEventLoop { mtm }));
-
-    let this = AppState::get_mut(mtm);
     // Prevent EventLoopProxy from firing again.
     this.event_loop_proxy.invalidate();
+    drop(this);
+
+    get_handler(mtm).terminate();
 }
 
 fn handle_wrapped_event(mtm: MainThreadMarker, event: EventWrapper) {

src/platform_impl/apple/uikit/event_loop.rs

@@ -243,7 +243,7 @@ impl EventLoop {
         })
     }
 
-    pub fn run_app<A: ApplicationHandler>(self, mut app: A) -> ! {
+    pub fn run_app<A: ApplicationHandler>(self, app: A) -> ! {
         let application: Option<Retained<UIApplication>> =
             unsafe { msg_send![UIApplication::class(), sharedApplication] };
         assert!(
@@ -259,7 +259,7 @@ impl EventLoop {
             fn _NSGetArgv() -> *mut *mut *mut c_char;
         }
 
-        app_state::launch(self.mtm, &mut app, || unsafe {
+        app_state::launch(self.mtm, app, || unsafe {
             UIApplicationMain(
                 *_NSGetArgc(),
                 NonNull::new(*_NSGetArgv()).unwrap(),

src/platform_impl/linux/wayland/event_loop/mod.rs

@@ -203,11 +203,10 @@ impl EventLoop {
         if !self.exiting() {
             self.poll_events_with_timeout(timeout, &mut app);
         }
+
         if let Some(code) = self.exit_code() {
             self.loop_running = false;
 
-            app.exiting(&self.active_event_loop);
-
             PumpStatus::Exit(code)
         } else {
             PumpStatus::Continue

src/platform_impl/linux/x11/mod.rs

@@ -498,8 +498,6 @@ impl EventLoop {
         if let Some(code) = self.exit_code() {
             self.loop_running = false;
 
-            app.exiting(self.window_target());
-
             PumpStatus::Exit(code)
         } else {
             PumpStatus::Continue

src/platform_impl/orbital/event_loop.rs

@@ -650,8 +650,6 @@ impl EventLoop {
             }
         }
 
-        app.exiting(&self.window_target);
-
         Ok(())
     }
 

src/platform_impl/web/event_loop/runner.rs

@@ -158,7 +158,6 @@ impl Runner {
             Event::Resumed => self.app.resumed(&self.event_loop),
             Event::CreateSurfaces => self.app.can_create_surfaces(&self.event_loop),
             Event::AboutToWait => self.app.about_to_wait(&self.event_loop),
-            Event::LoopExiting => self.app.exiting(&self.event_loop),
         }
     }
 }
@@ -639,7 +638,9 @@ impl Shared {
         self.apply_control_flow();
         // We don't call `handle_loop_destroyed` here because we don't need to
         // perform cleanup when the Web browser is going to destroy the page.
-        self.handle_event(Event::LoopExiting);
+        //
+        // We do want to run the application handler's `Drop` impl.
+        *self.0.runner.borrow_mut() = RunnerEnum::Destroyed;
     }
 
     // handle_event takes in events and either queues them or applies a callback
@@ -737,7 +738,6 @@ impl Shared {
     }
 
     fn handle_loop_destroyed(&self) {
-        self.handle_event(Event::LoopExiting);
         let all_canvases = std::mem::take(&mut *self.0.all_canvases.borrow_mut());
         *self.0.page_transition_event_handle.borrow_mut() = None;
         *self.0.on_mouse_move.borrow_mut() = None;
@@ -879,6 +879,5 @@ pub(crate) enum Event {
     CreateSurfaces,
     Resumed,
     AboutToWait,
-    LoopExiting,
     UserWakeUp,
 }

src/platform_impl/windows/event_loop/runner.rs

@@ -333,7 +333,6 @@ impl EventLoopRunner {
                 self.call_new_events(true);
                 self.call_event_handler(|app, event_loop| app.about_to_wait(event_loop));
                 self.last_events_cleared.set(Instant::now());
-                self.call_event_handler(|app, event_loop| app.exiting(event_loop));
             },
             (_, Uninitialized) => panic!("cannot move state to Uninitialized"),
 
@@ -341,9 +340,7 @@ impl EventLoopRunner {
             (Idle, HandlingMainEvents) => {
                 self.call_new_events(false);
             },
-            (Idle, Destroyed) => {
-                self.call_event_handler(|app, event_loop| app.exiting(event_loop));
-            },
+            (Idle, Destroyed) => {},
 
             (HandlingMainEvents, Idle) => {
                 // This is always the last event we dispatch before waiting for new events
@@ -353,7 +350,6 @@ impl EventLoopRunner {
             (HandlingMainEvents, Destroyed) => {
                 self.call_event_handler(|app, event_loop| app.about_to_wait(event_loop));
                 self.last_events_cleared.set(Instant::now());
-                self.call_event_handler(|app, event_loop| app.exiting(event_loop));
             },
 
             (Destroyed, _) => panic!("cannot move state from Destroyed"),