fix watchTree silent crash by pinzart90 · Pull Request #12975 · DynamoDS/Dynamo

pinzart90 · 2022-06-07T15:55:40Z

https://jira.autodesk.com/browse/DYN-5005

Purpose

Fix silent crash when opening up graph

Declarations

Check these if you believe they are true

The codebase is in a better state after this PR
Is documented according to the standards
The level of testing this PR includes is appropriate
User facing strings, if any, are extracted into *.resx files
All tests pass using the self-service CI.
Snapshot of UI changes, if any.
Changes to the API follow Semantic Versioning and are documented in the API Changes document.
This PR modifies some build requirements and the readme is updated

Release Notes

(FILL ME IN) Brief description of the fix / enhancement. Mandatory section

Reviewers

(FILL ME IN) Reviewer 1 (If possible, assign the Reviewer for the PR)

(FILL ME IN, optional) Any additional notes to reviewers or testers.

FYIs

(FILL ME IN, Optional) Names of anyone else you wish to be notified of

pinzart90 · 2022-06-07T16:02:51Z

A wpf control can trigger Loaded event multiple times (some without DataContext)

Here is the callstack caught with GFlags

KERNELBASE!RaiseException + 0x69   
clr!NakedThrowHelper2 + 0x9   
clr!NakedThrowHelper_RspAligned + 0x1e   
clr!NakedThrowHelper_FixRsp + 0x5   
DynamoCoreWpf!Dynamo.Controls.WatchTree.WatchTree_Loaded + 0x83   
PresentationCore_ni!System.Windows.EventRoute.InvokeHandlersImpl + 0xf2   
PresentationCore_ni!System.Windows.UIElement.RaiseEventImpl + 0x88   
PresentationFramework_ni!System.Windows.BroadcastEventHelper.BroadcastEvent + 0x190   
PresentationFramework_ni!System.Windows.BroadcastEventHelper.BroadcastLoadedEvent + 0xbd   
PresentationCore_ni!MS.Internal.LoadedOrUnloadedOperation.DoWork + 0x27   
PresentationCore_ni!System.Windows.Media.MediaContext.FireLoadedPendingCallbacks + 0x47   
PresentationCore_ni!System.Windows.Media.MediaContext.FireInvokeOnRenderCallbacks + 0x40   
PresentationCore_ni!System.Windows.Media.MediaContext.RenderMessageHandlerCore + 0xa2   
PresentationCore_ni!System.Windows.Media.MediaContext.RenderMessageHandler + 0x74   
PresentationCore_ni!System.Windows.Interop.HwndTarget.OnResize + 0x16c   
PresentationCore_ni!System.Windows.Interop.HwndTarget.HandleMessage + 0x2f4   
PresentationCore_ni!System.Windows.Interop.HwndSource.HwndTargetFilterMessage + 0x3f   
WindowsBase_ni!MS.Win32.HwndWrapper.WndProc + 0xc4   
WindowsBase_ni!MS.Win32.HwndSubclass.DispatcherCallbackOperation + 0x84   
WindowsBase_ni!System.Windows.Threading.ExceptionWrapper.InternalRealCall + 0x68   
WindowsBase_ni!System.Windows.Threading.ExceptionWrapper.TryCatchWhen + 0x36   
WindowsBase_ni!System.Windows.Threading.Dispatcher.LegacyInvokeImpl + 0x172   
WindowsBase_ni!MS.Win32.HwndSubclass.SubclassWndProc + 0x152   
WindowsBase_ni + 0x323e42   
clr!UMThunkStub + 0x6e   
user32!UserCallWinProcCheckWow + 0x2f8   
user32!DispatchClientMessage + 0x9c   
user32!_fnDWORD + 0x33   
ntdll!KiUserCallbackDispatcherContinue   
win32u!NtUserMessageCall + 0x14   
user32!SendMessageWorker + 0x823   
user32!RealDefWindowProcWorker + 0x2c3   
user32!RealDefWindowProcW + 0x4f   
uxtheme!_ThemeDefWindowProc + 0x609   
uxtheme!ThemeDefWindowProcW + 0x11   
user32!DefWindowProcW + 0x1a3   
user32!UserCallWinProcCheckWow + 0x2f8   
user32!CallWindowProcW + 0x8e   
WindowsBase_ni!MS.Win32.HwndSubclass.DefWndProcWrapper + 0x97   
WindowsBase_ni + 0x323e42   
clr!UMThunkStub + 0x6e   
user32!UserCallWinProcCheckWow + 0x2f8   
user32!CallWindowProcW + 0x8e   
WindowsBase_ni!MS.Win32.HwndSubclass.SubclassWndProc + 0x1e2   
WindowsBase_ni + 0x323e42   
clr!UMThunkStub + 0x6e   
user32!UserCallWinProcCheckWow + 0x2f8   
user32!DispatchClientMessage + 0x9c   
user32!_fnINLPWINDOWPOS + 0x30   
ntdll!KiUserCallbackDispatcherContinue   
win32u!NtUserMsgWaitForMultipleObjectsEx + 0x14   
user32!RealMsgWaitForMultipleObjectsEx + 0x1e   
combase!CCliModalLoop::BlockFn + 0x18d   
combase!ClassicSTAThreadWaitForHandles + 0xa5   
combase!CoWaitForMultipleHandles + 0x76   
clr!MsgWaitHelper + 0x8e   
clr!Thread::DoAppropriateWaitWorker + 0x2e6   
clr!Thread::DoAppropriateWait + 0x7d   
clr!SynchronizationContextNative::WaitHelper + 0x181   
WindowsBase_ni!System.Windows.Threading.DispatcherSynchronizationContext.Wait + 0x2a   
clr!CallDescrWorkerInternal + 0x83   
clr!CallDescrWorkerWithHandler + 0x4e   
clr!MethodDescCallSite::CallTargetWorker + 0xfa   
clr!Thread::DoSyncContextWait + 0xc0   
clr!Thread::DoAppropriateWaitWorker + 0x1f9ac5   
clr!Thread::DoAppropriateWait + 0x7d   
clr!WaitHandleNative::CorWaitOneNative + 0x161   
mscorlib_ni!System.Threading.WaitHandle.InternalWaitOne + 0x1c   
mscorlib_ni!System.Threading.WaitHandle.WaitOne + 0x2f   
System_ni!System.Net.FileWebRequest.GetResponse + 0xa7   
PresentationCore_ni!MS.Internal.WpfWebRequestHelper.GetResponse + 0x16   
PresentationFramework_ni!System.Windows.ResourceDictionary.set_Source + 0x140   
System_Xaml_ni!MS.Internal.Xaml.Runtime.ClrObjectRuntime.SetValue + 0x3b   
System_Xaml_ni!MS.Internal.Xaml.Runtime.PartialTrustTolerantRuntime.SetValue + 0x46   
System_Xaml_ni!System.Xaml.XamlObjectWriter.Logic_ApplyPropertyValue + 0x9a   
System_Xaml_ni!System.Xaml.XamlObjectWriter.Logic_DoAssignmentToParentProperty + 0x177   
System_Xaml_ni!System.Xaml.XamlObjectWriter.Logic_AssignProvidedValue + 0x47   
System_Xaml_ni!System.Xaml.XamlObjectWriter.WriteEndMember + 0x2d5   
PresentationFramework_ni!System.Windows.Markup.WpfXamlLoader.TransformNodes + 0x38f   
PresentationFramework_ni!System.Windows.Markup.WpfXamlLoader.Load + 0x262   
PresentationFramework_ni!System.Windows.Markup.WpfXamlLoader.LoadBaml + 0x60   
PresentationFramework_ni!System.Windows.Markup.XamlReader.LoadBaml + 0x249   
0x7ff7a22269c5   
0x7ff7a222632c   
DynamoCoreWpf!Dynamo.Controls.NodeView.TogglePreviewControlAllowance + 0x34   
0x7ff7a222582d   
0x7ff5a105db11   
0x7ff7a21eb6bc   
0x7ff7a21eb623   
DynamoCoreWpf!Dynamo.Controls.DynamoView.ApplyNodeViewCustomization + 0x2d   
DynamoCoreWpf!Dynamo.ViewModels.DynamoViewModel.OnNodeViewReady + 0x3c   
PresentationCore_ni!System.Windows.EventRoute.InvokeHandlersImpl + 0xf2   
PresentationCore_ni!System.Windows.UIElement.RaiseEventImpl + 0x88   
PresentationFramework_ni!System.Windows.BroadcastEventHelper.BroadcastEvent + 0x190   
PresentationFramework_ni!System.Windows.BroadcastEventHelper.BroadcastLoadedEvent + 0xbd   
PresentationCore_ni!MS.Internal.LoadedOrUnloadedOperation.DoWork + 0x27   
PresentationCore_ni!System.Windows.Media.MediaContext.FireLoadedPendingCallbacks + 0x47   
PresentationCore_ni!System.Windows.Media.MediaContext.FireInvokeOnRenderCallbacks + 0x40   
PresentationCore_ni!System.Windows.Media.MediaContext.RenderMessageHandlerCore + 0xa2   
PresentationCore_ni!System.Windows.Media.MediaContext.RenderMessageHandler + 0x74

