[NUI][API11] Remove DynamicProperty callback if URL changed + Make more thread safe...

Let us remove DynamicProperty callbacks if URL changes.
So far, we discard the callbacks if visual was changed.
But, the callbacks, which NUI stored, were not removed and remained alive
whenever we changed the URL.

Let us ensure their removal when URL changed (~= Visual Changed)

+

To avoid race condition, let we add lock feature before change `InternalSavedDynamicPropertyCallbacks`
(Since we can access this dictionary from various threads.) and make `weakReferencesOfLottie` as `ConcurrentDictionary` type.

Note : Since we need to iterate by `InternalSavedDynamicPropertyCallbacks.Keys` we need to use lock,
instead of `ConcurrentDictionary`.

TODO : There are some cases where we don't want to destroy registered callbacks. They will remain active in the future.
We make relative sample at `LottieAnimationViewDynamicPropertyTest.cs`. Let we pass `Test4` case show dynmaic property well.

Signed-off-by: Eunki, Hong <eunkiki.hong@samsung.com>

diff --git a/src/Tizen.NUI/src/public/BaseComponents/LottieAnimationView.cs b/src/Tizen.NUI/src/public/BaseComponents/LottieAnimationView.cs
index 3466e8afe0cc4f711b7436cbe01b7e23de30eb66..d5fbc457a4fbaa796fbc843a85fd4679dc322bbb 100755 (executable)
--- a/src/Tizen.NUI/src/public/BaseComponents/LottieAnimationView.cs
+++ b/src/Tizen.NUI/src/public/BaseComponents/LottieAnimationView.cs
@@ -19,6 +19,7 @@ using global::System;
 using global::System.Runtime.InteropServices;
 using System.ComponentModel;
 using System.Collections.Generic;
+using System.Collections.Concurrent;
 using System.Globalization;
 using Tizen.NUI;
 using Tizen.NUI.Binding;
@@ -106,11 +107,13 @@ namespace Tizen.NUI.BaseComponents
                 return;
             }
 
-            CleanCallbackDictionaries();
-
-            //Release your own unmanaged resources here.
-            //You should not access any managed member here except static instance.
-            //because the execution order of Finalizes is non-deterministic.
+            if (type == DisposeTypes.Explicit)
+            {
+                //Release your own unmanaged resources here.
+                //You should not access any managed member here except static instance.
+                //because the execution order of Finalizes is non-deterministic.
+                CleanCallbackDictionaries(true);
+            }
 
             //disconnect event signal
             if (finishedEventHandler != null && visualEventSignalCallback != null)
@@ -128,8 +131,11 @@ namespace Tizen.NUI.BaseComponents
         [EditorBrowsable(EditorBrowsableState.Never)]
         protected override void Dispose(bool disposing)
         {
-            // Note : We can clean dictionaries even this API called from GC Thread.
-            CleanCallbackDictionaries();
+            if (!disposing)
+            {
+                // Note : We can clean dictionaries even this API called from GC Thread.
+                CleanCallbackDictionaries(true);
+            }
             base.Dispose(disposing);
         }
         #endregion Constructor, Destructor, Dispose
