Skip to content

Commit

Permalink
fix(Windows): fix a deadlock in WindowState (rust-windowing#338)
Browse files Browse the repository at this point in the history
* fix(Windows): fix a deadlock in `WindowState`

* add change file
  • Loading branch information
amrbashir authored Mar 6, 2022
1 parent 11dac10 commit 475e64d
Show file tree
Hide file tree
Showing 6 changed files with 44 additions and 24 deletions.
5 changes: 5 additions & 0 deletions .changes/window-state.keyboard-deadlock.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
---
"tao": "patch"
---

Fix a deadlock on Windows when using `Window::set_visible(true)` in the `EventLoop::run` closure.
17 changes: 8 additions & 9 deletions src/platform_impl/windows/event_loop.rs
Original file line number Diff line number Diff line change
Expand Up @@ -801,11 +801,7 @@ unsafe fn process_control_flow<T: 'static>(runner: &EventLoopRunner<T>) {
fn update_modifiers<T>(window: HWND, subclass_input: &SubclassInput<T>) -> ModifiersState {
use crate::event::WindowEvent::ModifiersChanged;

let modifiers = {
let mut layouts = LAYOUT_CACHE.lock().unwrap();
layouts.get_agnostic_mods()
};

let modifiers = LAYOUT_CACHE.lock().get_agnostic_mods();
let mut window_state = subclass_input.window_state.lock();
if window_state.modifiers_state != modifiers {
window_state.modifiers_state = modifiers;
Expand Down Expand Up @@ -904,10 +900,13 @@ unsafe fn public_window_callback_inner<T: 'static>(
return;
}
let events = {
let mut window_state = subclass_input.window_state.lock();
window_state
.key_event_builder
.process_message(window, msg, wparam, lparam, &mut result)
let mut key_event_builders =
crate::platform_impl::platform::keyboard::KEY_EVENT_BUILDERS.lock();
if let Some(key_event_builder) = key_event_builders.get_mut(&WindowId(window.0)) {
key_event_builder.process_message(window, msg, wparam, lparam, &mut result)
} else {
Vec::new()
}
};
for event in events {
subclass_input.send_event(Event::WindowEvent {
Expand Down
34 changes: 23 additions & 11 deletions src/platform_impl/windows/keyboard.rs
Original file line number Diff line number Diff line change
@@ -1,6 +1,10 @@
use parking_lot::{Mutex, MutexGuard};
use std::{
char, collections::HashSet, ffi::OsString, mem::MaybeUninit, os::windows::ffi::OsStringExt,
sync::MutexGuard,
char,
collections::{HashMap, HashSet},
ffi::OsString,
mem::MaybeUninit,
os::windows::ffi::OsStringExt,
};

use windows::Win32::{
Expand All @@ -17,10 +21,13 @@ use unicode_segmentation::UnicodeSegmentation;
use crate::{
event::{ElementState, KeyEvent},
keyboard::{Key, KeyCode, KeyLocation, NativeKeyCode},
platform_impl::platform::{
event_loop::ProcResult,
keyboard_layout::{get_or_insert_str, Layout, LayoutCache, WindowsModifiers, LAYOUT_CACHE},
KeyEventExtra,
platform_impl::{
platform::{
event_loop::ProcResult,
keyboard_layout::{get_or_insert_str, Layout, LayoutCache, WindowsModifiers, LAYOUT_CACHE},
KeyEventExtra,
},
WindowId,
},
};

Expand All @@ -37,6 +44,11 @@ pub struct MessageAsKeyEvent {
pub is_synthetic: bool,
}

lazy_static! {
pub(crate) static ref KEY_EVENT_BUILDERS: Mutex<HashMap<WindowId, KeyEventBuilder>> =
Mutex::new(HashMap::new());
}

/// Stores information required to make `KeyEvent`s.
///
/// A single Tao `KeyEvent` contains information which the Windows API passes to the application
Expand Down Expand Up @@ -98,7 +110,7 @@ impl KeyEventBuilder {
}
*result = ProcResult::Value(LRESULT(0));

let mut layouts = LAYOUT_CACHE.lock().unwrap();
let mut layouts = LAYOUT_CACHE.lock();
let event_info =
PartialKeyEventInfo::from_message(wparam, lparam, ElementState::Pressed, &mut layouts);

Expand Down Expand Up @@ -148,7 +160,7 @@ impl KeyEventBuilder {
// At this point, we know that there isn't going to be any more events related to
// this key press
let event_info = self.event_info.take().unwrap();
let mut layouts = LAYOUT_CACHE.lock().unwrap();
let mut layouts = LAYOUT_CACHE.lock();
let ev = event_info.finalize(&mut layouts.strings);
return vec![MessageAsKeyEvent {
event: ev,
Expand Down Expand Up @@ -220,7 +232,7 @@ impl KeyEventBuilder {
return vec![];
}
};
let mut layouts = LAYOUT_CACHE.lock().unwrap();
let mut layouts = LAYOUT_CACHE.lock();
// It's okay to call `ToUnicode` here, because at this point the dead key
// is already consumed by the character.
let kbd_state = get_kbd_state();
Expand Down Expand Up @@ -259,7 +271,7 @@ impl KeyEventBuilder {
win32wm::WM_KEYUP | win32wm::WM_SYSKEYUP => {
*result = ProcResult::Value(LRESULT(0));

let mut layouts = LAYOUT_CACHE.lock().unwrap();
let mut layouts = LAYOUT_CACHE.lock();
let event_info =
PartialKeyEventInfo::from_message(wparam, lparam, ElementState::Released, &mut layouts);
let mut next_msg = MaybeUninit::uninit();
Expand Down Expand Up @@ -306,7 +318,7 @@ impl KeyEventBuilder {
) -> Vec<MessageAsKeyEvent> {
let mut key_events = Vec::new();

let mut layouts = LAYOUT_CACHE.lock().unwrap();
let mut layouts = LAYOUT_CACHE.lock();
let (locale_id, _) = layouts.get_current_layout();

let is_key_pressed = |vk: VIRTUAL_KEY| &kbd_state[usize::from(vk)] & 0x80 != 0;
Expand Down
2 changes: 1 addition & 1 deletion src/platform_impl/windows/keyboard_layout.rs
Original file line number Diff line number Diff line change
@@ -1,8 +1,8 @@
use parking_lot::Mutex;
use std::{
collections::{hash_map::Entry, HashMap, HashSet},
ffi::OsString,
os::windows::ffi::OsStringExt,
sync::Mutex,
};

use lazy_static::lazy_static;
Expand Down
6 changes: 6 additions & 0 deletions src/platform_impl/windows/window.rs
Original file line number Diff line number Diff line change
Expand Up @@ -52,6 +52,8 @@ use crate::{
},
};

use super::keyboard::{KeyEventBuilder, KEY_EVENT_BUILDERS};

struct HMenuWrapper(HMENU);
unsafe impl Send for HMenuWrapper {}
unsafe impl Sync for HMenuWrapper {}
Expand Down Expand Up @@ -913,6 +915,10 @@ unsafe fn init<T: 'static>(
menu: None,
};

KEY_EVENT_BUILDERS
.lock()
.insert(win.id(), KeyEventBuilder::default());

win.set_skip_taskbar(pl_attribs.skip_taskbar);

let dimensions = attributes
Expand Down
4 changes: 1 addition & 3 deletions src/platform_impl/windows/window_state.rs
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,7 @@ use crate::{
dpi::{PhysicalPosition, Size},
icon::Icon,
keyboard::ModifiersState,
platform_impl::platform::{event_loop, keyboard::KeyEventBuilder, minimal_ime::MinimalIme, util},
platform_impl::platform::{event_loop, minimal_ime::MinimalIme, util},
window::{CursorIcon, Fullscreen, Theme, WindowAttributes},
};
use parking_lot::MutexGuard;
Expand Down Expand Up @@ -36,7 +36,6 @@ pub struct WindowState {
pub preferred_theme: Option<Theme>,
pub high_surrogate: Option<u16>,

pub key_event_builder: KeyEventBuilder,
pub ime_handler: MinimalIme,

pub window_flags: WindowFlags,
Expand Down Expand Up @@ -126,7 +125,6 @@ impl WindowState {
current_theme,
preferred_theme,
high_surrogate: None,
key_event_builder: KeyEventBuilder::default(),
ime_handler: MinimalIme::default(),
window_flags: WindowFlags::empty(),
}
Expand Down

0 comments on commit 475e64d

Please sign in to comment.