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

Improve `HWND` validity testing in `HwndHost` #2252

Open
wants to merge 1 commit into
base: master
from

Conversation

@vatsan-madhavan
Copy link
Member

vatsan-madhavan commented Nov 27, 2019

Fixes #2239

s/_hwnd.Handle/Handle in a few places, notably in in a check in BuildOrReparentWindow

--

The key change is in BuildOrReparentWindow where an additional check for IsWindow() is desirable; without this check, non-NULL yet invalid HWND's are able to make their way into the "reparent under a SystemResources managed message-only HWND" logic, which in turn tries to query other characteristics of the (invalid) HWND.

One of these steps eventually fails, and WPF decides that the failure is sufficiently bad and crashes (this happens in MS.Internal.DpiUtil.HwndDpiInfo.NearestMonitorInfoFromWindow(IntPtr hwnd)).

Also changing _hwnd.Handle to Handle in a few other places where additional check for HWND validity seems valuable.

/cc @teh173

NearestMonitorDpiCrash.zip

The key change is in BuildOrReparentWindow where an additional check for  `IsWindow()` is desirable; without this check, non-`NULL` yet invalid `HWND`'s are able to make their way into the "reparent under a `SystemResources` managed message-only `HWND`" logic, which in turn tries to query other characteristics of the (invalid) `HWND`. One of these steps eventually fails, and WPF decides that the failure is sufficiently bad and decides to crash (this happens in `MS.Internal.DpiUtil.HwndDpiInfo.NearestMonitorInfoFromWindow(IntPtr hwnd)`).

Also changing `_hwnd.Handle` to `Handle` in a few other places where additional check for `HWND` validity seems valuable.
@msftbot msftbot bot requested a review from rladuca Nov 27, 2019
@msftbot msftbot bot added the PR label Nov 27, 2019
@msftbot msftbot bot requested a review from SamBent Nov 27, 2019
@vatsan-madhavan vatsan-madhavan added this to the 5.0 milestone Nov 27, 2019
@vatsan-madhavan vatsan-madhavan requested a review from arpitmathur Nov 27, 2019
@vatsan-madhavan vatsan-madhavan self-assigned this Nov 27, 2019
@teh173

This comment has been minimized.

Copy link

teh173 commented Dec 2, 2019

@vatsan-madhavan Thanks! Just wondering, (when) do changes like this get backported to .NET 4.8 Framework?

@vatsan-madhavan

This comment has been minimized.

Copy link
Member Author

vatsan-madhavan commented Dec 2, 2019

Thanks! Just wondering, (when) do changes like this get backported to .NET 4.8 Framework?

If there is a strong enough business justification (large number of crashes in the ecosystem, for e.g.), or a customer support request channeled via enterprise-support, we consider adding fixes to 4.8. Otherwise only the latest actively developed version of the product gets fixes. At this time, that means .NET 5.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
2 participants
You can’t perform that action at this time.