From 79d9db6240aa44458cdc7616f3094c88a1352b21 Mon Sep 17 00:00:00 2001 From: Adrien Prokopowicz <6529475+prokopyl@users.noreply.github.com> Date: Tue, 11 Aug 2026 00:58:57 +0200 Subject: [PATCH 1/3] Win32: Fix dropping handle triggering a warning if window was already closed --- src/platform/win/window.rs | 4 ++++ src/wrappers/win32/window/data.rs | 7 +++++-- src/wrappers/win32/window/proc.rs | 24 ++++++++++++------------ 3 files changed, 21 insertions(+), 14 deletions(-) diff --git a/src/platform/win/window.rs b/src/platform/win/window.rs index afe0519b..1946c1fa 100644 --- a/src/platform/win/window.rs +++ b/src/platform/win/window.rs @@ -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() { diff --git a/src/wrappers/win32/window/data.rs b/src/wrappers/win32/window/data.rs index fd5d6a86..ae191cc6 100644 --- a/src/wrappers/win32/window/data.rs +++ b/src/wrappers/win32/window/data.rs @@ -27,9 +27,12 @@ impl WindowData { } /// Returns an owned pointer from the given raw pointer, without transferring ownership. - pub unsafe fn from_raw(raw: NonNull>) -> Rc { + pub unsafe fn handle( + raw: NonNull>, handler: impl FnOnce(&WindowData) -> 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> { diff --git a/src/wrappers/win32/window/proc.rs b/src/wrappers/win32/window/proc.rs index ee8bc7c6..a2bdda61 100644 --- a/src/wrappers/win32/window/proc.rs +++ b/src/wrappers/win32/window/proc.rs @@ -21,17 +21,17 @@ pub unsafe extern "system" fn wnd_proc( 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,7 +48,7 @@ pub unsafe extern "system" fn wnd_proc( 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::()); @@ -56,7 +56,7 @@ pub unsafe extern "system" fn wnd_proc( // 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( // 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) + }) + } } } } From b674bbfaa8f21051204d7feddc0d52bcc50c5eb6 Mon Sep 17 00:00:00 2001 From: Adrien Prokopowicz <6529475+prokopyl@users.noreply.github.com> Date: Tue, 11 Aug 2026 01:16:12 +0200 Subject: [PATCH 2/3] Win32: Fix circular reference permanently holding onto WindowHandler --- src/platform/win/drop_target.rs | 19 +++++++----- src/platform/win/window.rs | 49 +++++++++++++++++++++---------- src/platform/win/window_state.rs | 24 ++------------- src/wrappers/win32/window.rs | 2 +- src/wrappers/win32/window/data.rs | 4 +++ 5 files changed, 53 insertions(+), 45 deletions(-) diff --git a/src/platform/win/drop_target.rs b/src/platform/win/drop_target.rs index 7dc35601..98c65e33 100644 --- a/src/platform/win/drop_target.rs +++ b/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::::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, }; diff --git a/src/platform/win/window.rs b/src/platform/win/window.rs index 1946c1fa..995645eb 100644 --- a/src/platform/win/window.rs +++ b/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; @@ -205,6 +205,7 @@ pub struct BaseviewWindow { initial_size: Size, handler_builder: Cell>, + handler: OnceCell>, host: Host, // Things not directly used, but kept so their Drop impl runs when the window is destroyed @@ -237,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, @@ -281,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 { @@ -337,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(()) } @@ -370,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; @@ -384,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) @@ -411,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 @@ -473,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 } @@ -500,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 { @@ -510,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 } @@ -534,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); @@ -585,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) { diff --git a/src/platform/win/window_state.rs b/src/platform/win/window_state.rs index 75e00247..9a317db0 100644 --- a/src/platform/win/window_state.rs +++ b/src/platform/win/window_state.rs @@ -5,8 +5,8 @@ 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}; @@ -22,8 +22,6 @@ pub(crate) struct WindowState { pub mouse_button_counter: Cell, pub mouse_was_outside_window: Cell, pub cursor_icon: Cell, - // Initialized late so the `Window` can hold a reference to this `WindowState` - pub handler: OnceCell>, pub user32: ExtendedUser32, pub shared: Rc, @@ -40,7 +38,6 @@ 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, @@ -49,23 +46,6 @@ impl WindowState { } } - 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() diff --git a/src/wrappers/win32/window.rs b/src/wrappers/win32/window.rs index 1b7816ec..26f94dd9 100644 --- a/src/wrappers/win32/window.rs +++ b/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; diff --git a/src/wrappers/win32/window/data.rs b/src/wrappers/win32/window/data.rs index ae191cc6..fc7f0f79 100644 --- a/src/wrappers/win32/window/data.rs +++ b/src/wrappers/win32/window/data.rs @@ -58,6 +58,10 @@ impl WindowData { } } + 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 { From 7f9c0d9796315042b077e5670f86daed77ab0064 Mon Sep 17 00:00:00 2001 From: Adrien Prokopowicz <6529475+prokopyl@users.noreply.github.com> Date: Tue, 11 Aug 2026 02:26:07 +0200 Subject: [PATCH 3/3] Fix lint --- src/platform/win/window_state.rs | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/src/platform/win/window_state.rs b/src/platform/win/window_state.rs index 9a317db0..b8e739de 100644 --- a/src/platform/win/window_state.rs +++ b/src/platform/win/window_state.rs @@ -9,7 +9,7 @@ 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; @@ -27,7 +27,7 @@ pub(crate) struct WindowState { pub shared: Rc, #[cfg(feature = "opengl")] - pub gl_context: OnceCell, + pub gl_context: std::cell::OnceCell, } impl WindowState { @@ -42,7 +42,7 @@ impl WindowState { shared, #[cfg(feature = "opengl")] - gl_context: OnceCell::new(), + gl_context: std::cell::OnceCell::new(), } }