@@ -187,6 +193,9 @@ namespace Tizen.NUI.BaseComponents
         {
             set
             {
+                // Invalidate previous dynamic property callbacks.
+                CleanCallbackDictionaries(false);
+
                 // Reset cached infomations.
                 currentStates.contentInfo = null;
                 currentStates.markerInfo = null;
@@ -976,30 +985,53 @@ namespace Tizen.NUI.BaseComponents
         [EditorBrowsable(EditorBrowsableState.Never)]
         public void DoActionExtension(LottieAnimationViewDynamicProperty info)
         {
+            // Called from main thread
             dynamicPropertyCallbackId++;
 
-            weakReferencesOfLottie?.Add(dynamicPropertyCallbackId, new WeakReference<LottieAnimationView>(this));
-            InternalSavedDynamicPropertyCallbacks?.Add(dynamicPropertyCallbackId, info.Callback);
+            lock (InternalPropertyCallbacksLock)
+            {
+                // Add to dictionary only if we can assume that this view is not in disposing state.
+                if (InternalSavedDynamicPropertyCallbacks != null)
+                {
+                    weakReferencesOfLottie?.TryAdd(dynamicPropertyCallbackId, new WeakReference<LottieAnimationView>(this));
+
+                    InternalSavedDynamicPropertyCallbacks.Add(dynamicPropertyCallbackId, info.Callback);
+                    NUILog.Debug($"<[{GetId()}] added extension actions for {dynamicPropertyCallbackId} (total : {weakReferencesOfLottie?.Count} my : {InternalSavedDynamicPropertyCallbacks?.Count})>");
+                }
+            }
 
             Interop.View.DoActionExtension(SwigCPtr, ImageView.Property.IMAGE, ActionSetDynamicProperty, dynamicPropertyCallbackId, info.KeyPath, (int)info.Property, Marshal.GetFunctionPointerForDelegate<System.Delegate>(rootCallback));
 
             if (NDalicPINVOKE.SWIGPendingException.Pending) throw NDalicPINVOKE.SWIGPendingException.Retrieve();
         }
 
-        private void CleanCallbackDictionaries()
+        private void CleanCallbackDictionaries(bool disposing)
         {
-            if (weakReferencesOfLottie?.Count > 0 && InternalSavedDynamicPropertyCallbacks != null)
+            // Called from main, or GC threads
+            lock (InternalPropertyCallbacksLock)
             {
-                foreach (var key in InternalSavedDynamicPropertyCallbacks?.Keys)
+                NUILog.Debug($"<[{GetId()}] remove extension actions with disposing:{disposing} (total : {weakReferencesOfLottie?.Count} my : {InternalSavedDynamicPropertyCallbacks?.Count})>");
+                if (weakReferencesOfLottie?.Count > 0 && InternalSavedDynamicPropertyCallbacks != null)
                 {
-                    if (weakReferencesOfLottie.ContainsKey(key))
+                    foreach (var key in InternalSavedDynamicPropertyCallbacks.Keys)
                     {
-                        weakReferencesOfLottie.Remove(key);
+                        // Note : We can assume that key is unique.
+                        if (weakReferencesOfLottie.ContainsKey(key))
+                        {
+                            NUILog.Debug($"<[{GetId()}] remove extension actions for {key}>");
+                            weakReferencesOfLottie.TryRemove(key, out var _);
+                        }
                     }
                 }
+                InternalSavedDynamicPropertyCallbacks?.Clear();
+                NUILog.Debug($"<[{GetId()}] remove extension actions finished (total : {weakReferencesOfLottie?.Count})>");
+
+                if (disposing)
+                {
+                    // Ensure to make it as null if we want to dispose current view now.
+                    InternalSavedDynamicPropertyCallbacks = null;
+                }
             }
-            InternalSavedDynamicPropertyCallbacks?.Clear();
-            InternalSavedDynamicPropertyCallbacks = null;
         }
 
         /// <summary>
@@ -1026,6 +1058,8 @@ namespace Tizen.NUI.BaseComponents
             }
 
             base.UpdateImage();
+
+            // TODO : It is necessary to relocate `InternalSavedDynamicPropertyCallbacks` as the visuals have been altered, while the URL remains unchaged.
         }
 
         /// <summary>
@@ -1299,6 +1333,7 @@ namespace Tizen.NUI.BaseComponents
             return ret;
         }
 
+        internal object InternalPropertyCallbacksLock = new object();
         internal Dictionary<int, DynamicPropertyCallbackType> InternalSavedDynamicPropertyCallbacks = new Dictionary<int, DynamicPropertyCallbackType>();
 
         [UnmanagedFunctionPointer(CallingConvention.Cdecl)]
