Win32 fix destruction

src/platform/win/drop_target.rs

@@ -13,7 +13,8 @@ use windows_core::Ref;
 use windows_sys::Win32::UI::Shell::DragQueryFileW;
 
 use super::window_state::WindowState;
-use crate::wrappers::win32::window::HWnd;
+use crate::platform::BaseviewWindow;
+use crate::wrappers::win32::window::{HWnd, WindowData};
 use crate::{DropData, DropEffect, Event, EventStatus, MouseEvent};
 
 #[implement(IDropTarget)]
@@ -39,18 +40,22 @@ impl DropTarget {
 
     #[allow(non_snake_case)]
     fn on_event(&self, pdwEffect: Option<*mut DROPEFFECT>, event: MouseEvent) {
-        let Some(window_state) = self.window_state.upgrade() else {
+        let Some(window_data_ptr) = self.hwnd.get_userdata_ptr() else {
             return;
         };
 
         let event = Event::Mouse(event);
-        let event_status = window_state.handle_event(event);
+        let event_status = unsafe {
+            WindowData::<BaseviewWindow>::handle(window_data_ptr, |window| {
+                window.inner().map(|w| w.handle_event(event))
+            })
+        };
 
         let effect = match event_status {
-            EventStatus::AcceptDrop(DropEffect::Copy) => DROPEFFECT_COPY,
-            EventStatus::AcceptDrop(DropEffect::Move) => DROPEFFECT_MOVE,
-            EventStatus::AcceptDrop(DropEffect::Link) => DROPEFFECT_LINK,
-            EventStatus::AcceptDrop(DropEffect::Scroll) => DROPEFFECT_SCROLL,
+            Some(EventStatus::AcceptDrop(DropEffect::Copy)) => DROPEFFECT_COPY,
+            Some(EventStatus::AcceptDrop(DropEffect::Move)) => DROPEFFECT_MOVE,
+            Some(EventStatus::AcceptDrop(DropEffect::Link)) => DROPEFFECT_LINK,
+            Some(EventStatus::AcceptDrop(DropEffect::Scroll)) => DROPEFFECT_SCROLL,
             _ => DROPEFFECT_NONE,
         };
 

src/platform/win/window.rs

@@ -4,9 +4,9 @@ use windows_sys::Win32::{
     UI::{Controls::WM_MOUSELEAVE, WindowsAndMessaging::*},
 };
 
-use crate::{warn, HandlerError};
+use crate::{warn, EventStatus, HandlerError, WindowHandler};
 use dpi::{PhysicalPosition, PhysicalSize, Size};
-use std::cell::Cell;
+use std::cell::{Cell, OnceCell};
 use std::num::NonZeroUsize;
 use windows_sys::Win32::Foundation::POINT;
 
@@ -186,6 +186,10 @@ impl WindowHandle {
 
 impl Drop for WindowHandle {
     fn drop(&mut self) {
+        if !self.state.is_alive.get() {
+            return;
+        }
+
         if let Some(hwnd) = self.hwnd.take() {
             let _guard = self.state.originate_host_destroy();
             if let Err(e) = hwnd.destroy() {
@@ -201,6 +205,7 @@ pub struct BaseviewWindow {
     initial_size: Size,
 
     handler_builder: Cell<Option<WindowHandlerBuilder>>,
+    handler: OnceCell<Box<dyn WindowHandler>>,
     host: Host,
 
     // Things not directly used, but kept so their Drop impl runs when the window is destroyed
@@ -233,6 +238,7 @@ impl BaseviewWindow {
                     window_state,
                     initial_size: init.settings.size,
                     handler_builder: Cell::new(Some(init.builder)),
+                    handler: OnceCell::new(),
                     shared_state,
                     host: init.host,
 
@@ -277,6 +283,23 @@ impl BaseviewWindow {
 
         self.host.request_resize(new_size)
     }
+
+    pub(crate) fn handle_on_frame(&self) {
+        let Some(handler) = self.handler.get() else { return };
+
+        if let Err(e) = handler.on_frame() {
+            warn!("Error while rendering frame: {}", e);
+            self.window_state.request_close();
+        }
+    }
+
+    pub(crate) fn handle_event(&self, event: Event) -> EventStatus {
+        let Some(handler) = self.handler.get() else {
+            return EventStatus::Ignored;
+        };
+
+        handler.on_event(event)
+    }
 }
 
 impl Drop for BaseviewWindow {
@@ -333,7 +356,7 @@ impl WindowImpl for BaseviewWindow {
             let context = crate::WindowContext::new(Rc::clone(&self.window_state));
             self.handler_builder.take().unwrap().build(context)?
         };
-        let Ok(()) = window_state.handler.set(handler) else { unreachable!() };
+        let Ok(()) = self.handler.set(handler) else { unreachable!() };
 
         Ok(())
     }
@@ -366,7 +389,7 @@ unsafe fn wnd_proc_inner(
                 window_state.mouse_was_outside_window.set(false);
 
                 let enter_event = Event::Mouse(MouseEvent::CursorEntered);
-                window_state.handle_event(enter_event);
+                window_bv.handle_event(enter_event);
             }
 
             let x = (lparam & 0xFFFF) as i16 as i32;
@@ -380,12 +403,12 @@ unsafe fn wnd_proc_inner(
                     .get_modifiers_from_mouse_wparam(wparam),
             });
 
-            window_state.handle_event(move_event);
+            window_bv.handle_event(move_event);
             Some(0)
         }
 
         WM_MOUSELEAVE => {
-            window_state.handle_event(Event::Mouse(MouseEvent::CursorLeft));
+            window_bv.handle_event(Event::Mouse(MouseEvent::CursorLeft));
 
             window_state.mouse_was_outside_window.set(true);
             Some(0)
@@ -407,7 +430,7 @@ unsafe fn wnd_proc_inner(
                     .get_modifiers_from_mouse_wparam(wparam),
             });
 
-            window_state.handle_event(event);
+            window_bv.handle_event(event);
             Some(0)
         }
         WM_LBUTTONDOWN | WM_LBUTTONUP | WM_MBUTTONDOWN | WM_MBUTTONUP | WM_RBUTTONDOWN
@@ -469,20 +492,20 @@ unsafe fn wnd_proc_inner(
                 };
 
                 window_state.mouse_button_counter.set(mouse_button_counter);
-                window_state.handle_event(Event::Mouse(event));
+                window_bv.handle_event(Event::Mouse(event));
             }
 
             None
         }
         WM_TIMER => {
             if wparam == WIN_FRAME_TIMER.get() {
-                window_state.handle_on_frame()
+                window_bv.handle_on_frame()
             }
 
             Some(0)
         }
         WM_CLOSE => {
-            window_state.handle_event(Event::Window(WindowEvent::WillClose));
+            window_bv.handle_event(Event::Window(WindowEvent::WillClose));
 
             None
         }
@@ -496,7 +519,7 @@ unsafe fn wnd_proc_inner(
             );
 
             if let Some(event) = opt_event {
-                window_state.handle_event(Event::Keyboard(event));
+                window_bv.handle_event(Event::Keyboard(event));
             }
 
             if msg != WM_SYSKEYDOWN {
@@ -506,12 +529,12 @@ unsafe fn wnd_proc_inner(
             }
         }
         WM_SETFOCUS => {
-            window_state.handle_event(Event::Window(WindowEvent::Focused));
+            window_bv.handle_event(Event::Window(WindowEvent::Focused));
 
             None
         }
         WM_KILLFOCUS => {
-            window_state.handle_event(Event::Window(WindowEvent::Unfocused));
+            window_bv.handle_event(Event::Window(WindowEvent::Unfocused));
 
             None
         }
@@ -530,7 +553,7 @@ unsafe fn wnd_proc_inner(
             let previous = window_state.shared.current_size.replace(new_size);
             let new_size = WindowSize::from_physical(new_size, window_state.shared.scale_factor());
 
-            let handler = window_state.handler.get()?;
+            let handler = window_bv.handler.get()?;
             if let Err(e) = handler.resized(new_size) {
                 warn!("Window Handler failed to resize: {}", e);
                 window_state.shared.current_size.set(previous);
@@ -581,7 +604,7 @@ unsafe fn wnd_proc_inner(
             let _ = window.set_nc_rect(suggested_nc_rect);
 
             if changed {
-                let handler = window_state.handler.get()?;
+                let handler = window_bv.handler.get()?;
                 let new_size = WindowSize::from_physical(new_size, dpi.scale_factor());
 
                 if let Err(e) = handler.resized(new_size) {

src/platform/win/window_state.rs

@@ -5,11 +5,11 @@ use crate::wrappers::win32::cursor::SystemCursor;
 use crate::wrappers::win32::h_instance::HInstance;
 use crate::wrappers::win32::window::HWnd;
 use crate::wrappers::win32::{Dpi, ExtendedUser32};
-use crate::{warn, WindowSettings};
-use crate::{Event, EventStatus, MouseCursor, WindowHandler, WindowSize};
+use crate::WindowSettings;
+use crate::{MouseCursor, WindowSize};
 use dpi::{PhysicalSize, Size};
 use raw_window_handle::{DisplayHandle, Win32WindowHandle};
-use std::cell::{Cell, OnceCell, Ref, RefCell};
+use std::cell::{Cell, Ref, RefCell};
 use std::num::NonZeroIsize;
 use std::rc::Rc;
 use windows_sys::Win32::UI::WindowsAndMessaging::PostMessageW;
@@ -22,14 +22,12 @@ pub(crate) struct WindowState {
     pub mouse_button_counter: Cell<usize>,
     pub mouse_was_outside_window: Cell<bool>,
     pub cursor_icon: Cell<MouseCursor>,
-    // Initialized late so the `Window` can hold a reference to this `WindowState`
-    pub handler: OnceCell<Box<dyn WindowHandler>>,
 
     pub user32: ExtendedUser32,
     pub shared: Rc<WindowSharedState>,
 
     #[cfg(feature = "opengl")]
-    pub gl_context: OnceCell<super::gl::GlContext>,
+    pub gl_context: std::cell::OnceCell<super::gl::GlContext>,
 }
 
 impl WindowState {
@@ -40,32 +38,14 @@ impl WindowState {
             mouse_button_counter: Cell::new(0),
             mouse_was_outside_window: true.into(),
             cursor_icon: Cell::new(MouseCursor::Default),
-            handler: OnceCell::new(),
             user32,
             shared,
 
             #[cfg(feature = "opengl")]
-            gl_context: OnceCell::new(),
+            gl_context: std::cell::OnceCell::new(),
         }
     }
 
-    pub(crate) fn handle_on_frame(&self) {
-        let Some(handler) = self.handler.get() else { return };
-
-        if let Err(e) = handler.on_frame() {
-            warn!("Error while rendering frame: {}", e);
-            self.request_close();
-        }
-    }
-
-    pub(crate) fn handle_event(&self, event: Event) -> EventStatus {
-        let Some(handler) = self.handler.get() else {
-            return EventStatus::Ignored;
-        };
-
-        handler.on_event(event)
-    }
-
     /// Returns the current size of this window.
     pub fn size(&self) -> WindowSize {
         self.shared.size()

src/wrappers/win32/window.rs

@@ -12,7 +12,7 @@ mod wgl;
 #[cfg(feature = "opengl")]
 pub use wgl::*;
 
-use data::WindowData;
+pub use data::WindowData;
 use dpi::PhysicalSize;
 pub use handle::HWnd;
 pub use proc::wnd_proc;

src/wrappers/win32/window/data.rs

@@ -27,9 +27,12 @@ impl<W: WindowImpl> WindowData<W> {
     }
 
     /// Returns an owned pointer from the given raw pointer, without transferring ownership.
-    pub unsafe fn from_raw(raw: NonNull<WindowData<W>>) -> Rc<Self> {
+    pub unsafe fn handle<T>(
+        raw: NonNull<WindowData<W>>, handler: impl FnOnce(&WindowData<W>) -> T,
+    ) -> T {
         let this = ManuallyDrop::new(Rc::from_raw(raw.as_ptr()));
-        Rc::clone(&this)
+        let this = Rc::clone(&this);
+        handler(&this)
     }
 
     pub fn initialize(&self, window: HWnd) -> core::result::Result<(), crate::platform::Error> {
@@ -55,6 +58,10 @@ impl<W: WindowImpl> WindowData<W> {
         }
     }
 
+    pub fn inner(&self) -> Option<&W> {
+        self.inner_impl.get()
+    }
+
     pub unsafe fn handle_message(
         &self, window: HWnd, message_code: u32, w_param: WPARAM, l_param: LPARAM,
     ) -> Option<LRESULT> {

src/wrappers/win32/window/proc.rs

@@ -21,17 +21,17 @@ pub unsafe extern "system" fn wnd_proc<W: WindowImpl>(
 
             let Some(inner_ptr) = NonNull::new(inner_ptr) else {
                 // If the state pointer was null for some weird reason, we just abort.
-                // TODO: log error
+                crate::error!("Failed to create window: lpCreateParams was NULL");
                 return -1;
             };
 
-            if let Err(_e) = window.set_userdata_ptr(inner_ptr.as_ptr()) {
+            if let Err(e) = window.set_userdata_ptr(inner_ptr.as_ptr()) {
                 // The call to SetWindowLongPtrW failed for some reason, we cannot continue.
 
                 // Recover and free the received pointer data.
                 drop(Rc::from_raw(inner_ptr.as_ptr()));
 
-                // TODO: log error
+                crate::error!("Failed to create window: SetWindowLongPtrW failed: {}", e);
                 return -1;
             }
 
@@ -48,15 +48,15 @@ pub unsafe extern "system" fn wnd_proc<W: WindowImpl>(
                 Ok(()) => 0,
 
                 // If initializer failed, abort.
-                Err(_) => {
+                Err(e) => {
                     // First, revoke ownership from the window, we don't want it to be used by any subsequent messages.
                     let _ = window.set_userdata_ptr(core::ptr::null::<W>());
 
                     // Try to recover and free the received pointer data. But if this also fails, better to leak
                     // it than risk crashing
                     drop(Rc::from_raw(inner_ptr.as_ptr()));
 
-                    // TODO: log error
+                    crate::error!("Window initializer failed while trying to create window: {}", e);
                     -1
                 }
             }
@@ -83,13 +83,13 @@ pub unsafe extern "system" fn wnd_proc<W: WindowImpl>(
 
             // This guarantees WindowData remains valid until the end of this scope,
             // even if the event handler leads to the window being destroyed
-            let inner = unsafe { WindowData::from_raw(inner_ptr) };
-
-            let result = inner.handle_message(window, message_code, w_param, l_param);
-
-            drop(inner);
-
-            result.unwrap_or_else(handle_default)
+            unsafe {
+                WindowData::handle(inner_ptr, |inner| {
+                    inner
+                        .handle_message(window, message_code, w_param, l_param)
+                        .unwrap_or_else(handle_default)
+                })
+            }
         }
     }
 }