Microsoft-ui-xaml: Bug: High severity crash of Windows 10 graphics stack

Created on 11 Jan 2020  ·  10Comments  ·  Source: microsoft/microsoft-ui-xaml

Describe the bug

I found a bug in UWP/WinUI that brings down the Windows 10 graphics stack. I’ve toned down my comments but my frustration with this class of bug is very high. It should never happen and indicates deep process and architectural issues. This is the type of bug that will make companies drop (or never pick up) UWP/WinUI. WPF is still miles ahead of UWP/WinUI because it was developed with enough resources, R&D/planning, managed code and a full testing department/infrastructure.

What is the bug? I didn't narrow it down fully (since it crashes windows and is in native code) but I believe accessing the Width of a Border within a Canvas (before the Width is calculated) crashes Windows 10's graphics stack. The Border width itself is calculated from its child TextBlock.

There may be security implications with this type of bug.

Sample application is embedded below. Keep in mind I pulled this code from a large application and just made it functional. Don't try to understand everything it's doing and look only at the last lines in the CrashWindows() method.

App4.zip

Steps to reproduce the bug

Run the attached sample application on your PC. DO NOT HAVE ANY UNSAVED WORK ON YOUR COMPUTER. CLOSE ALL OTHER APPLICATIONS BEFORE RUNNING THIS.

Expected behavior
Windows 10's graphics stack does not crash.

Version Info
NuGet package version: Microsoft.UI.Xaml 2.3.191211002
Windows 10 Home 10.0.18362
2013 Macbook Pro 15inch running Windows 10 using Bootcamp, NVIDIA GeForce GT 750M (the graphics card/driver may be important in understanding this bug)

bug team-Rendering

Most helpful comment

Thanks for reporting this and providing a repro app!

I repro the issue. The problem, at least in the reduced repro, is that labelBorder.Width is NaN, so SetLeft() ends up setting NaN. XAML passed this NaN along to Composition, which eventually hits a fatal error in dwm.exe (the system compositor responsible for putting any rendering on screen). The crash of dwm.exe makes the screen stop rendering until it restarts, and once it restarts, the NaN is still there so it crashes again, and after a few such attempts it logs the user off as a more extreme attempt to get rid of whatever caused the problem.

This is definitely something we need to fix, likely both in XAML and in dwm.exe. We'll use this issue to continue to track a XAML fix, and I've logged the dwm.exe crash internally as https://microsoft.visualstudio.com/DefaultCollection/OS/_workitems/edit/24644418.

All 10 comments

I have cleaned up your project @robloo and it seems like this is an issue with corner radius.

"Minimal" repro app:
Windows UI Crash App.zip

I can repro the issue with following configuration:
Microsoft.UI.Xaml 2.3.191211002
Windows Build 19541
NVIDIA GTX 1070, driver: 441.66

@chingucoding Thanks for confirming this and reducing the code quite a bit.

The reason I think it may be related to Width is because changing the code to 'ActualWidth' instead of 'Width' prevents the crash. Since this turns on/off the issue, it may be the source.

This also works in your sample app if you change this line:

Crash:
Canvas.SetLeft(labelBorder, (300 - labelBorder.Width - 6));

No Crash:
Canvas.SetLeft(labelBorder, (300 - labelBorder.ActualWidth - 6));

That said, I'm sure you left setting the border's CornerRadius in the code as that turns on/off the issue as well.

That said, I'm sure you left setting the border's CornerRadius in the code as that turns on/off the issue as well.

Yes leaving the corner radius out also ends in the bug not triggering. So this is probably a combination of the actual sizing calculation and corner radius calculation.

@codendone @gegao18 This seems to mess up with the graphics stack. It does not crash so i dont have a callstack, but gets it into a wierd state that requires a machine reboot to fix. Can you please take a look ?

It crashes the windows display driver.

Display driver failed to start; using Microsoft Basic Display Driver instead. Check Windows Update for a newer display driver.

Thanks for reporting this and providing a repro app!

I repro the issue. The problem, at least in the reduced repro, is that labelBorder.Width is NaN, so SetLeft() ends up setting NaN. XAML passed this NaN along to Composition, which eventually hits a fatal error in dwm.exe (the system compositor responsible for putting any rendering on screen). The crash of dwm.exe makes the screen stop rendering until it restarts, and once it restarts, the NaN is still there so it crashes again, and after a few such attempts it logs the user off as a more extreme attempt to get rid of whatever caused the problem.

This is definitely something we need to fix, likely both in XAML and in dwm.exe. We'll use this issue to continue to track a XAML fix, and I've logged the dwm.exe crash internally as https://microsoft.visualstudio.com/DefaultCollection/OS/_workitems/edit/24644418.

@codendone Awesome work tracking this down. This describes exactly what I was seeing on my end.

Any ideas what the XAML fix might look like? I imagine NaN will still fully be supported as it is in WPF, just WinUI will pass a 'valid' number to Composition replacing NaN when needed.

@robloo Yes, I believe NaN should still be supported in this XAML API, so the XAML fix would be to not pass NaN to Composition. We still need to determine exactly where/how it is best to do that XAML change.

I'm not a heavy Xaml user, and I've found the notion of separate Width & Actualwidth (and apparently there are more .. Rendersize.Width) completely ridiculous and bug prone. Maybe it all serves some purpose. But I only ever want a single variable representing the width of a control.

Even in my own custom UI's I've never needed more than a single variable to represent pixel width which should be consistent between logic and graphics.

Hi @Gavin-Williams, I'm going to be a little blunt here so please forgive me. The various flavors of Microsoft UI tech that use XAML as markup have a very powerful layout system. It is far more advanced than Windows Forms where everything is just arranged on a panel-type control with Top/Left/Height/Width properties. Both Width and ActualWidth are mandatory for the layout system and cannot be removed. This allows sizing children based on available parent width, etc.

Setting Width to NaN is valid in several cases for auto-sizing. ActualWidth is usually always the real, rendered, device-independent pixel width. Two completely different concepts as the first may be thought of as a request in some cases (and the request should not be overwritten) and the second is actually what was possible.

Please see https://docs.microsoft.com/en-us/dotnet/framework/wpf/advanced/layout

There is a difference between the properties of Height and Width and ActualHeight and ActualWidth. For example, the ActualHeight property is a calculated value based on other height inputs and the layout system. The value is set by the layout system itself, based on an actual rendering pass, and may therefore lag slightly behind the set value of properties, such as Height, that are the basis of the input change.

Because ActualHeight is a calculated value, you should be aware that there could be multiple or incremental reported changes to it as a result of various operations by the layout system. The layout system may be calculating required measure space for child elements, constraints by the parent element, and so on.

Edit: I should also add that I've needed to distinguish between Width and ActualWidth in several cases in real-apps. It is useful for more advanced/dynamic layouts but maybe not for your case. Canvas is somewhat of an exception here though where everything is layout with fixed position and height/width -- since Canvas can render any FrameworkElement (and those FrameworkElement can be nested as above) it also gets the Width/ActualWidth distinction but it does have less meaning in the context of a Canvas.

Was this page helpful?
0 / 5 - 0 ratings