pinzart90 · 2022-06-07T16:09:25Z

I still did not figure out why the exception is not caught by sandbox...
It does look like the main thread to me ..

pinzart90 · 2022-06-07T16:16:47Z

src/DynamoCoreWpf/Views/Preview/WatchTree.xaml.cs

        private readonly int minHeightSize = 38;

-        public WatchTree()
+        public WatchTree(WatchViewModel vm)


this should be ok to break right ?

mjkkirschner · 2022-06-07T16:25:39Z

@pinzart90 what is the actual exception? Is something null?
I imagine the exception may not be caught because the view is not done being loaded so the request handler for the crash dialog may not be attached yet?

pinzart90 · 2022-06-08T04:13:13Z

@pinzart90 what is the actual exception? Is something null? I imagine the exception may not be caught because the view is not done being loaded so the request handler for the crash dialog may not be attached yet?

The _vm (WatchViewModel) is null

You might be right about the crash handler...

aparajit-pratap

Just curious, what is the tool you used to debug this (screenshot in the PR description)? Can the exception be found while debugging in VS?

aparajit-pratap · 2022-06-08T15:30:31Z

src/Libraries/CoreNodeModelsWpf/NodeViewCustomizations/Watch.cs

            this.watch = nodeModel;
            this.syncContext = new DispatcherSynchronizationContext(nodeView.Dispatcher);

