Skip to content

Commit bdef144

Browse files
committed
fix(borders): prevent use-after-free in border destruction
This commit fixes a use-after-free bug in the border_manager that was causing crashes with exception code 0xc000041d when borders were being destroyed during workspace/monitor changes. The root cause was a race condition between the main thread destroying a border window and the border's window thread still processing queued window messages (WM_PAINT, EVENT_OBJECT_LOCATIONCHANGE). The crash occurred when these callbacks invoked render_target.EndDraw(), which internally calls HwndPresenter::Present() to present the rendered frame to the HWND. By this point, the HWND and its associated DirectX surfaces had already been freed, resulting in Direct2D attempting to dereference freed memory (0xFEEEFEEEFEEEFEEE - Windows heap poison value). The issue stemmed from two problems in destroy_border(): 1. Direct2D resources (render_target and brushes) were not being released before closing the window. These COM objects hold internal pointers to the HWND and its DirectX swap chain/surfaces. When close_window() was called, Windows began tearing down these resources while the Direct2D objects still held dangling pointers to them. 2. GWLP_USERDATA was not being cleared before closing the window. This meant that any messages already queued in the window's message queue could still retrieve the border_pointer and attempt to render using the now-invalid Direct2D resources. This commit addresses both issues: - In destroy_border() (mod.rs): Explicitly set render_target to None and clear the brushes HashMap before calling destroy(). This ensures that Direct2D COM objects are properly released while the HWND is still valid, preventing EndDraw() from accessing freed HWND resources. - In destroy() (border.rs): Clear GWLP_USERDATA before calling close_window(). This ensures that any pending window messages will see a null pointer and exit early from the callbacks (which already have null pointer checks in place). These changes create two layers of defense against the race condition: the callbacks won't access the border_pointer (it's null), and even if they somehow did, the render_target would be None so no rendering operations would occur. I think this is the root cause of a lot of crash tickets that people are mistakenly attributing to specific applications which lack reproducibility across different users/machines, i.e. #1626, #1624.
1 parent 0b39551 commit bdef144

File tree

2 files changed

+12
-0
lines changed

2 files changed

+12
-0
lines changed

komorebi/src/border_manager/border.rs

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -313,6 +313,11 @@ impl Border {
313313
}
314314

315315
pub fn destroy(&self) -> color_eyre::Result<()> {
316+
// clear user data **BEFORE** closing window
317+
// pending messages will see a null pointer and exit early
318+
unsafe {
319+
SetWindowLongPtrW(self.hwnd(), GWLP_USERDATA, 0);
320+
}
316321
WindowsApi::close_window(self.hwnd)
317322
}
318323

komorebi/src/border_manager/mod.rs

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -767,6 +767,13 @@ fn remove_border(
767767
fn destroy_border(border: Box<Border>) -> color_eyre::Result<()> {
768768
let raw_pointer = Box::into_raw(border);
769769
unsafe {
770+
// release d2d resources **BEFORE** destroying window
771+
// this drops render_target and brushes while HWND is still valid
772+
// prevents EndDraw() from accessing freed HWND resources
773+
(*raw_pointer).render_target = None;
774+
(*raw_pointer).brushes.clear();
775+
776+
// Now safe to destroy window
770777
(*raw_pointer).destroy()?;
771778
}
772779
Ok(())

0 commit comments

Comments
 (0)