diff --git a/src/Tizen.NUI/src/public/BaseComponents/LottieAnimationView.cs b/src/Tizen.NUI/src/public/BaseComponents/LottieAnimationView.cs index c7b4d45fa65..f9370e5398c 100755 --- 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.Runtime.InteropServices; using System.ComponentModel; using System.Collections.Generic; +using System.Collections.Concurrent; using System.Globalization; using Tizen.NUI; using Tizen.NUI.Binding; @@ -111,11 +112,13 @@ protected override void Dispose(DisposeTypes type) 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) @@ -133,8 +136,11 @@ protected override void Dispose(DisposeTypes type) [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 @@ -192,6 +198,9 @@ private string InternalURL { set { + // Invalidate previous dynamic property callbacks. + CleanCallbackDictionaries(false); + // Reset cached infomations. currentStates.contentInfo = null; currentStates.markerInfo = null; @@ -1108,30 +1117,53 @@ public Tuple GetMinMaxFrame() [EditorBrowsable(EditorBrowsableState.Never)] public void DoActionExtension(LottieAnimationViewDynamicProperty info) { + // Called from main thread dynamicPropertyCallbackId++; - weakReferencesOfLottie?.Add(dynamicPropertyCallbackId, new WeakReference(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(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(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; } /// @@ -1166,6 +1198,8 @@ protected override void UpdateImage() } base.UpdateImage(); + + // TODO : It is necessary to relocate `InternalSavedDynamicPropertyCallbacks` as the visuals have been altered, while the URL remains unchaged. } /// @@ -1448,6 +1482,7 @@ internal VisualEventSignal VisualEventSignal() return ret; } + internal object InternalPropertyCallbacksLock = new object(); internal Dictionary InternalSavedDynamicPropertyCallbacks = new Dictionary(); [UnmanagedFunctionPointer(CallingConvention.Cdecl)] @@ -1457,6 +1492,9 @@ internal VisualEventSignal VisualEventSignal() 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 current = null; LottieAnimationView currentView = null; DynamicPropertyCallbackType currentCallback = null; @@ -1464,28 +1502,32 @@ static internal void RootCallback(int id, int returnType, uint frameNumber, ref if (weakReferencesOfLottie.TryGetValue(id, out current)) { - if (current.TryGetTarget(out currentView) && (currentView != null) && !currentView.Disposed && !currentView.IsDisposeQueued) + if (current.TryGetTarget(out currentView) && (currentView != null) && !currentView.IsDisposedOrQueued) { - 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; } @@ -1588,6 +1630,11 @@ private void OnFinished() private void onVisualEventSignal(IntPtr targetView, int visualIndex, int signalId) { + if (IsDisposedOrQueued) + { + return; + } + OnFinished(); if (targetView != IntPtr.Zero) @@ -1618,7 +1665,7 @@ private void onVisualEventSignal(IntPtr targetView, int visualIndex, int signalI static private int dynamicPropertyCallbackId = 0; //static private Dictionary dynamicPropertyCallbacks = new Dictionary(); - static private Dictionary> weakReferencesOfLottie = new Dictionary>(); + static private ConcurrentDictionary> weakReferencesOfLottie = new ConcurrentDictionary>(); 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 5571eb4ef85..e73a151744c 100644 --- 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 @@ public class LottieAnimationViewDynamicPropertyTest : IExample Window win; View root; Timer timer; + List lavList = new(); public void Activate() { win = NUIApplication.GetDefaultWindow(); @@ -39,9 +40,11 @@ public void Activate() bool OnTick(object sender, Timer.TickEventArgs e) { bool ret = false; - ret = Test1(); - //ret = Test2(); - // ret = Test3(); + // ret = Test1(); + // ret = Test2(); + ret = Test3(); + // ret = Test4(); + // ret = Test5(); return ret; } @@ -92,8 +95,55 @@ bool Test2() 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 @@ bool Test3() } lav.LoopCount = -1; lav.BackgroundColor = Color.White; - win.Add(lav); + root.Add(lav); lav.Play(); var ret = lav.GetContentInfo(); @@ -135,12 +185,21 @@ void MakeAll() int width = (int)(root.Size.Width / (NUM_OF_VIEW + 2)); 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 { @@ -181,21 +240,39 @@ void MakeAll() } { - var lav = new LottieAnimationView(); + LottieAnimationView lav; + if (lavList?.Count > NUM_OF_VIEW + 0) + { + lav = lavList[NUM_OF_VIEW + 0]; + } + else + { + lav = new LottieAnimationView(); + lavList?.Add(lav); + root.Add(lav); + } lav.Size2D = new Size2D(width, width); lav.URL = Tizen.Applications.Application.Current.DirectoryInfo.Resource + "30.json"; lav.LoopCount = -1; lav.BackgroundColor = Color.Black; - root.Add(lav); lav.Play(); } { - var lav = new LottieAnimationView(); + LottieAnimationView lav; + if (lavList?.Count > NUM_OF_VIEW + 1) + { + lav = lavList[NUM_OF_VIEW + 1]; + } + else + { + lav = new LottieAnimationView(); + lavList?.Add(lav); + root.Add(lav); + } lav.Size2D = new Size2D(width, width); lav.URL = Tizen.Applications.Application.Current.DirectoryInfo.Resource + "100.json"; lav.LoopCount = -1; lav.BackgroundColor = Color.Black; - root.Add(lav); LottieAnimationViewDynamicProperty pro = new LottieAnimationViewDynamicProperty { @@ -221,6 +298,7 @@ void MakeAll() void DisposeAll() { tlog.Debug(tag, $"DisposeAll() start"); + lavList?.Clear(); int childNum = (int)root.ChildCount; for (int i = childNum - 1; i >= 0; i--) { @@ -237,6 +315,7 @@ void DisposeAll() void ImplicitDispose() { tlog.Debug(tag, $"ImplicitDispose() start"); + lavList?.Clear(); int childNum = (int)root.ChildCount; for (int i = childNum - 1; i >= 0; i--) { @@ -316,6 +395,8 @@ public void Deactivate() timer.Stop(); DisposeAll(); root.Dispose(); + lavList?.Clear(); + lavList = null; } } }