Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[NUI][API10] Remove DynamicProperty callback if URL changed + Make more thread safe enough #6508

Merged
merged 1 commit into from
Dec 10, 2024
Merged

[NUI][API10] Remove DynamicProperty callback if URL changed + Make more thread safe enough #6508

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
91 changes: 68 additions & 23 deletions src/Tizen.NUI/src/public/BaseComponents/LottieAnimationView.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,7 @@
using global::System.Runtime.InteropServices;
using System.ComponentModel;
using System.Collections.Generic;
using System.Collections.Concurrent;
using System.Globalization;

namespace Tizen.NUI.BaseComponents
Expand Down Expand Up @@ -80,11 +81,13 @@
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)
Expand All @@ -102,8 +105,11 @@
[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
Expand Down Expand Up @@ -131,6 +137,9 @@
{
set
{
// Invalidate previous dynamic property callbacks.
CleanCallbackDictionaries(false);

string ret = (value == null ? "" : value);
currentStates.url = ret;
currentStates.changed = true;
Expand Down Expand Up @@ -182,7 +191,7 @@
NUILog.Debug($"< Get!");

int ret = 0;
Interop.View.InternalRetrievingVisualPropertyInt(this.SwigCPtr, ImageView.Property.IMAGE, ImageVisualProperty.PlayState, out ret);

Check warning on line 194 in src/Tizen.NUI/src/public/BaseComponents/LottieAnimationView.cs

View workflow job for this annotation

GitHub Actions / build 

get_PlayState calls InternalRetrievingVisualPropertyInt but does not use the HRESULT or error code that the method returns. This could lead to unexpected behavior in error conditions or low-resource situations. Use the result in a conditional statement, assign the result to a variable, or pass it as an argument to another method.

currentStates.playState = (PlayStateType)ret;
NUILog.Debug($"gotten play state={currentStates.playState} >");
Expand All @@ -203,7 +212,7 @@
int ret = currentStates.totalFrame;
if (ret == -1)
{
Interop.View.InternalRetrievingVisualPropertyInt(this.SwigCPtr, ImageView.Property.IMAGE, ImageVisualProperty.TotalFrameNumber, out ret);

Check warning on line 215 in src/Tizen.NUI/src/public/BaseComponents/LottieAnimationView.cs

View workflow job for this annotation

GitHub Actions / build 

get_TotalFrame calls InternalRetrievingVisualPropertyInt but does not use the HRESULT or error code that the method returns. This could lead to unexpected behavior in error conditions or low-resource situations. Use the result in a conditional statement, assign the result to a variable, or pass it as an argument to another method.

currentStates.totalFrame = ret;
NUILog.Debug($"TotalFrameNumber get! ret={ret}");
Expand Down Expand Up @@ -261,7 +270,7 @@
{
int ret = 0;

Interop.View.InternalRetrievingVisualPropertyInt(this.SwigCPtr, ImageView.Property.IMAGE, ImageVisualProperty.CurrentFrameNumber, out ret);

Check warning on line 273 in src/Tizen.NUI/src/public/BaseComponents/LottieAnimationView.cs

View workflow job for this annotation

GitHub Actions / build 

get_InternalCurrentFrame calls InternalRetrievingVisualPropertyInt but does not use the HRESULT or error code that the method returns. This could lead to unexpected behavior in error conditions or low-resource situations. Use the result in a conditional statement, assign the result to a variable, or pass it as an argument to another method.

NUILog.Debug($"CurrentFrameNumber get! val={ret}");
return ret;
Expand Down Expand Up @@ -296,7 +305,7 @@

NUILog.Debug($"<[{GetId()}] SET loopMode={currentStates.loopMode}>");

Interop.View.InternalUpdateVisualPropertyInt(this.SwigCPtr, ImageView.Property.IMAGE, ImageVisualProperty.LoopingMode, (int)currentStates.loopMode);

Check warning on line 308 in src/Tizen.NUI/src/public/BaseComponents/LottieAnimationView.cs

View workflow job for this annotation

GitHub Actions / build 

set_InternalLoopingMode calls InternalUpdateVisualPropertyInt but does not use the HRESULT or error code that the method returns. This could lead to unexpected behavior in error conditions or low-resource situations. Use the result in a conditional statement, assign the result to a variable, or pass it as an argument to another method.
}
}
get
Expand Down Expand Up @@ -349,7 +358,7 @@

NUILog.Debug($"<[{GetId()}]SET currentStates.loopCount={currentStates.loopCount}>");

Interop.View.InternalUpdateVisualPropertyInt(this.SwigCPtr, ImageView.Property.IMAGE, ImageVisualProperty.LoopCount, currentStates.loopCount);

Check warning on line 361 in src/Tizen.NUI/src/public/BaseComponents/LottieAnimationView.cs

View workflow job for this annotation

GitHub Actions / build 

set_InternalLoopCount calls InternalUpdateVisualPropertyInt but does not use the HRESULT or error code that the method returns. This could lead to unexpected behavior in error conditions or low-resource situations. Use the result in a conditional statement, assign the result to a variable, or pass it as an argument to another method.
}
}
get
Expand Down Expand Up @@ -387,7 +396,7 @@

NUILog.Debug($"<[{GetId()}]SET val={currentStates.stopEndAction}>");

Interop.View.InternalUpdateVisualPropertyInt(this.SwigCPtr, ImageView.Property.IMAGE, ImageVisualProperty.StopBehavior, (int)currentStates.stopEndAction);

Check warning on line 399 in src/Tizen.NUI/src/public/BaseComponents/LottieAnimationView.cs

View workflow job for this annotation

GitHub Actions / build 

set_InternalStopBehavior calls InternalUpdateVisualPropertyInt but does not use the HRESULT or error code that the method returns. This could lead to unexpected behavior in error conditions or low-resource situations. Use the result in a conditional statement, assign the result to a variable, or pass it as an argument to another method.
}
}
get
Expand Down Expand Up @@ -429,7 +438,7 @@

