View Issue Details

IDProjectCategoryView StatusLast Update
0001769NoesisGUIC++ SDKpublic2020-08-28 23:03
ReporternikobarliAssigned Tojsantos 
PrioritynormalSeveritycrashReproducibilityalways
Status resolvedResolutionfixed 
Product Version3.0.4 
Target Version3.0.5Fixed in Version 
Summary0001769: Crash inside event handler
DescriptionWe are in the process of testing Noesis 3.0 with our codes.
Currently we are experiencing crashes inside our event handlers (which worked back in 2.2.6).
Debugging the problem reveals that something bad happen when inside a MouseDown handler, we are attaching another MouseDown handler.

# Internally there seems to be conversion from SingleDelegate to MultiDelegate ? Not sure ...

It's difficult for us to create a repro for this bug, but we happened to be able to avoid the crash with the attached Delegate.inl.patch.

Could you please check if the patch makes sense ? It seems that mVector->v may be replaces inside the handler, so it's not safe to use the expression for (Delegate& d: mVector->v) ?
TagsNo tags attached.
PlatformAny

Activities

nikobarli

nikobarli

2020-08-04 14:42

reporter  

Delegate.inl.patch (1,202 bytes)
Index: Src/Packages/Core/Kernel/Include/NsCore/Delegate.inl
===================================================================
--- Src/Packages/Core/Kernel/Include/NsCore/Delegate.inl	(revision 145902)
+++ Src/Packages/Core/Kernel/Include/NsCore/Delegate.inl	(working copy)
@@ -581,16 +581,24 @@
         {
             // Hold reference to the vector to avoid it being destructed in the iteration loop
             InvokerGuard guard(mVector);
+            const Delegates& v = mVector->v;
 
-            for (Delegate& d: mVector->v)
+            uint32_t numDelegates = v.Size();
+            for (uint32_t i = 0; i < numDelegates; ++i)
             {
-                if (abort(param))
-                {
-                    break;
-                }
+                if (abort(param)) break;
+                (v[i])(args...);
+            }
 
-                d(args...);
-            }
+            // for (Delegate& d: mVector->v)
+            // {
+            //     if (abort(param))
+            //     {
+            //         break;
+            //     }
+            // 
+            //     d(args...);
+            // }
         }
 
         void Copy(Impl* dest) const override
Delegate.inl.patch (1,202 bytes)
jsantos

jsantos

2020-08-05 11:17

manager   ~0006551

Hi Niko! I don't see how the proposed patch can be fix the problem because it is just using a reference, not copying the vector. And in fact, that vector cannot be destroyed because it is a ref counted object and one extra reference is being kept at the beginning of the method.

Are you sure this patch fixes your crash? Could you please double check?
nikobarli

nikobarli

2020-08-06 05:42

reporter   ~0006553

Hi Jesus,

Yes, I am pretty sure that the problem goes away with the patch. But, yes it may just be the symptom that went away, not the real bug.

I bundled two set of binaries here:

https://drive.google.com/drive/folders/1fdxot2XLzqIBg9y01k6k78c5cRQvgEo6?usp=sharing

Debug.NoPatch.zip is the binary with the original Noesis.
Debug.Patched.zip is the binary with the patched Noesis.

Please unzip the first one, and run NoesisTutorial.exe. When the window appears, click "Reactive Input" button. Then click anywhere inside the new window. The application will crash. I included the pdb files for Noesis.dll and NoesisApp.dll so that you can attach a debugger to the binary.
nikobarli

nikobarli

2020-08-06 06:30

reporter   ~0006554

Forgot to say that you'll need VS2019 runtime to use the binary (because it is compiled with VS2019 16.4.8)
jsantos

jsantos

2020-08-06 11:13

manager   ~0006556

Thanks for this Niko, having the PDBs will allow me to investigate further
jsantos

jsantos

2020-08-28 19:48

manager   ~0006598

Last edited: 2020-08-28 19:48

View 2 revisions

Hi Niko, I am playing with this right now. The font seems to be missing though, maybe you are using a japanese system font? Could you please attach it here?

Anyway, let's see if I can crash it trying all the sections. :)

jsantos

jsantos

2020-08-28 20:06

manager   ~0006599

Ok, I am being able to crash it, it is the 10th button from below.
jsantos

jsantos

2020-08-28 20:10

manager   ~0006600

I am getting the crash inside NoesisUtil.dll. Could you also provide the associated pdb?

     NoesisUtil.dll!00007ffa2501829f()	Unknown
     NoesisUtil.dll!00007ffa250225ab()	Unknown
     Noesis.dll!Noesis::Delegate<void __cdecl(Noesis::BaseComponent *,Noesis::RoutedEventArgs const &)>::Invoke(Noesis::BaseComponent * <args_0>, const Noesis::RoutedEventArgs & <args_1>, const void * param, bool(*)(const void *) abort) Line 185	C++
>	Noesis.dll!Noesis::UIElement::NotifyHandlers(const Noesis::RoutedEventArgs & args) Line 2633	C++
     Noesis.dll!Noesis::UIElement::BubblingEvent(const Noesis::RoutedEventArgs & args) Line 2685	C++
     Noesis.dll!Noesis::UIElement::RaiseEvent(const Noesis::RoutedEventArgs & args) Line 1162	C++
     Noesis.dll!Noesis::Mouse::RaiseButtonDownEvents(Noesis::UIElement * element, Noesis::MouseButton button, unsigned int clickCount) Line 332	C++
     Noesis.dll!Noesis::Mouse::ButtonDown(int x, int y, Noesis::MouseButton button) Line 364	C++
     Noesis.dll!Noesis::View::MouseButtonDown(int x, int y, Noesis::MouseButton button) Line 492	C++
jsantos

jsantos

2020-08-28 23:03

manager   ~0006601

Nevermind, your patch was good! The problem was with the range-based loop, it was not compatible with the vector memory being reallocated in the middle of the loop.

Thanks a lot for taking your time and proposing a patch

Issue History

Date Modified Username Field Change
2020-08-04 14:42 nikobarli New Issue
2020-08-04 14:42 nikobarli File Added: Delegate.inl.patch
2020-08-05 11:15 jsantos Assigned To => jsantos
2020-08-05 11:15 jsantos Status new => assigned
2020-08-05 11:15 jsantos Target Version => 3.0.5
2020-08-05 11:15 jsantos Description Updated View Revisions
2020-08-05 11:17 jsantos Note Added: 0006551
2020-08-05 11:17 jsantos Status assigned => feedback
2020-08-06 05:42 nikobarli Note Added: 0006553
2020-08-06 05:42 nikobarli Status feedback => assigned
2020-08-06 06:30 nikobarli Note Added: 0006554
2020-08-06 11:13 jsantos Note Added: 0006556
2020-08-28 19:48 jsantos Note Added: 0006598
2020-08-28 19:48 jsantos Status assigned => feedback
2020-08-28 19:48 jsantos Note Edited: 0006598 View Revisions
2020-08-28 20:06 jsantos Note Added: 0006599
2020-08-28 20:10 jsantos Note Added: 0006600
2020-08-28 23:03 jsantos Status feedback => resolved
2020-08-28 23:03 jsantos Resolution open => fixed
2020-08-28 23:03 jsantos Note Added: 0006601