Replace dtor with HashSet-based approach

Author: estroBiologist

Cargo.toml

@@ -30,7 +30,6 @@ nix = "0.22.0"
 [target.'cfg(target_os="windows")'.dependencies]
 winapi = { version = "0.3.8", features = ["libloaderapi", "winuser", "windef", "minwindef", "guiddef", "combaseapi", "wingdi", "errhandlingapi", "ole2", "oleidl", "shellapi", "winerror"] }
 uuid = { version = "0.8", features = ["v4"], optional = true }
-dtor = "=0.0.6"
 
 [target.'cfg(target_os="macos")'.dependencies]
 cocoa = "0.24.0"

src/win/hook.rs

@@ -1,74 +1,99 @@
 use std::{
+    collections::HashSet,
     ffi::c_int,
     ptr,
-    sync::{Mutex, Once},
+    sync::{LazyLock, Mutex, RwLock},
 };
 
-use dtor::dtor;
 use winapi::{
     shared::{
         minwindef::{LPARAM, WPARAM},
-        windef::{HHOOK, POINT},
+        windef::{HHOOK, HWND, POINT},
     },
     um::{
         libloaderapi::GetModuleHandleA,
         processthreadsapi::GetCurrentThreadId,
         winuser::{
-            CallNextHookEx, GetClassNameA, GetWindowLongPtrW, SetWindowsHookExA,
-            UnhookWindowsHookEx, GWLP_USERDATA, HC_ACTION, MSG, PM_REMOVE, WH_GETMESSAGE, WM_CHAR,
-            WM_KEYDOWN, WM_KEYUP, WM_SYSCHAR, WM_SYSKEYDOWN, WM_SYSKEYUP, WM_USER,
+            CallNextHookEx, SetWindowsHookExA, UnhookWindowsHookEx, HC_ACTION, MSG, PM_REMOVE,
+            WH_GETMESSAGE, WM_CHAR, WM_KEYDOWN, WM_KEYUP, WM_SYSCHAR, WM_SYSKEYDOWN, WM_SYSKEYUP,
+            WM_USER,
         },
     },
 };
 
-use crate::win::{wnd_proc, WindowState};
+use crate::win::wnd_proc;
 
-static HOOK: Mutex<WinKeyboardHook> = Mutex::new(WinKeyboardHook::new());
-static ONCE: Once = Once::new();
+static HOOK: Mutex<Option<KeyboardHook>> = Mutex::new(None);
+
+// track all windows opened by this instance of baseview
+// we use an RwLock here since the vast majority of uses (event interceptions)
+// will only need to read from the HashSet
+static OPEN_WINDOWS: LazyLock<RwLock<HashSet<HWNDWrapper>>> = LazyLock::new(|| RwLock::default());
+
+pub(crate) struct KeyboardHookHandle(HWNDWrapper);
+
+struct KeyboardHook(HHOOK);
+
+#[derive(Hash, PartialEq, Eq, Clone, Copy)]
+struct HWNDWrapper(HWND);
+
+// SAFETY: it's a pointer behind a mutex. we'll live
+unsafe impl Send for KeyboardHook {}
+unsafe impl Sync for KeyboardHook {}
+
+// SAFETY: ditto
+unsafe impl Send for HWNDWrapper {}
+unsafe impl Sync for HWNDWrapper {}
+
+impl Drop for KeyboardHookHandle {
+    fn drop(&mut self) {
+        deinit_keyboard_hook(self.0);
+    }
+}
 
 // initialize keyboard hook
 // some DAWs (particularly Ableton) intercept incoming keyboard messages,
 // but we're naughty so we intercept them right back