@@ -1308,6 +1343,9 @@ namespace Tizen.NUI.BaseComponents
 
         static internal void RootCallback(int id, int returnType, uint frameNumber, ref float val1, ref float val2, ref float val3)
         {
+            // Called from various worker threads.
+            // Be careful about thread safety!
+
             WeakReference<LottieAnimationView> current = null;
             LottieAnimationView currentView = null;
             DynamicPropertyCallbackType currentCallback = null;
@@ -1317,26 +1355,30 @@ namespace Tizen.NUI.BaseComponents
             {
                 if (current.TryGetTarget(out currentView) && (currentView != null) && !currentView.Disposed && !currentView.IsDisposeQueued)
                 {
-                    if (currentView.InternalSavedDynamicPropertyCallbacks != null &&
-                        currentView.InternalSavedDynamicPropertyCallbacks.TryGetValue(id, out currentCallback))
+                    lock (currentView.InternalPropertyCallbacksLock)
+                    {
+                        currentView.InternalSavedDynamicPropertyCallbacks?.TryGetValue(id, out currentCallback);
+                    }
+
+                    if (currentCallback != null)
                     {
-                        ret = currentCallback?.Invoke(returnType, frameNumber);
+                        ret = currentCallback.Invoke(returnType, frameNumber);
                     }
                     else
                     {
-                        Tizen.Log.Error("NUI", "can't find the callback in LottieAnimationView, just return here!");
+                        Tizen.Log.Debug("NUI", "can't find the callback in LottieAnimationView (Maybe disposed). just return here!");
                         return;
                     }
                 }
                 else
                 {
-                    Tizen.Log.Error("NUI", "can't find the callback in LottieAnimationView, just return here!");
+                    Tizen.Log.Debug("NUI", "LottieAnimationView already disposed. just return here!");
                     return;
                 }
             }
             else
             {
-                Tizen.Log.Error("NUI", "can't find LottieAnimationView by id, just return here!");
+                Tizen.Log.Debug("NUI", "can't find LottieAnimationView by id (Maybe disposed). just return here!");
                 return;
             }
 
@@ -1434,6 +1476,11 @@ namespace Tizen.NUI.BaseComponents
 
         private void onVisualEventSignal(IntPtr targetView, int visualIndex, int signalId)
         {
+            if (Disposed || IsDisposeQueued)
+            {
+                return;
+            }
+
             OnFinished();
 
             if (targetView != IntPtr.Zero)
@@ -1464,7 +1511,7 @@ namespace Tizen.NUI.BaseComponents
 
         static private int dynamicPropertyCallbackId = 0;
         //static private Dictionary<int, DynamicPropertyCallbackType> dynamicPropertyCallbacks = new Dictionary<int, DynamicPropertyCallbackType>();
-        static private Dictionary<int, WeakReference<LottieAnimationView>> weakReferencesOfLottie = new Dictionary<int, WeakReference<LottieAnimationView>>();
+        static private ConcurrentDictionary<int, WeakReference<LottieAnimationView>> weakReferencesOfLottie = new ConcurrentDictionary<int, WeakReference<LottieAnimationView>>();
 
         private void debugPrint()
         {

diff --git a/test/Tizen.NUI.Samples/Tizen.NUI.Samples/Samples/LottieAnimationViewDynamicPropertyTest.cs b/test/Tizen.NUI.Samples/Tizen.NUI.Samples/Samples/LottieAnimationViewDynamicPropertyTest.cs
index d3da7c2e4f36072e2af3cdfd1dea378f3e51f0d2..0b848819596233a969de4a10351bb8d34d44f236 100644 (file)
--- a/test/Tizen.NUI.Samples/Tizen.NUI.Samples/Samples/LottieAnimationViewDynamicPropertyTest.cs
+++ b/test/Tizen.NUI.Samples/Tizen.NUI.Samples/Samples/LottieAnimationViewDynamicPropertyTest.cs
@@ -15,6 +15,7 @@ namespace Tizen.NUI.Samples
         Window win;
         View root;
         Timer timer;
+        List<LottieAnimationView> lavList = new();
         public void Activate()
         {
             win = NUIApplication.GetDefaultWindow();
@@ -39,9 +40,11 @@ namespace Tizen.NUI.Samples
         bool OnTick(object sender, Timer.TickEventArgs e)
         {
             bool ret = false;
-            //ret = Test1();
-            //ret = Test2();
+            // ret = Test1();
+            // ret = Test2();
             ret = Test3();
+            // ret = Test4();
+            // ret = Test5();
             return ret;
         }
 
@@ -92,8 +95,55 @@ namespace Tizen.NUI.Samples
             return true;
         }
 
-        global::System.Random rand = new global::System.Random();
+        //create objects with same as before. Let's check whether leak occured or not.
         bool Test3()
+        {
+            if (timer.Interval == TIMER_INTERVAL)
+            {
+                // Change the interval more tight, to check memory leak
+                timer.Interval = TIMER_INTERVAL / 10;
+            }
+
+            MakeAll();
+            if (cnt % 2 == 0)
+            {
+                ForceFullGC();
+            }
+            cnt++;
+            return true;
+        }
+
+        //create objects first, and change ImageView property what might give effort to lottie images.
+        //Let's check whether dynamic callback still called or not.
+        bool Test4()
+        {
+            if (cnt == 0)
+            {
+                MakeAll();
+            }
+            else
+            {
+                LottieAnimationView lav = null;
+                if (lavList?.Count > 0)
+                {
+                    lav = lavList[0];
+                }
+                if (lav != null)
+                {
+                    // TODO : It will not affect to lottie image, but will create new visual.
+                    //        Let's make this sample works well in future.
+                    tlog.Debug(tag, $"non-Lottie relative property change start");
+                    lav.FastTrackUploading = true;
+                    lav.Play();
+                    tlog.Debug(tag, $"non-Lottie relative property change done");
+                }
+            }
+            cnt++;
+            return cnt < 2;
+        }
+
+        global::System.Random rand = new global::System.Random();
+        bool Test5()
         {
             var lav = new LottieAnimationView();
             lav.Size2D = new Size2D(300, 300);
@@ -108,7 +158,7 @@ namespace Tizen.NUI.Samples
             }
             lav.LoopCount = -1;
             lav.BackgroundColor = Color.White;
-            win.Add(lav);
+            root.Add(lav);
             lav.Play();
 
             var ret = lav.GetContentInfo();
@@ -135,12 +185,21 @@ namespace Tizen.NUI.Samples
             int width = (int)(root.Size.Width / NUM_OF_VIEW);
             for (int i = 0; i < NUM_OF_VIEW; i++)
             {
-                var lav = new LottieAnimationView();
+                LottieAnimationView lav;
+                if (lavList?.Count > i)
+                {
+                    lav = lavList[i];
+                }
+                else
+                {
+                    lav = new LottieAnimationView();
+                    lavList?.Add(lav);
+                    root.Add(lav);
+                }
                 lav.Size2D = new Size2D(width, width);
                 lav.URL = Tizen.Applications.Application.Current.DirectoryInfo.Resource + "done.json";
                 lav.LoopCount = -1;
                 lav.BackgroundColor = Color.White;
-                root.Add(lav);
 
                 LottieAnimationViewDynamicProperty pro = new LottieAnimationViewDynamicProperty
                 {
@@ -185,6 +244,7 @@ namespace Tizen.NUI.Samples
         void DisposeAll()
         {
             tlog.Debug(tag, $"DisposeAll() start");
+            lavList?.Clear();
             int childNum = (int)root.ChildCount;
             for (int i = childNum - 1; i >= 0; i--)
             {
@@ -201,6 +261,7 @@ namespace Tizen.NUI.Samples
         void ImplicitDispose()
         {
             tlog.Debug(tag, $"ImplicitDispose() start");
+            lavList?.Clear();
             int childNum = (int)root.ChildCount;
             for (int i = childNum - 1; i >= 0; i--)
             {
@@ -266,6 +327,8 @@ namespace Tizen.NUI.Samples
             timer.Stop();
             DisposeAll();
             root.Dispose();
+            lavList?.Clear();
+            lavList = null;
         }
     }
 }