NUILog.Debug($"<[{GetId()}]SET currentStates.redrawInScalingDown={currentStates.redrawInScalingDown}>");

Interop.View.InternalUpdateVisualPropertyBool(this.SwigCPtr, ImageView.Property.IMAGE, ImageVisualProperty.RedrawInScalingDown, currentStates.redrawInScalingDown);

Check warning on line 441 in src/Tizen.NUI/src/public/BaseComponents/LottieAnimationView.cs

View workflow job for this annotation

GitHub Actions / build 

set_InternalRedrawInScalingDown calls InternalUpdateVisualPropertyBool but does not use the HRESULT or error code that the method returns. This could lead to unexpected behavior in error conditions or low-resource situations. Use the result in a conditional statement, assign the result to a variable, or pass it as an argument to another method.
}
}
get
Expand Down Expand Up @@ -475,7 +484,7 @@

NUILog.Debug($"<[{GetId()}]SET currentStates.EnableFrameCache={currentStates.enableFrameCache}>");

Interop.View.InternalUpdateVisualPropertyBool(this.SwigCPtr, ImageView.Property.IMAGE, ImageVisualProperty.EnableFrameCache, currentStates.enableFrameCache);

Check warning on line 487 in src/Tizen.NUI/src/public/BaseComponents/LottieAnimationView.cs

View workflow job for this annotation

GitHub Actions / build 

set_InternalEnableFrameCache calls InternalUpdateVisualPropertyBool but does not use the HRESULT or error code that the method returns. This could lead to unexpected behavior in error conditions or low-resource situations. Use the result in a conditional statement, assign the result to a variable, or pass it as an argument to another method.
}
}
get
Expand Down Expand Up @@ -507,7 +516,7 @@
currentStates.mark1 = null;
currentStates.mark2 = null;

Interop.View.InternalUpdateVisualPropertyIntPair(this.SwigCPtr, ImageView.Property.IMAGE, ImageVisualProperty.PlayRange, currentStates.framePlayRangeMin, currentStates.framePlayRangeMax);

Check warning on line 519 in src/Tizen.NUI/src/public/BaseComponents/LottieAnimationView.cs

View workflow job for this annotation

GitHub Actions / build 

SetMinMaxFrame calls InternalUpdateVisualPropertyIntPair but does not use the HRESULT or error code that the method returns. This could lead to unexpected behavior in error conditions or low-resource situations. Use the result in a conditional statement, assign the result to a variable, or pass it as an argument to another method.

NUILog.Debug($" [{GetId()}] currentStates.min:({currentStates.framePlayRangeMin}, max:{currentStates.framePlayRangeMax})>");
}
Expand Down Expand Up @@ -692,7 +701,7 @@

if (string.IsNullOrEmpty(currentStates.mark2))
{
Interop.View.InternalUpdateVisualPropertyString(this.SwigCPtr, ImageView.Property.IMAGE, ImageVisualProperty.PlayRange, currentStates.mark1);

Check warning on line 704 in src/Tizen.NUI/src/public/BaseComponents/LottieAnimationView.cs

View workflow job for this annotation

GitHub Actions / build 

SetMinMaxFrameByMarker calls InternalUpdateVisualPropertyString but does not use the HRESULT or error code that the method returns. This could lead to unexpected behavior in error conditions or low-resource situations. Use the result in a conditional statement, assign the result to a variable, or pass it as an argument to another method.
}
else
{
Expand Down Expand Up @@ -758,30 +767,53 @@
[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, SetDynamicProperty, 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;
}
#endregion Method

Expand Down Expand Up @@ -1019,6 +1051,7 @@
return ret;
}

internal object InternalPropertyCallbacksLock = new object();
internal Dictionary<int, DynamicPropertyCallbackType> InternalSavedDynamicPropertyCallbacks = new Dictionary<int, DynamicPropertyCallbackType>();

[UnmanagedFunctionPointer(CallingConvention.Cdecl)]
Expand All @@ -1028,6 +1061,9 @@

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;
Expand All @@ -1037,26 +1073,30 @@
{
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;
}

Expand Down Expand Up @@ -1145,6 +1185,11 @@

private void onVisualEventSignal(IntPtr targetView, int visualIndex, int signalId)
{
if (Disposed || IsDisposeQueued)
{
return;
}

OnFinished();

if (targetView != IntPtr.Zero)
Expand Down Expand Up @@ -1175,7 +1220,7 @@

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()
{
Expand Down
75 changes: 69 additions & 6 deletions test/Tizen.NUI.Samples/Tizen.NUI.Samples/Samples/LottieAnimationViewDynamicPropertyTest.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,7 @@ public class LottieAnimationViewDynamicPropertyTest : IExample
Window win;
View root;
Timer timer;
List<LottieAnimationView> lavList = new List<LottieAnimationView>();
public void Activate()
{
win = NUIApplication.GetDefaultWindow();
Expand All @@ -39,9 +40,11 @@ public void Activate()
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;
}

Expand Down Expand Up @@ -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.OrientationCorrection = 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);
Expand All @@ -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();
Expand All @@ -135,12 +185,21 @@ void MakeAll()
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
{
Expand Down Expand Up @@ -185,6 +244,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--)
{
Expand All @@ -201,6 +261,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--)
{
Expand Down Expand Up @@ -266,6 +327,8 @@ public void Deactivate()
timer.Stop();
DisposeAll();
root.Dispose();
lavList?.Clear();
lavList = null;
}
}
}
Loading