-
-
Notifications
You must be signed in to change notification settings - Fork 3.2k
fix(windows): fix double free (STATUS_HEAP_CORRUPTION) of resizing handler's userdata #13968
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
Using WM_NCDESTROY instead of WM_DESTROY is more correct for freeing userdata, as windows can receive multiple WM_DESTROY events if they're parented.
Package Changes Through 5e1d54fThere are 8 changes which include tauri-utils with minor, tauri-cli with minor, @tauri-apps/cli with minor, tauri-bundler with patch, tauri-runtime-wry with patch, tauri with minor, @tauri-apps/api with minor, tauri-plugin with minor 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 |
Legend-Master
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks, could you also add a change file?
https://github.com/tauri-apps/tauri/blob/dev/.changes/README.md
as windows can receive multiple WM_DESTROY events if they're parented
I still want to know how/why is it receiving multiple WM_DESTROY events though, I don't think that should normally happen, and we might have another bug to fix
|
I wouldn't be surprised if it's due to some weird behavior with cross-parenting and not Tauri's fault at all? Google didn't help at all in that regard (I started off writing this trying to just prevent the double-free rather than seeing if the logic being in WM_DESTROY was the root issue). I'll go ahead and add a changes file! |
|
Reading online says WM_DESTROY can be received multiple times if one sends WM_DESTROY manually instead of calling For the record, without ASAN, the behavior in production Tauri as of right now means it still takes quite a while to receive a STATUS_HEAP_CORRUPTION. We had to create and destroy a bunch of windows to turn the double-free into a tangible error. It wouldn't shock me if there's issues elsewhere that nobody's caught just because they're so probabilistic, and getting ASAN to work on Windows is a chore (´・ω・`) |
Yeah, also calling
I don't think we have any code sending
If it doesn't cause any other problems after this change, I guess we just call it a day 🙃 |
Legend-Master
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks again 🙃
Using WM_NCDESTROY instead of WM_DESTROY is more correct for freeing userdata, as windows can receive multiple WM_DESTROY events if they're parented. WM_NCDESTROY is the last message a window can ever receive, whereas we were experiencing behavior where our application would crash due to a Tauri window receiving multiple WM_DESTROY events.
We tracked this down with ASAN (which reports a double-free almost immediately in our application) and this change fixes the issue.