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

Access Violation App Crash #954

Open
aleibhardt opened this issue Apr 3, 2024 · 1 comment
Open

Access Violation App Crash #954

aleibhardt opened this issue Apr 3, 2024 · 1 comment

Comments

@aleibhardt
Copy link

I have a crash in my UWP application that is hitting when i switch between portrait and landscape mode.
I haven't been able to find a minimal way to reproduce this in a sample app yet.

I have been able to get the crash under a debugger though.
Here is the stack trace that I get:

00 0000009b`8f8fe0b0 00007ff8`0d0680a7     MSVCP140_APP!mtx_do_lock+0x9c [d:\a01\_work\3\s\src\vctools\crt\github\stl\src\mutex.cpp @ 103] 
01 (Inline Function) --------`--------     Microsoft_Graphics_Canvas_7ff80d000000!std::_Mutex_base::lock+0x9 [C:\Program Files\Microsoft Visual Studio\2022\Enterprise\VC\Tools\MSVC\14.37.32822\include\mutex @ 76] 
02 (Inline Function) --------`--------     Microsoft_Graphics_Canvas_7ff80d000000!std::unique_lock<std::mutex>::{ctor}+0x12 [C:\Program Files\Microsoft Visual Studio\2022\Enterprise\VC\Tools\MSVC\14.37.32822\include\mutex @ 179] 
03 (Inline Function) --------`--------     Microsoft_Graphics_Canvas_7ff80d000000!ABI::Microsoft::Graphics::Canvas::UI::Xaml::BaseControl<ABI::Microsoft::Graphics::Canvas::UI::Xaml::CanvasControlTraits>::GetLock+0x19 [D:\a\1\s\winrt\lib\xaml\BaseControl.h @ 648] 
04 (Inline Function) --------`--------     Microsoft_Graphics_Canvas_7ff80d000000!ABI::Microsoft::Graphics::Canvas::UI::Xaml::BaseControl<ABI::Microsoft::Graphics::Canvas::UI::Xaml::CanvasControlTraits>::OnXamlRootChanged::__l2::<lambda_4a8ce6f4bf424589ccf5c976a9930e6f>::operator()+0x1c [D:\a\1\s\winrt\lib\xaml\BaseControl.h @ 1010] 
05 0000009b`8f8fe110 00007ff8`0d067513     Microsoft_Graphics_Canvas_7ff80d000000!ExceptionBoundary<<lambda_4a8ce6f4bf424589ccf5c976a9930e6f> >+0x4b [D:\a\1\s\winrt\inc\ErrorHandling.h @ 221] 
06 0000009b`8f8fe160 00007ff8`0d05b7e9     Microsoft_Graphics_Canvas_7ff80d000000!ABI::Microsoft::Graphics::Canvas::UI::Xaml::BaseControl<ABI::Microsoft::Graphics::Canvas::UI::Xaml::CanvasControlTraits>::OnXamlRootChanged+0x13 [D:\a\1\s\winrt\lib\xaml\BaseControl.h @ 1023] 
07 (Inline Function) --------`--------     Microsoft_Graphics_Canvas_7ff80d000000!Microsoft::WRL::Callback::__l2::<lambda_9fbdbe06eca3691e643510eeee9cd40f>::operator()+0x12 [C:\Program Files (x86)\Windows Kits\10\Include\10.0.22621.0\winrt\wrl\event.h @ 479] 
08 0000009b`8f8fe190 00007ff8`72af8502     Microsoft_Graphics_Canvas_7ff80d000000!Microsoft::WRL::Details::DelegateArgTraits<long (__cdecl ABI::Windows::Foundation::IEventHandler_impl<ABI::Windows::Foundation::Internal::AggregateType<ABI::Windows::ApplicationModel::SuspendingEventArgs *,ABI::Windows::ApplicationModel::ISuspendingEventArgs *> >::*)(IInspectable *,ABI::Windows::ApplicationModel::ISuspendingEventArgs *)>::DelegateInvokeHelper<ABI::Windows::Foundation::IEventHandler<ABI::Windows::ApplicationModel::SuspendingEventArgs *>,<lambda_9fbdbe06eca3691e643510eeee9cd40f>,1,IInspectable *,ABI::Windows::ApplicationModel::ISuspendingEventArgs *>::Invoke+0x19 [C:\Program Files (x86)\Windows Kits\10\Include\10.0.22621.0\winrt\wrl\event.h @ 354] 
09 0000009b`8f8fe1c0 00007ff8`72623dd5     Windows_UI_Xaml!DirectUI::CEventSourceBase<DirectUI::IUntypedEventSource,Windows::Foundation::ITypedEventHandler<Windows::UI::Xaml::XamlRoot *,Windows::UI::Xaml::XamlRootChangedEventArgs *>,Windows::UI::Xaml::IXamlRoot,Windows::UI::Xaml::IXamlRootChangedEventArgs>::Raise+0x4d464e [onecoreuap\windows\dxaml\xcp\dxaml\lib\JoltClasses.h @ 307] 
0a 0000009b`8f8fe230 00007ff8`72623ca5     Windows_UI_Xaml!DirectUI::XamlRoot::RaiseChangedEvent+0x81 [onecoreuap\windows\dxaml\xcp\dxaml\lib\xamlroot_partial.cpp @ 76] 
0b 0000009b`8f8fe270 00007ff8`726f9343     Windows_UI_Xaml!CFxCallbacks::XamlRoot_RaiseChanged+0x21 [onecoreuap\windows\dxaml\xcp\dxaml\lib\fxcallbacks.cpp @ 1610] 
0c 0000009b`8f8fe2a0 00007ff8`72762709     Windows_UI_Xaml!CContentRoot::RaisePendingXamlRootChangedEventIfNeeded+0x9b [onecoreuap\windows\dxaml\xcp\components\contentroot\contentroot.cpp @ 319] 
0d (Inline Function) --------`--------     Windows_UI_Xaml!CContentRoot::RaiseXamlRootChanged+0xf [onecoreuap\windows\dxaml\xcp\components\contentroot\contentroot.cpp @ 309] 
0e 0000009b`8f8fe2e0 00007ff8`72763bae     Windows_UI_Xaml!CCoreServices::OnWindowSizeChanged+0x55 [onecoreuap\windows\dxaml\xcp\core\dll\xcpcore.cpp @ 5439] 
0f 0000009b`8f8fe330 00007ff8`72892f0f     Windows_UI_Xaml!DirectUI::Window::OnWindowSizeChanged+0xde [onecoreuap\windows\dxaml\xcp\dxaml\lib\window_partial.cpp @ 1100] 
10 0000009b`8f8fe3b0 00007ff8`725fe035     Windows_UI_Xaml!DirectUI::Window::OnCoreWindowLayoutBoundsChanged+0x6f [onecoreuap\windows\dxaml\xcp\dxaml\lib\window_partial.cpp @ 1228] 
11 (Inline Function) --------`--------     Windows_UI_Xaml!Microsoft::WRL::Callback::__l2::<lambda_26927f60691664ba82371d86effe030e>::operator()+0x1f [onecore\external\sdk\inc\wrl\event.h @ 479] 
12 0000009b`8f8fe400 00007ff8`73c8a8fd     Windows_UI_Xaml!Microsoft::WRL::Details::DelegateArgTraits<long (__cdecl Windows::Foundation::ITypedEventHandler_impl<Windows::Foundation::Internal::AggregateType<Windows::UI::Core::CoreWindow *,Windows::UI::Core::ICoreWindow *>,IInspectable *>::*)(Windows::UI::Core::ICoreWindow *,IInspectable *)>::DelegateInvokeHelper<Windows::Foundation::ITypedEventHandler<Windows::UI::Core::CoreWindow *,IInspectable *>,<lambda_26927f60691664ba82371d86effe030e>,-1,Windows::UI::Core::ICoreWindow *,IInspectable *>::Invoke+0x25 [onecore\external\sdk\inc\wrl\event.h @ 354] 
13 0000009b`8f8fe430 00007ff8`73c8916e     Windows_UI!Microsoft::WRL::InvokeTraits<-2>::InvokeDelegates<<lambda_d10e6de357b49d61c68f2108b804a6c5>,Windows::Foundation::ITypedEventHandler<Windows::UI::Core::CoreWindow *,IInspectable *> >+0x9d
14 0000009b`8f8fe490 00007ff8`73c890c8     Windows_UI!Microsoft::WRL::EventSource<Windows::Foundation::ITypedEventHandler<Windows::UI::Core::CoreWindow * __ptr64,IInspectable * __ptr64>,Microsoft::WRL::InvokeModeOptions<-2> >::DoInvoke<<lambda_d10e6de357b49d61c68f2108b804a6c5> >+0x92
15 0000009b`8f8fe4f0 00007ff8`73c88fc5     Windows_UI!Windows::UI::Core::WindowServer::OnLayoutBoundsChange+0x4c
16 0000009b`8f8fe530 00007ff8`73c88f44     Windows_UI!Windows::UI::Core::WindowServer::OnVisibleBoundsChangeEvent+0x65
17 0000009b`8f8fe6f0 00007ff8`73c96451     Windows_UI!Windows::UI::Core::WindowServer::OnVisibleBoundsChange+0x44
18 0000009b`8f8fe720 00007ff8`73c94c95     Windows_UI!Windows::UI::Core::WindowServer::OnVisibleBoundsChanged+0x21
19 0000009b`8f8fe750 00007ff8`7e99354b     Windows_UI!Microsoft::WRL::Details::DelegateArgTraits<long (__cdecl Windows::Foundation::IEventHandler_impl<IInspectable * __ptr64>::*)(IInspectable * __ptr64,IInspectable * __ptr64) __ptr64>::DelegateInvokeHelper<Windows::Foundation::IEventHandler<IInspectable * __ptr64>,<lambda_db1e4b56a38545873f6e01e1fbba6b19>,-1,IInspectable * __ptr64,IInspectable * __ptr64>::Invoke+0x25
1a 0000009b`8f8fe780 00007ff8`7e905470     CoreUIComponents!Microsoft::WRL::InvokeTraits<-2>::InvokeDelegates<<lambda_b9c6a9526e98e8096438348dee47a029>,Windows::Foundation::IEventHandler<IInspectable * __ptr64> >+0x6b
1b 0000009b`8f8fe7e0 00007ff8`7e905101     CoreUIComponents!Microsoft::WRL::EventSource<Windows::Foundation::IEventHandler<IInspectable * __ptr64>,Microsoft::WRL::InvokeModeOptions<-2> >::InvokeAll<std::nullptr_t,std::nullptr_t>+0x7c
1c 0000009b`8f8fe830 00007ff8`7e904a73     CoreUIComponents!Windows::Phone::UI::Core::NavigationClient::NotifyVisibleBoundsChangeToCoreWindow+0xb9
1d 0000009b`8f8fe870 00007ff8`7e95654e     CoreUIComponents!Windows::Phone::UI::Core::NavigationClient::PropertyChanged+0x463
...

The crash is an access violation. I'm also seeing a large number of reports from partner center for similar crashes.

Any idea if this is an issue in win2D?
I figured i'm doing something wrong, but the level of information i'm getting here is very minimal.
Any advice on what I can do to debug this would be greatly appreciated?

The app is a UWP app. I've tested on v 1.26 and v1.27.1 of win2d, and both versions show the crash.
Our app targets 1903 version of windows.
Crash happens on windows 10, and windows 11 devices.

I can reproduce this somewhat reliably (but not 100%). I am able to get a crash dump if that is useful.

@ZodmanPerth
Copy link

I've found replication steps that work for me 100% of the time. I've investigated the issue and found the root cause - there is a logic error in the way Win2D handles the attach/detach of event handlers of a BaseControl (e.g. CanvasControl) that allows event handlers to remain attached after a BaseControl has been finalised. When the event fires it causes the access violation.

High-level explanation

Win2D attaches/detaches event handlers to a BaseControl as it is loaded and unloaded from the visual tree. Unfortunately the UWP loaded and unloaded events could be recieved by Win2D out of order, making this attach/detach process difficult. Win2D compensates by using a simple counter to match received loaded/unloaded pairs. However it does not accommodate all combinations of how loaded and unloaded can be fired and this logic fails, resulting in event handlers still being attached to BaseControls that have been destoyed (finalised). When this happens, the UWP app can crash as soon as the user performs simple actions such as moving or resizing the window (if the BaseControl's XamlRoot is responding to window resize events for example), as it ends up handling events on a destroyed BaseControl.

Detailed discovery

Our app renders document pages as a number of tiled segments where each segment uses a CanvasControl. In some layouts, we put one or two pages inside a UWP FlipView, and have at most 3 Pages in the visual tree at a time. As the user navigates back and forward through pages, or changes layouts, old segments (i.e. CanvasControls) are torn down and new segments (i.e. CanvasControls) are created and rendered.

I took a fork of the Win2D library and hooked it up to our app. In my local fork:

  • I added a static id counter so that as each BaseControl is created it gets assigned a unique id number.
  • I instrumented some key events of BaseControl.h using OutputDebugString to be shown in the Visual Studio Output Window:
    • OnLoaded start and end
    • OnUnloaded start and end
    • Unregister and Register of event handlers
    • OnXamlRootChanged start and end
    • Destruction/Finalisation
  • I also intrumented when the m_isLoaded changes value

In our app I did the same:

  • Added a static id counter so that as each segment is created it gets a unique id. These numbers will be in sync with those id's being reported from Win2D.
  • I instrumented some key events:
    • segment creation
    • attaching the segment to the visual tree
    • disposing the segment, which also detaches the segment from the visual tree

With the instumentation in place I replicated the error and grabbed all the text in Visual Studio's Output Window. I'm using TextAnalysisTool.NET to work with the output, applying filters and colours.

Here's a screen capture showing the series of events leading up to one of the crashes due to an access violation:

Image

Some notes about this image:

  • lines marked with "#####" have originated from the UWP app
  • lines marked with "@@@@@" have originated from the Win2D fork
  • each line from Win2D shows the thread id producing it.
    • thread 64608 is the app's UI thread
    • thread 19044 is the app's garbage collector thread
  • each line is marked with the id of the BaseControl/segment it relates to
  • the value of m_isLoaded is output when the value changes
  • each line from Win2D shows the current value of m_loadedCount for that id (at the end of the line)

In this image I've filtered out everything but id 4, which is where the app crashes. Here's an account of what we're seeing here:

  • the app creates segment 4 and it is attached to the visual tree (lines 1002 and 1003)
  • Win2D processes a Loaded event on lines 1019 to 1023. This includes the unregister/reregister of the events, and in this case sets m_isLoaded to true.
  • lines 1072 to 1081 show Win2D processing changes to properties on the XAML root element, which can thought of as window resize events throughout this run.
  • Win2D processes an Unloaded event on lines 1093 to 1096. This includes setting m_isLoaded to false and deregistering event handlers.
  • Win2D processes another Unloaded event on lines 1126 and 1127. We expect loaded and unloaded events to happen in pairs in any order, however in this case UWP has only sent an unloaded event for some reason. This is not handled by Win2D and we end up in a bad state.

At this point in the log it's worth mentioning the m_loadedCount that is logged at the end of each line. This is the mechanism Win2D is using to match loaded and unloaded event pairs to ensure we only register/unregister event handlers when and only when we need to. It's possible for two loaded events to be followed by two unloaded events for example, so the count will go from 0->1, then 1->2, then 2->1, then 1->0. There is a comment in the Win2D code that discusses this inside the OnLoaded event handler:

// OnLoaded and OnUnloaded are fired from XAML asynchronously, and unfortunately they could
// be out of order. If the element is removed from tree A and added to tree B, we could get
// a Loaded event for tree B *before* we see the Unloaded event for tree A. To handle this:
//  * When we see a Loaded event when we're already loaded, just unregister event handlers
//      for the old tree and register for the new one.
//  * When we get an Unloaded event, check the loaded count. If the element has already been
//      added to another tree, the load count will be nonzero. In that case, we don't update
//      anything because the OnLoaded call has already taken care of things.

However as we've seen so far in this log, UWP has fired the unloaded event without a matching loaded event, putting the counter in a negative value. This is OK for the moment as we can see the event handlers are currently unregistered (line 1094) meaning the event handlers are not attached, but we will see soon that there is a problem with the app in the current state. In the image I've highlighed the app being in a good state with a blue or green background highlight, and the app being in a bad state where the access violation can occur with a red background highlight.

Let's continue the walkthrough. On lines 1138 to 1232 we see an out of order pair of loaded/unloaded events arrive as the segment is added back into the visual tree. However because the BaseControl is in a bad state, m_isLoaded is not set to true when OnLoaded is handled. This could cause issues as execution continues but that's not important for the issue we're talking about here. What we must note is that the event handlers are registed again (line 1231).

Lines 1252 to 1327 show more Window resizing (XAML root property changes).

Now things get interesting. Line 1409 shows the UWP app removes the segment from the visual tree and disposes it. Win2D detects the unload event on lines 1413 and 1414, but fails to deregister the event handler (because of the bad state). The app is now in a precarious state, waiting for the segment to be finalised (destructed) by the garbage collector.

Lines 1460 to 1535 show more Window resizing (XAML root property changes).

Line 1751 (yellow background) shows the garbage collector has kicked in and finalised the segment with id 4 that still has the event handlers attached. The app is now a ticking time bomb - if any of those event fires we could get an access violation.

Line 1913 shows the Window resizing (XAML root property changes). The attached event handler fires, and Win2D attempts to get a lock to check the visibility of the destroyed object, causing an access violation and crashing the app.

Important

I've investigated to see if there's anything I can do in our app code to prevent loaded/unloaded events firing out of order, or from firing the unloaded event without a matching loaded event. There is nothing app developer's can do to prevent this, so the suggestions from Win2D on avoiding memory leaks do not apply.
UWP app developers that encounter this access violation issue need to fix the problem inside Win2D.

Fixing Win2D

There are 34 combinations of loaded/unloaded events (and the occasional unloaded event on it's own) that Win2D must deal with to prevent holding a registered event handler on a finalised object. Currently Win2D is only dealing with 14 of them. I won't put the details of these scenarios here, but have written tests that document and deal with them.

Fortunately there is a very simple fix for Win2D to accommodate all possible combinations, by changing 2 lines of code inside the OnUnloaded event handler like this:

... 

if (--m_loadedCount < 1)
{
    auto lock = GetLock();
    m_isLoaded = false;
    m_loadedCount = 0;      // we're unloaded now - the loaded count doesn't need to fall below zero
    lock.unlock();
        
    Unloaded();
    UnregisterEventHandlers();
}

... 

Contributing to the UWP branch

I would prefer that our app (and other UWP apps) use the official Win2D nuget package instead of maintaining private forks with the patched code. I'm happy to create PR including the fix and the unit tests that exercise it.

If I contribute such a PR will Microsoft review the change and follow up with a release of Win2D for UWP containing the fix? Or has support for Win2D on UWP officially been dropped?

CC @shawnhar @Sergio0694

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

No branches or pull requests

2 participants