-//
-// this is invoked by Window::open() since Rust doesn't have runtime static ctors
-pub(crate) fn init_keyboard_hook() {
-    ONCE.call_once(|| {
-        HOOK.lock().unwrap().hook = unsafe {
+pub(crate) fn init_keyboard_hook(hwnd: HWND) -> KeyboardHookHandle {
+    // register hwnd to global window set
+    OPEN_WINDOWS.write().unwrap().insert(HWNDWrapper(hwnd));
+
+    let hook = &mut *HOOK.lock().unwrap();
+
+    if hook.is_some() {
+        // keyboard hook already exists, just return handle
+        KeyboardHookHandle(HWNDWrapper(hwnd))
+    } else {
+        // keyboard hook doesn't exist (no windows open before this), create it
+        let new_hook = KeyboardHook(unsafe {
             SetWindowsHookExA(
                 WH_GETMESSAGE,
                 Some(keyboard_hook_callback),
                 GetModuleHandleA(ptr::null()),
                 GetCurrentThreadId(),
             )
-        };
-    });
-}
+        });
 
-#[dtor]
-fn deinit_keyboard_hook() {
-    let hook = HOOK.lock().unwrap();
+        *hook = Some(new_hook);
 
-    if !hook.hook.is_null() {
-        unsafe {
-            UnhookWindowsHookEx(hook.hook);
-        }
+        KeyboardHookHandle(HWNDWrapper(hwnd))
     }
 }
 
-struct WinKeyboardHook {
-    hook: HHOOK,
-}
+fn deinit_keyboard_hook(hwnd: HWNDWrapper) {
+    let windows = &mut *OPEN_WINDOWS.write().unwrap();
+
+    windows.remove(&hwnd);
 
-impl WinKeyboardHook {
-    const fn new() -> Self {
-        Self { hook: ptr::null_mut() }
+    if windows.is_empty() {
+        if let Ok(Some(hook)) = HOOK.lock().as_deref() {
+            unsafe {
+                UnhookWindowsHookEx(hook.0);
+            }
+        }
     }
 }
 
-// SAFETY: it's a pointer behind a mutex. we'll live
-unsafe impl Send for WinKeyboardHook {}
-unsafe impl Sync for WinKeyboardHook {}
-
 unsafe extern "system" fn keyboard_hook_callback(
     n_code: c_int, wparam: WPARAM, lparam: LPARAM,
 ) -> isize {
@@ -91,7 +116,7 @@ unsafe extern "system" fn keyboard_hook_callback(
 }
 
 // check if `msg` is a keyboard message addressed
-// to a baseview window, and intercept it if so
+// to a window in OPEN_WINDOWS, and intercept it if so
 unsafe fn offer_message_to_baseview(msg: *mut MSG) -> bool {
     let msg = &*msg;
 
@@ -102,21 +127,11 @@ unsafe fn offer_message_to_baseview(msg: *mut MSG) -> bool {
         _ => return false,
     }
 
-    // check if this is a baseview window (gross)
-    let mut classname = [0u8; 9];
+    // check if this is one of our windows. if so, intercept it
+    if OPEN_WINDOWS.read().unwrap().contains(&HWNDWrapper(msg.hwnd)) {
+        let _ = wnd_proc(msg.hwnd, msg.message, msg.wParam, msg.lParam);
 
-    // SAFETY: It's Probably ASCII Lmao
-    if GetClassNameA(msg.hwnd, &mut classname as *mut u8 as *mut i8, 9) != 0 {
-        if &classname[0..8] == "Baseview".as_bytes() {
-            let _ = wnd_proc(
-                msg.hwnd,
-                msg.message,
-                msg.wParam,
-                msg.lParam,
-            );
-
-            return true;
-        }
+        return true;
     }
 
     false

src/win/window.rs

@@ -33,7 +33,7 @@ use raw_window_handle::{
 
 const BV_WINDOW_MUST_CLOSE: UINT = WM_USER + 1;
 
-use crate::win::hook;
+use crate::win::hook::{self, KeyboardHookHandle};
 use crate::{
     Event, MouseButton, MouseCursor, MouseEvent, PhyPoint, PhySize, ScrollDelta, Size, WindowEvent,
     WindowHandler, WindowInfo, WindowOpenOptions, WindowScalePolicy,
@@ -508,6 +508,11 @@ pub(super) struct WindowState {
     scale_policy: WindowScalePolicy,
     dw_style: u32,
 
+    // handle to the win32 keyboard hook
+    // we don't need to read from this, just carry it around so the Drop impl can run
+    #[allow(dead_code)]
+    kb_hook: KeyboardHookHandle,
+
     /// Tasks that should be executed at the end of `wnd_proc`. This is needed to avoid mutably
     /// borrowing the fields from `WindowState` more than once. For instance, when the window
     /// handler requests a resize in response to a keyboard event, the window state will already be
@@ -687,15 +692,13 @@ impl Window<'_> {
             );
             // todo: manage error ^
 
-            hook::init_keyboard_hook();
+            let kb_hook = hook::init_keyboard_hook(hwnd);
 
             #[cfg(feature = "opengl")]
-            let gl_context: Option<GlContext> = options
-                .gl_config
-                .map(|gl_config| {
-                    let mut handle = Win32WindowHandle::empty();
-                    handle.hwnd = hwnd as *mut c_void;
-                    let handle = RawWindowHandle::Win32(handle);
+            let gl_context: Option<GlContext> = options.gl_config.map(|gl_config| {
+                let mut handle = Win32WindowHandle::empty();
+                handle.hwnd = hwnd as *mut c_void;
+                let handle = RawWindowHandle::Win32(handle);
 
                 GlContext::create(&handle, gl_config).expect("Could not create OpenGL context")
             });
@@ -721,6 +724,8 @@ impl Window<'_> {
 
                 deferred_tasks: RefCell::new(VecDeque::with_capacity(4)),
 
+                kb_hook,
+
                 #[cfg(feature = "opengl")]
                 gl_context,
             });