dotMorten / WinUIEx

WinUI Extensions
https://dotmorten.github.io/WinUIEx
MIT License
615 stars 40 forks source link

WindowManager.Get(this) crashes on start #73

Closed adamplonka closed 1 year ago

adamplonka commented 2 years ago

This is the same issue as in https://github.com/microsoft/microsoft-ui-xaml/issues/6353 This code:

IntPtr hWnd = WindowNative.GetWindowHandle(this);
WindowId wndId = Microsoft.UI.Win32Interop.GetWindowIdFromWindow(hWnd);
var win = AppWindow.GetFromWindowId(wndId);

works in the (unpackaged) application, but using WindowEx or any method from WindowManager crashes with exception

The handle is invalid. (Exception from HRESULT: 0x80070006 (E_HANDLE)
   at WinRT.ExceptionHelpers.<ThrowExceptionForHR>g__Throw|20_0(Int32 hr)
   at ABI.Microsoft.UI.Windowing.IAppWindowStaticsMethods.GetFromWindowId(IObjectReference _obj, WindowId windowId)
   at WinUIEx.WindowExtensions.GetAppWindowFromWindowHandle(IntPtr hwnd)
   at WinUIEx.WindowManager..ctor(Window window, WindowMessageMonitor monitor)

It looks like AppWindow.GetFromWindowId(wndId) has a limited number of usages. Putting the above code in a loop results in an exception within couple of iterations. Could the AppWindow instance be cached inside of WindowManager instead of calling this code every time?

Steps to reproduce:

  1. Create an app using WinUIEx (you can use Template Studio for WinUI v5.2.1 - https://marketplace.visualstudio.com/items?itemName=TemplateStudio.TemplateStudioForWinUICs )
  2. Try to run and restart the (Unpackaged) version of application several times (it usually happens after around 30 debugging sessions after which it doesn't start no more without system restart)
dotMorten commented 2 years ago

Could the AppWindow instance be cached inside of WindowManager instead of calling this code every time? ... Try to run and restart the (Unpackaged) version of application several times...

I'm not quite following here. You can't cache it across restarts. It would be different each time.

adamplonka commented 2 years ago

No, not across restarts. Currently all calls to set MinWidth, MinHeight etc first get an instance of AppWindow but this instance is created every time using AppWindow.GetFromWindowId. I found this to be an issue because calls to this method randomly throw an exception in unpackaged mode. Usually it's fine to call this method once but after couple of calls it starts failing.

Oh and about restarts - once you reach the state where this method fails, it keeps failing even after application restart. Looks like it leaves some garbage in the system, I haven't yet figured what's really happening and the exact steps to reproduce, I just know it happens repetitively after some time, might be connected to windows sleep/hibernation mode (last time it happened to me was right after waking it from sleep). Apllication runs fine when not using any calls to AppWindow.

I don't think this is a bug in WinUiEx or that this library can do anything about it, looks like the bug is on much lower level, maybe in WinRT. Anyway WinUiEx can temporarily mitigate the issue by limiting calls to AppWindow.GetFromWindowId before it gets fixed.

dotMorten commented 2 years ago

This to me sounds more like a pretty serious issue the WinAppSDK needs to fix soon. Sure caching it could reduce the likelihood of this happening early on and buy you a little more time, but ultimately running a few WinAppSDK apps and you're in for a full windows restart. The linked issue above doesn't seem to convey the behavior you're describing. @rkarman is an upcoming fix for this coming? Sounds pretty serious.

adamplonka commented 1 year ago

Just letting know that this error stopped occurring after one of windows updates. I'm still a little concerned about backward compatibility but as I couldn't reproduce this bug for quite a long time, I'm closing this issue for now.