[NUI] Fix WeakEvent bugs

* Checks that target object is explicitly disposed but not collected yet before invoke
* Consider list item removal/addition while traversing for invoke
* WeakHandler Equal method works well in static case
* Cleanup dead handlers not everytime

Signed-off-by: Jiyun Yang <ji.yang@samsung.com>

diff --git a/src/Tizen.NUI/src/internal/Common/Disposable.cs b/src/Tizen.NUI/src/internal/Common/Disposable.cs
index 72d1dc8..e394b1c 100644 (file)
--- a/src/Tizen.NUI/src/internal/Common/Disposable.cs
+++ b/src/Tizen.NUI/src/internal/Common/Disposable.cs
@@ -199,6 +199,12 @@ namespace Tizen.NUI
         /// </summary>
         [EditorBrowsable(EditorBrowsableState.Never)]
         protected internal bool Disposed => disposed;
+
+        /// <summary>
+        /// The flag to check if it is disposed by DisposeQueue.
+        /// </summary>
+        [EditorBrowsable(EditorBrowsableState.Never)]
+        protected internal bool IsDisposeQueued => isDisposeQueued;
     }
 
     internal static class DisposableExtension

diff --git a/src/Tizen.NUI/src/internal/Common/WeakEvent.cs b/src/Tizen.NUI/src/internal/Common/WeakEvent.cs
index 83c2c59..5a1e9e8 100755 (executable)
--- a/src/Tizen.NUI/src/internal/Common/WeakEvent.cs
+++ b/src/Tizen.NUI/src/internal/Common/WeakEvent.cs
@@ -21,68 +21,46 @@ using System.Reflection;
 
 namespace Tizen.NUI
 {
-    internal class WeakEvent<T>
+    internal class WeakEvent<T> where T : Delegate
     {
+        private const int cleanUpThreshold = 100; // Experimetal constant
+        private int cleanUpCount = 0;
         private List<WeakHandler<T>> handlers = new List<WeakHandler<T>>();
 
         protected int Count => handlers.Count;
 
         public virtual void Add(T handler)
         {
-            if (handlers == null)
-            {
-                handlers = new List<WeakHandler<T>>();
-            }
-
             handlers.Add(new WeakHandler<T>(handler));
-
             OnCountIncreased();
+
+            CleanUpDeadHandlersIfNeeds();
         }
 
         public virtual void Remove(T handler)
         {
-            if (handlers == null)
-            {
-                return;
-            }
-
-            int count = handlers.Count;
+            int lastIndex = handlers.FindLastIndex(item => item.Equals(handler));
 
-            handlers.RemoveAll(item => !item.IsAlive || item.Equals(handler));
-
-            if (count > handlers.Count)
+            if (lastIndex >= 0)
             {
+                handlers.RemoveAt(lastIndex);
                 OnCountDicreased();
             }
+
+            CleanUpDeadHandlersIfNeeds();
         }
 
         public void Invoke(object sender, EventArgs args)
         {
-            if (handlers == null)
+            // Iterate copied one to prevent addition/removal item in the handler call.
+            var copiedArray = handlers.ToArray();
+            foreach (var item in copiedArray)
             {
-                return;
+                item.Invoke(sender, args);
             }
 
-            var disposed = new HashSet<WeakHandler<T>>();
-
-            int count = handlers.Count;
-
-            foreach (var item in handlers)
-            {
-                if (item.IsAlive)
-                {
-                    item.Invoke(sender, args);
-                    continue;
-                }
-                disposed.Add(item);
-            }
-
-            handlers.RemoveAll(disposed.Contains);
-
-            if (count > handlers.Count)
-            {
-                OnCountDicreased();
-            }
+            // Clean up GC items
+            CleanUpDeadHandlers();
         }
 
         protected virtual void OnCountIncreased()
@@ -94,29 +72,67 @@ namespace Tizen.NUI
         {
         }
 
-        internal class WeakHandler<U>
+        private void CleanUpDeadHandlersIfNeeds()
         {
-            private WeakReference weakReference;
+            if (++cleanUpCount == cleanUpThreshold)
+            {
+                CleanUpDeadHandlers();
+            }
+        }
+
+        private void CleanUpDeadHandlers()
+        {
+            cleanUpCount = 0;
+            int count = handlers.Count;
+            handlers.RemoveAll(item => !item.IsAlive);
+            if (count > handlers.Count) OnCountDicreased();
+        }
+
+        internal class WeakHandler<U> where U : Delegate
+        {
+            private WeakReference weakTarget; // Null value means the method is static.
             private MethodInfo methodInfo;
 
             public WeakHandler(U handler)
             {
                 Delegate d = (Delegate)(object)handler;
-                if (d.Target != null) weakReference = new WeakReference(d.Target);
+                if (d.Target != null) weakTarget = new WeakReference(d.Target);
                 methodInfo = d.Method;
             }
 
+            private bool IsStatic => weakTarget == null;
+
+            public bool IsAlive
+            {
+                get
+                {
+                    var rooting = weakTarget?.Target;
+
+                    return IsStatic || !IsDisposed(rooting);
+                }
+            }
+
+            private static bool IsDisposed(object target)
+            {
+                if (target == null) return true;
+
+                if (target is BaseHandle basehandle) return basehandle.Disposed || basehandle.IsDisposeQueued;
+
+                if (target is Disposable disposable) return disposable.Disposed || disposable.IsDisposeQueued;
+
+                return false;
+            }
+
             public bool Equals(U handler)
             {
                 Delegate other = (Delegate)(object)handler;
-                return other != null && other.Target == weakReference?.Target && other.Method.Equals(methodInfo);
+                bool isOtherStatic = other.Target == null;
+                return (isOtherStatic || weakTarget?.Target == other.Target) && methodInfo.Equals(other.Method);
             }
 
-            public bool IsAlive => weakReference == null || weakReference.IsAlive;
-
             public void Invoke(params object[] args)
             {
-                if (weakReference == null)
+                if (IsStatic)
                 {
                     Delegate.CreateDelegate(typeof(U), methodInfo).DynamicInvoke(args);
                 }
@@ -125,10 +141,12 @@ namespace Tizen.NUI
                     // Because GC is done in other thread,
                     // it needs to check again that the reference is still alive before calling method.
                     // To do that, the reference should be assigned to the local variable first.
-                    var localRefCopied = weakReference.Target;
+                    var rooting = weakTarget.Target;
 
-                    // Do not change this to if (weakReference.Target != null)
-                    if (localRefCopied != null) Delegate.CreateDelegate(typeof(U), localRefCopied, methodInfo).DynamicInvoke(args);
+                    if (IsAlive)
+                    {
+                        Delegate.CreateDelegate(typeof(U), rooting, methodInfo).DynamicInvoke(args);
+                    }
                 }
             }
         }