View Issue Details

Jump to NotesJump to History
IDProjectCategoryView StatusDate SubmittedLast Update
0001644NoesisGUIC# SDKpublic2020-03-24 14:432020-04-03 16:24
Reporterai_enabledAssigned Tosfernandez 
PrioritynormalSeverityminorReproducibilityalways
Status resolvedResolutionfixed 
Product Version2.2.6 
Target Version3.0.0Fixed in Version3.0.0 
Summary0001644: Dispatcher issues
DescriptionHi guys,

first, this operation.Invoke() call https://github.com/Noesis/Managed/blob/782acc60bf2f2befbab0d701a3e3d5bd467859ab/Src/Noesis/Core/Src/Core/Dispatcher.cs#L137 will crash the process if there is an exception because it's not handled.
I suggest changing it to:
                    try
                    {
                        operation.Invoke();
                    }
                    catch (OperationCanceledException)
                    {
                        // that's fine
                    }
                    catch (Exception)
                    {
                        // TODO: add logging
                    }

(the issue happened to me)

Second, NoesisGUI C# SDK 2.2.6 setting up the synchronization context here https://github.com/Noesis/Managed/blob/ce47687f17cb6c66c93b1151154d906ff9cf8d23/Src/Noesis/Core/Src/Core/NoesisGUI.cs#L58
this is conflicting with our own "main thread" SynchronizationContext . I'm not sure if you actually intended to set your SynchronizationContext there. For my branch, I've just removed this line, but I think you need to pay extra attention as it might lead to unexpected consequences for your customers. You cannot make an assumption that the main thread (with its SynchronizationContext) completely belongs to NoesisGUI. Setting your SynchronizationContext temporary is fine but implicitly enforcing it is not a good idea from my experience.

Regards!
TagsNo tags attached.
PlatformAny

Activities

sfernandez

sfernandez

2020-03-25 12:03

manager   ~0006171

Yes, that was incorrect in 2.2.6 and we changed in 3.0, we no longer set the SynchronizationContext when NoesisGUI is initialized.

We will incorporate the exception catch block to the invoke call, although we will use the UnhandledException callback we expose in our API, because we don't handle the exceptions occurring in user code. I guess you already installed your own exception handler with Noesis.Error.SetUnhandledCallback(), right?

Thanks for the feedback.
ai_enabled

ai_enabled

2020-03-25 21:08

updater   ~0006172

Yes, my NoesisGUI wrapper for MonoGame is setting up the callback with Noesis.Error.SetUnhandledCallback()

Good to know you're already aware of this issue.
Regards!

Issue History

Date Modified Username Field Change
2020-03-24 14:43 ai_enabled New Issue
2020-03-24 14:44 ai_enabled Description Updated View Revisions
2020-03-24 14:44 ai_enabled Description Updated View Revisions
2020-03-25 12:03 sfernandez Assigned To => sfernandez
2020-03-25 12:03 sfernandez Status new => feedback
2020-03-25 12:03 sfernandez Note Added: 0006171
2020-03-25 21:08 ai_enabled Note Added: 0006172
2020-03-25 21:08 ai_enabled Status feedback => assigned
2020-04-03 16:24 sfernandez Status assigned => resolved
2020-04-03 16:24 sfernandez Resolution open => fixed
2020-04-03 16:24 sfernandez Fixed in Version => 3.0.0
2020-04-03 16:24 sfernandez Target Version => 3.0.0