-            var watchTree = new WatchTree();


Is any of this code new to 2.15 that causes this to fail only in 2.15?

Hard to tell by looking at the code...multiple things can contribute to this bug
The critical area (IMO) is the Loaded event ...that assumes a DataContext exists. That part of the code has been there for a while.

The loaded event can be triggered multiple times with/without data context. Looks like at least in 2.15...a Loaded event is triggered without data context (when opening the .dyn attached to the jira task)
This seems to be acceptable (from what I have read) but we should not always assume the context exists.

pinzart90 · 2022-06-08T15:43:41Z

Just curious, what is the tool you used to debug this (screenshot in the PR description)? Can the exception be found while debugging in VS?

The exception does not trigger when the debugger is attached.
I used GFlags to capture the stack when the process is killed by the exception

aparajit-pratap · 2022-06-08T16:21:35Z

LGTM!

pinzart90 · 2022-06-13T14:57:40Z

I removed the new unit test because it did not reproduce the issue (without the code fix).
Also..seems like I cannot reproduce this issue on master anymore (probably changes in the UI have masked this issue).
I still say it is a good idea to do this change.

mjkkirschner · 2022-06-13T16:19:36Z

let's not merge this until we remove the 3 megabyte file which no longer reproduces the bug - I will remove it.

mjkkirschner · 2022-06-13T20:05:15Z

I also can't reproduce this with either test file, in my tests it seems watchTree_loaded is now only called once, but the changes make sense to me and seem easier to reason about than what was happening previously.

I'm going to merge this and monitor for any regressions.

pinzart90 added 2 commits June 7, 2022 11:41

fix watchTree silent crash

6ea8815

Update WatchTree.xaml.cs

d8d3402

pinzart90 marked this pull request as ready for review June 7, 2022 16:07

pinzart90 requested a review from mjkkirschner June 7, 2022 16:07

pinzart90 assigned aparajit-pratap Jun 7, 2022

pinzart90 requested a review from QilongTang June 7, 2022 16:07

pinzart90 commented Jun 7, 2022

View reviewed changes

pinzart90 requested a review from aparajit-pratap June 8, 2022 14:38

aparajit-pratap approved these changes Jun 8, 2022

View reviewed changes

aparajit-pratap reviewed Jun 8, 2022

View reviewed changes

add test

f49b66a

pinzart90 requested a review from aparajit-pratap June 8, 2022 17:35

aparajit-pratap approved these changes Jun 9, 2022

View reviewed changes

pinzart90 added 3 commits June 9, 2022 14:23

Update DynamoViewTests.cs

56729bb

Update DynamoViewTests.cs

d9570b5

remove test

5d485d1

pinzart90 requested a review from aparajit-pratap June 13, 2022 14:57

Delete BiggestRectangle.dyn

e7cff2a

mjkkirschner merged commit 11c560e into master Jun 13, 2022

mjkkirschner deleted the silent_crash branch June 13, 2022 20:06

Conversation

pinzart90 commented Jun 7, 2022 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Purpose

Declarations

Release Notes

Reviewers

FYIs

Uh oh!

pinzart90 commented Jun 7, 2022

Uh oh!

pinzart90 commented Jun 7, 2022

Uh oh!

pinzart90 Jun 7, 2022

Choose a reason for hiding this comment

Uh oh!

mjkkirschner Jun 7, 2022

Choose a reason for hiding this comment

Uh oh!

mjkkirschner commented Jun 7, 2022

Uh oh!

pinzart90 commented Jun 8, 2022

Uh oh!

aparajit-pratap left a comment • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

aparajit-pratap Jun 8, 2022

Choose a reason for hiding this comment

Uh oh!

pinzart90 Jun 8, 2022

Choose a reason for hiding this comment

Uh oh!

pinzart90 commented Jun 8, 2022

Uh oh!

aparajit-pratap commented Jun 8, 2022

Uh oh!

pinzart90 commented Jun 13, 2022

Uh oh!

mjkkirschner commented Jun 13, 2022

Uh oh!

mjkkirschner commented Jun 13, 2022 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

pinzart90 commented Jun 7, 2022 •

edited

Loading

aparajit-pratap left a comment •

edited

Loading

mjkkirschner commented Jun 13, 2022 •

edited

Loading