From 6af385c09e0a1ca88f2a1ff47a2befb6751127c9 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=E5=BC=A0=E5=B0=8F=E7=99=BD?= <364772080@qq.com> Date: Sat, 27 Jul 2024 08:40:55 +0800 Subject: [PATCH] windows: Fix some weird IME window panic (#15286) Previously, we used messages greater than `WM_USER` to pass information between `WindowsPlatform` and `WindowsWindow`. For example, to close a window, we handled it as follows: 1. The window sends a message with `WM_USER + 2` to `WindowsPlatform`. 2. `WindowsPlatform`, upon receiving this message, casts the `lparam` to `HWND` and closes the window. According to Microsoft's documentation, it is safe to use values between `WM_USER` and `0xBFFF` as messages. However, certain versions of Microsoft's IME use `WM_USER + 2` for UNKNOWN purposes. This causes step 2 to be erroneously triggered. The IME window's `lparam` value could be arbitrary, leading to an attempt to close an arbitrary `HWND` and resulting in errors. It is quite surprising that Microsoft indicates using `WM_USER + 2` is safe, yet Microsoft itself breaks this convention. I mean, well done Microsoft! This PR addresses the issue by using the `wparam` with a specific random value for validation purpose when sending the aforementioned message. Before `WindowsPlatform` attempts to close the window, it will first verify the `wparam` value. Special thanks to @shenjackyuanjie for helping me on this. Co-authored-by: shenjackyuanjie <3695888@qq.com> Release Notes: - Fixed weird panic when IME window is closing(#15185, #12563). --------- Co-authored-by: shenjack <3695888@qq.com> --- crates/gpui/src/platform/windows/events.rs | 8 +++++++- crates/gpui/src/platform/windows/platform.rs | 21 ++++++++++++++++++-- crates/gpui/src/platform/windows/window.rs | 5 +++++ 3 files changed, 31 insertions(+), 3 deletions(-) diff --git a/crates/gpui/src/platform/windows/events.rs b/crates/gpui/src/platform/windows/events.rs index 70aac60658ea3e4ce806106ac9cc2116ff5cd7d9..986928ddea2025a51ed0fb38430e809deba4767f 100644 --- a/crates/gpui/src/platform/windows/events.rs +++ b/crates/gpui/src/platform/windows/events.rs @@ -217,7 +217,13 @@ fn handle_destroy_msg(handle: HWND, state_ptr: Rc) -> Opt callback(); } unsafe { - PostMessageW(None, CLOSE_ONE_WINDOW, None, LPARAM(handle.0 as isize)).log_err(); + PostMessageW( + None, + CLOSE_ONE_WINDOW, + WPARAM(state_ptr.validation_number), + LPARAM(handle.0 as isize), + ) + .log_err(); } Some(0) } diff --git a/crates/gpui/src/platform/windows/platform.rs b/crates/gpui/src/platform/windows/platform.rs index 45e404788d1be93c26d5aceb29bbae7fb9997823..edd69d4dc922c74355181cf284007016aed73358 100644 --- a/crates/gpui/src/platform/windows/platform.rs +++ b/crates/gpui/src/platform/windows/platform.rs @@ -58,6 +58,7 @@ pub(crate) struct WindowsPlatform { clipboard_metadata_format: u32, windows_version: WindowsVersion, bitmap_factory: ManuallyDrop, + validation_number: usize, } pub(crate) struct WindowsPlatformState { @@ -111,6 +112,7 @@ impl WindowsPlatform { let clipboard_metadata_format = register_clipboard_format(CLIPBOARD_METADATA_FORMAT).unwrap(); let windows_version = WindowsVersion::new().expect("Error retrieve windows version"); + let validation_number = rand::random::(); Self { state, @@ -123,6 +125,7 @@ impl WindowsPlatform { clipboard_metadata_format, windows_version, bitmap_factory, + validation_number, } } @@ -159,7 +162,16 @@ impl WindowsPlatform { }); } - fn close_one_window(&self, target_window: HWND) -> bool { + fn close_one_window( + &self, + target_window: HWND, + validation_number: usize, + msg: *const MSG, + ) -> bool { + if validation_number != self.validation_number { + unsafe { DispatchMessageW(msg) }; + return false; + } let mut lock = self.raw_window_handles.write(); let index = lock .iter() @@ -206,7 +218,11 @@ impl Platform for WindowsPlatform { match msg.message { WM_QUIT => break 'a, CLOSE_ONE_WINDOW => { - if self.close_one_window(HWND(msg.lParam.0 as _)) { + if self.close_one_window( + HWND(msg.lParam.0 as _), + msg.wParam.0, + &msg, + ) { break 'a; } } @@ -316,6 +332,7 @@ impl Platform for WindowsPlatform { self.foreground_executor.clone(), lock.current_cursor, self.windows_version, + self.validation_number, )?; drop(lock); let handle = window.get_raw_handle(); diff --git a/crates/gpui/src/platform/windows/window.rs b/crates/gpui/src/platform/windows/window.rs index 0904b615c4f0cc29eb0b1aa863bac83ed0ca4e9f..ce388d8444f96f0b25c6463d321c53fe563934d4 100644 --- a/crates/gpui/src/platform/windows/window.rs +++ b/crates/gpui/src/platform/windows/window.rs @@ -62,6 +62,7 @@ pub(crate) struct WindowsWindowStatePtr { pub(crate) is_movable: bool, pub(crate) executor: ForegroundExecutor, pub(crate) windows_version: WindowsVersion, + pub(crate) validation_number: usize, } impl WindowsWindowState { @@ -224,6 +225,7 @@ impl WindowsWindowStatePtr { is_movable: context.is_movable, executor: context.executor.clone(), windows_version: context.windows_version, + validation_number: context.validation_number, })) } } @@ -250,6 +252,7 @@ struct WindowCreateContext { executor: ForegroundExecutor, current_cursor: HCURSOR, windows_version: WindowsVersion, + validation_number: usize, } impl WindowsWindow { @@ -260,6 +263,7 @@ impl WindowsWindow { executor: ForegroundExecutor, current_cursor: HCURSOR, windows_version: WindowsVersion, + validation_number: usize, ) -> Result { let classname = register_wnd_class(icon); let hide_title_bar = params @@ -300,6 +304,7 @@ impl WindowsWindow { executor, current_cursor, windows_version, + validation_number, }; let lpparam = Some(&context as *const _ as *const _); let creation_result = unsafe {