FIX: Fixed GDI object leak when a resizable window is created and then closed on Windows platform #14209
Conversation
Package Changes Through f851307There are 9 changes which include @tauri-apps/api with minor, tauri-cli with minor, tauri-utils with minor, tauri-runtime-wry with minor, tauri-runtime with minor, tauri with minor, tauri-bundler with minor, @tauri-apps/cli with minor, tauri-macros with patch Planned Package VersionsThe following package releases are the planned based on the context of changes in this pull request.
Add another change file through the GitHub UI by following this link. Read about change files or the docs at github.com/jbolda/covector |
|
|
||
| CombineRgn(Some(hrgn1), Some(hrgn1), Some(hrgn2), RGN_DIFF); | ||
|
|
||
| DeleteObject(hrgn2.into()); |
There was a problem hiding this comment.
This is not needed as HRGN type implements the Free trait, which means it automatically calls DeleteObject on drop
Are you sure this leaked? If so, can you give me instructions that I can use to reproduce and inspect?
There was a problem hiding this comment.
It implements Free trait but not Drop trait. As far as I understand that means that there are two patterns in windows-rs:
Raw handle:
If you’re just holding an HRGN, nothing will free it automatically. You must call DeleteObject yourself.
Owning handle wrapper:
If you wrap it in something like Owned or Option<Owned>, then the Drop implementation of Owned calls Free::free automatically, so the handle is released when it goes out of scope.
In our case CreateRectRgn return HRGN and it's not released automatically.
There was a problem hiding this comment.
You're right, in that case I think we should probably just wrap it in Owned then like this
Owned::new(CreateRectRgn(...));so that we don't need to worry about which method we should call to free an object since the Free trait already does that
There was a problem hiding this comment.
Or just hrgn2.free(), I guess.
There was a problem hiding this comment.
Yeah, but Owned is better so it is freed at the end of scope implicitly using the Drop trait which is more aligned with Rust
There was a problem hiding this comment.
And while you are there you can add some other fix of much less importance, actually just in case of pure art. Theoretically if SetWindowRgn fails the hrgn1 GDI object would also will leak in this function as it's not owned by Windows. Better to write it as:
if !SetWindowRgn(hwnd, Some(hrgn1), true).as_bool() {
// If it fails, we must free hrgn1 manually
hrgn1.free();
}In practice if SetWindowRgn fails something much more worse than GDI leak happens of course.
There was a problem hiding this comment.
yeah sounds good to me, feel free to update that as well
2 participants
When a window is created in a Tuari application with, for example, call to new WebviewWindow from the Javascript code and later is closed -- exactly 1 GDI object leaks. This happens because of the function set_drag_hwnd_rgn where 2 regions are created, one of them becomes managed by Windows after call to SetWindowRgn but another one remains dangling. Added correct release of the region handle after it is not used anymore.