Skip to content
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

Handle overwrite #1203

Closed
jgdrg opened this issue Oct 3, 2022 · 3 comments · Fixed by #1204
Closed

Handle overwrite #1203

jgdrg opened this issue Oct 3, 2022 · 3 comments · Fixed by #1204

Comments

@jgdrg
Copy link
Contributor

jgdrg commented Oct 3, 2022

Describe the bug
A 3rd-party application that calls Windows UI GDI functions on two threads at same time can cause handle overwrite

Expected behavior
Handles should not get overwritten and lost which leads to UI corruption/missing window objects and potentially crashes

Screenshots
If applicable, add screenshots to help explain your problem.

Environment (please complete the following information):

  • OS: Windows 10 20H2

Additional context
I have included a WINEDEBUG log. I tweaked the logging in wow_handle.c to use TRACE rather than DPRINTF so the thread ids are recorded to help show the issue.
I have highlighted an some examples of the issue like this:

**** 1694:trace:thunk:get_handle16_data allocate HGDI 5C01167C=>0704 ****
1694:trace:thunk:K32WOWHandle16HGDI HGDI3216 5C01167C 0704
1694:Ret  USER.39: BEGINPAINT() retval=00000704 ret=28e7:a92a ds=291f
**** 3ff8:trace:thunk:get_handle16_data allocate HGDI F2010926=>0704 ****
3ff8:trace:thunk:K32WOWHandle16HGDI HGDI3216 F2010926 0704

WINEDEBUG.zip

@cracyc
Copy link
Contributor

cracyc commented Oct 3, 2022

This is a race condition between BeginPaint and WM_ERASEBKGND, https://github.com/otya128/winevdm/blob/master/user/message.c#L2438. Since BeginPaint doesn't release the thunk lock but WINPROC_CallProc32ATo16 doesn't wait on it probably need a separate critical section around the handle allocation.

@cracyc
Copy link
Contributor

cracyc commented Oct 4, 2022

Try #1204

@jgdrg
Copy link
Contributor Author

jgdrg commented Oct 4, 2022

Thanks. That looks like it has fixed the issue.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants