Terminate app run loop on Windows when all windows have closed.

Author: xStrom

druid-shell/src/application.rs

@@ -24,13 +24,16 @@ use crate::platform::application as platform;
 ///
 /// # Note
 ///
-/// This is currently very limited in its functionality, and is currently
-/// designed to address a single case, which is handling menu commands when
-/// no window is open.
+/// This is currently limited in its functionality,
+/// having the ability to close all windows
+/// and handling menu commands when no window is open.
 ///
 /// It is possible that this will expand to cover additional functionality
 /// in the future.
 pub trait AppHandler {
+    /// Closes all the windows.
+    fn close_all_windows(&mut self) {}
+
     /// Called when a menu item is selected.
     #[allow(unused_variables)]
     fn command(&mut self, id: u32) {}

druid-shell/src/platform/windows/application.rs

@@ -17,52 +17,86 @@
 use std::mem;
 use std::ptr;
 
+use winapi::shared::minwindef::FALSE;
 use winapi::shared::minwindef::HINSTANCE;
 use winapi::shared::ntdef::LPCWSTR;
 use winapi::shared::windef::HCURSOR;
+use winapi::shared::winerror::HRESULT_FROM_WIN32;
+use winapi::um::errhandlingapi::GetLastError;
 use winapi::um::shellscalingapi::PROCESS_SYSTEM_DPI_AWARE;
 use winapi::um::wingdi::CreateSolidBrush;
 use winapi::um::winuser::{
-    DispatchMessageW, GetAncestor, GetMessageW, LoadIconW, PostQuitMessage, RegisterClassW,
-    TranslateAcceleratorW, TranslateMessage, GA_ROOT, IDI_APPLICATION, MSG, WNDCLASSW,
+    DispatchMessageW, GetAncestor, GetMessageW, LoadIconW, PostQuitMessage, PostThreadMessageW,
+    RegisterClassW, TranslateAcceleratorW, TranslateMessage, GA_ROOT, IDI_APPLICATION, MSG,
+    WNDCLASSW,
 };
 
+use log::{error, warn};
+
 use crate::application::AppHandler;
 
 use super::accels;
 use super::clipboard::Clipboard;
-use super::util::{self, ToWide, CLASS_NAME, OPTIONAL_FUNCTIONS};
-use super::window::win_proc_dispatch;
+use super::error::Error;
+use super::util::{
+    self, claim_main_thread, main_thread_id, release_main_thread, ToWide, CLASS_NAME,
+    OPTIONAL_FUNCTIONS,
+};
+use super::window::{win_proc_dispatch, DS_REQUEST_QUIT};
 
-pub struct Application;
+pub struct Application {
+    handler: Option<Box<dyn AppHandler>>,
+}
 
 impl Application {
-    pub fn new(_handler: Option<Box<dyn AppHandler>>) -> Application {
+    pub fn new(handler: Option<Box<dyn AppHandler>>) -> Application {
         Application::init();
-        Application
+        Application { handler }
     }
 
     pub fn run(&mut self) {
+        claim_main_thread();
         unsafe {
             // Handle windows messages
             loop {
                 let mut msg = mem::MaybeUninit::uninit();
                 let res = GetMessageW(msg.as_mut_ptr(), ptr::null_mut(), 0, 0);
                 if res <= 0 {
-                    return;
+                    if res == -1 {
+                        error!(
+                            "GetMessageW failed: {}",
+                            Error::Hr(HRESULT_FROM_WIN32(GetLastError()))
+                        );
+                    }
+                    break;
                 }
                 let mut msg: MSG = msg.assume_init();
                 let accels = accels::find_accels(GetAncestor(msg.hwnd, GA_ROOT));
                 let translated = accels.map_or(false, |it| {
                     TranslateAcceleratorW(msg.hwnd, it.handle(), &mut msg) != 0
                 });
-
                 if !translated {
-                    TranslateMessage(&msg);
-                    DispatchMessageW(&msg);
+                    // We check for DS_REQUEST_QUIT here because thread messages
+                    // will not be forwarded to any window procedures by DispatchMessageW.
+                    if msg.message == DS_REQUEST_QUIT {
+                        if let Some(handler) = &mut self.handler {
+                            // We want to queue up the destruction of all open windows.
+                            // Failure to do so will lead to resource leaks
+                            // and an eventual error code exit for the process.
+                            handler.close_all_windows();
+                        }
+                        // PostQuitMessage sets a quit request flag in the OS.
+                        // The actual WM_QUIT message is queued but won't be sent
+                        // until all other important events have been handled.
+                        PostQuitMessage(0);
+                    } else {
+                        TranslateMessage(&msg);
+                        DispatchMessageW(&msg);
+                    }
                 }
             }
         }
+        release_main_thread();
     }
 
     /// Initialize the app. At the moment, this is mostly needed for hi-dpi.
@@ -99,8 +133,15 @@ impl Application {
     }
 
     pub fn quit() {
-        unsafe {
-            PostQuitMessage(0);
+        if let Some(thread_id) = main_thread_id() {
+            unsafe {
+                if PostThreadMessageW(thread_id, DS_REQUEST_QUIT, 0, 0) == FALSE {
+                    warn!(
+                        "PostThreadMessageW failed: {}",
+                        Error::Hr(HRESULT_FROM_WIN32(GetLastError()))
+                    );
+                }
+            }
         }
     }
 

druid-shell/src/platform/windows/util.rs

@@ -22,27 +22,78 @@ use std::mem;
 use std::os::windows::ffi::{OsStrExt, OsStringExt};
 use std::ptr;
 use std::slice;
+use std::sync::atomic::{AtomicU32, Ordering};
 
 use winapi::ctypes::c_void;
 use winapi::shared::guiddef::REFIID;
-use winapi::shared::minwindef::{HMODULE, UINT};
+use winapi::shared::minwindef::{DWORD, HMODULE, UINT};
 use winapi::shared::ntdef::{HRESULT, LPWSTR};
 use winapi::shared::windef::HMONITOR;
 use winapi::shared::winerror::SUCCEEDED;
 use winapi::um::fileapi::{CreateFileA, GetFileType, OPEN_EXISTING};
 use winapi::um::handleapi::INVALID_HANDLE_VALUE;
 use winapi::um::libloaderapi::{GetModuleHandleW, GetProcAddress, LoadLibraryW};
 use winapi::um::processenv::{GetStdHandle, SetStdHandle};
+use winapi::um::processthreadsapi::GetCurrentThreadId;
 use winapi::um::shellscalingapi::{MONITOR_DPI_TYPE, PROCESS_DPI_AWARENESS};
 use winapi::um::unknwnbase::IUnknown;
 use winapi::um::winbase::{FILE_TYPE_UNKNOWN, STD_ERROR_HANDLE, STD_OUTPUT_HANDLE};
 use winapi::um::wincon::{AttachConsole, ATTACH_PARENT_PROCESS};
 use winapi::um::winnt::{FILE_SHARE_WRITE, GENERIC_READ, GENERIC_WRITE};
 
-use log::error;
+use log::{error, warn};
 
 use super::error::Error;
 
+static MAIN_THREAD_ID: AtomicU32 = AtomicU32::new(0);
+
+#[inline]
+fn current_thread_id() -> DWORD {
+    unsafe { GetCurrentThreadId() }
+}
+
+#[allow(dead_code)]
+pub fn assert_main_thread() {
+    let thread_id = current_thread_id();
+    let main_thread_id = MAIN_THREAD_ID.load(Ordering::Acquire);
+    assert_eq!(thread_id, main_thread_id);
+}
+
+pub fn claim_main_thread() {
+    let thread_id = current_thread_id();
+    let old_thread_id = MAIN_THREAD_ID.compare_and_swap(0, thread_id, Ordering::AcqRel);
+    if old_thread_id != 0 {
+        panic!(
+            "The main thread status has already been claimed by thread {}",
+            thread_id
+        );
+    }
+}
+
+pub fn release_main_thread() {
+    let thread_id = current_thread_id();
+    let old_thread_id = MAIN_THREAD_ID.compare_and_swap(thread_id, 0, Ordering::AcqRel);
+    if old_thread_id == 0 {
+        warn!("The main thread status was already vacant.");
+    } else if old_thread_id != thread_id {
+        panic!(
+            "The main thread status is owned by another thread {}",
+            thread_id
+        );
+    }
+}
+
+pub fn main_thread_id() -> Option<DWORD> {
+    let thread_id = MAIN_THREAD_ID.load(Ordering::Acquire);
+    // Although not explicitly documented, zero is an invalid thread id.
+    // It can be deducted from the behavior of GetThreadId / SetWindowsHookExA.
+    // https://devblogs.microsoft.com/oldnewthing/20040223-00/?p=40503
+    match thread_id {
+        0 => None,
+        id => Some(id),
+    }
+}
+
 pub fn as_result(hr: HRESULT) -> Result<(), Error> {
     if SUCCEEDED(hr) {
         Ok(())

druid-shell/src/platform/windows/window.rs

@@ -113,7 +113,7 @@ pub struct WindowHandle {
 
 /// A handle that can get used to schedule an idle handler. Note that
 /// this handle is thread safe. If the handle is used after the hwnd
-/// has been destroyed, probably not much will go wrong (the XI_RUN_IDLE
+/// has been destroyed, probably not much will go wrong (the DS_RUN_IDLE
 /// message may be sent to a stray window).
 #[derive(Clone)]
 pub struct IdleHandle {
@@ -187,9 +187,9 @@ struct DCompState {
 }
 
 /// Message indicating there are idle tasks to run.
-const XI_RUN_IDLE: UINT = WM_USER;
+const DS_RUN_IDLE: UINT = WM_USER;
 
-/// Message relaying a request to destroy the window
+/// Message relaying a request to destroy the window.
 ///
 /// Calling `DestroyWindow` from inside the handler is problematic
 /// because it will recursively cause a `WM_DESTROY` message to be
@@ -199,7 +199,12 @@ const XI_RUN_IDLE: UINT = WM_USER;
 /// As a solution, instead of immediately calling `DestroyWindow`, we
 /// send this message to request destroying the window, so that at the
 /// time it is handled, we can successfully borrow the handler.
-const XI_REQUEST_DESTROY: UINT = WM_USER + 1;
+const DS_REQUEST_DESTROY: UINT = WM_USER + 1;
+
+/// Message relaying a request to quit the app run loop.
+///
+/// Directly calling `PostQuitMessage` won't do proper cleanup.
+pub const DS_REQUEST_QUIT: UINT = WM_USER + 2;
 
 impl Default for PresentStrategy {
     fn default() -> PresentStrategy {
@@ -669,7 +674,7 @@ impl WndProc for MyWndProc {
                 }
                 Some(0)
             }
-            XI_REQUEST_DESTROY => {
+            DS_REQUEST_DESTROY => {
                 unsafe {
                     DestroyWindow(hwnd);
                 }
@@ -682,7 +687,7 @@ impl WndProc for MyWndProc {
                 } else {
                     self.log_dropped_msg(hwnd, msg, wparam, lparam);
                 }
-                None
+                Some(0)
             }
             WM_TIMER => {
                 let id = wparam;
@@ -719,7 +724,7 @@ impl WndProc for MyWndProc {
                 }
                 Some(0)
             }
-            XI_RUN_IDLE => {
+            DS_RUN_IDLE => {
                 if let Ok(mut s) = self.state.try_borrow_mut() {
                     let s = s.as_mut().unwrap();
                     let queue = self.handle.borrow().take_idle_queue();
@@ -1104,7 +1109,7 @@ impl WindowHandle {
         if let Some(w) = self.state.upgrade() {
             let hwnd = w.hwnd.get();
             unsafe {
-                PostMessageW(hwnd, XI_REQUEST_DESTROY, 0, 0);
+                PostMessageW(hwnd, DS_REQUEST_DESTROY, 0, 0);
             }
         }
     }
@@ -1344,7 +1349,7 @@ impl IdleHandle {
         let mut queue = self.queue.lock().unwrap();
         if queue.is_empty() {
             unsafe {
-                PostMessageW(self.hwnd, XI_RUN_IDLE, 0, 0);
+                PostMessageW(self.hwnd, DS_RUN_IDLE, 0, 0);
             }
         }
         queue.push(IdleKind::Callback(Box::new(callback)));
@@ -1354,7 +1359,7 @@ impl IdleHandle {
         let mut queue = self.queue.lock().unwrap();
         if queue.is_empty() {
             unsafe {
-                PostMessageW(self.hwnd, XI_RUN_IDLE, 0, 0);
+                PostMessageW(self.hwnd, DS_RUN_IDLE, 0, 0);
             }
         }
         queue.push(IdleKind::Token(token));

druid/src/win_handler.rs

@@ -118,6 +118,10 @@ impl<T> Windows<T> {
     fn get_mut(&mut self, id: WindowId) -> Option<&mut Window<T>> {
         self.windows.get_mut(&id)
     }
+
+    fn count(&self) -> usize {
+        self.windows.len() + self.pending.len()
+    }
 }
 
 impl<T> AppHandler<T> {
@@ -220,7 +224,13 @@ impl<T: Data> Inner<T> {
             if self.windows.windows.is_empty() {
                 // on mac we need to keep the menu around
                 self.root_menu = win.menu.take();
-                //FIXME: on windows we need to shutdown the app here?
+
+                #[cfg(target_os = "windows")]
+                {
+                    if self.windows.count() == 0 {
+                        Application::quit();
+                    }
+                }
             }
         }
 
@@ -262,6 +272,13 @@ impl<T: Data> Inner<T> {
         }
     }
 
+    /// Requests the platform to close all windows.
+    fn request_close_all_windows(&mut self) {
+        for win in self.windows.iter_mut() {
+            win.handle.close();
+        }
+    }
+
     fn show_window(&mut self, id: WindowId) {
         if let Some(win) = self.windows.get_mut(id) {
             win.handle.bring_to_front_and_focus();
@@ -495,7 +512,7 @@ impl<T: Data> AppState<T> {
     fn handle_cmd(&mut self, target: Target, cmd: Command) {
         use Target as T;
         match (target, &cmd.selector) {
-            // these are handled the same no matter where they  come from
+            // these are handled the same no matter where they come from
             (_, &sys_cmd::QUIT_APP) => self.quit(),
             (_, &sys_cmd::HIDE_APPLICATION) => self.hide_app(),
             (_, &sys_cmd::HIDE_OTHERS) => self.hide_others(),
@@ -505,7 +522,7 @@ impl<T: Data> AppState<T> {
                 }
             }
             // these should come from a window
-            // FIXME: we need to be  able to open a file without a window handle
+            // FIXME: we need to be able to open a file without a window handle
             (T::Window(id), &sys_cmd::SHOW_OPEN_PANEL) => self.show_open_panel(cmd, id),
             (T::Window(id), &sys_cmd::SHOW_SAVE_PANEL) => self.show_save_panel(cmd, id),
             (T::Window(id), &sys_cmd::CLOSE_WINDOW) => self.request_close_window(cmd, id),
@@ -567,6 +584,10 @@ impl<T: Data> AppState<T> {
         self.inner.borrow_mut().request_close_window(*id);
     }
 
+    fn request_close_all_windows(&mut self) {
+        self.inner.borrow_mut().request_close_all_windows();
+    }
+
     fn show_window(&mut self, cmd: Command) {
         let id: WindowId = *cmd
             .get_object()
@@ -595,6 +616,10 @@ impl<T: Data> AppState<T> {
 }
 
 impl<T: Data> crate::shell::AppHandler for AppHandler<T> {
+    fn close_all_windows(&mut self) {
+        self.app_state.request_close_all_windows();
+    }
+
     fn command(&mut self, id: u32) {
         self.app_state.handle_system_cmd(id, None)
     }