From 54a7efb362a9a77467ea69b0d961fffdca087deb Mon Sep 17 00:00:00 2001 From: Seungkeun Lee Date: Thu, 13 Oct 2016 16:15:16 +0900 Subject: [PATCH] Fix GC finalizer issue - Finalizer was called in GC thread and it can't access EFL object - Remove all finalizer code that access EFL object - Hold the reference of EvasObject that was added on Window - Fix wrong IDispose implemention - When Dispose(false) native handle should be freed, but we didn't, it cause seg-fault, after released C# layer, native callback can invoked. - So, We release native handle when Dispose(false) was called. - but, it will cause some error message 'Maybe it has already been freed.' Change-Id: Ic225799ac8cf9fd102a67d870043cb24bad55c7d --- src/ElmSharp/ElmSharp/Conformant.cs | 2 +- src/ElmSharp/ElmSharp/EvasObject.cs | 38 ++++++++++++++++++++----- src/ElmSharp/ElmSharp/ItemObject.cs | 13 +++++---- src/ElmSharp/ElmSharp/Window.cs | 11 +++++++ src/ElmSharp/Interop/Interop.EvasObjectEvent.cs | 18 ++++++------ src/ElmSharp/Interop/Interop.SmartEvent.cs | 20 +++++++------ test/ElmSharp.Test/TestRunner.cs | 1 + 7 files changed, 74 insertions(+), 29 deletions(-) diff --git a/src/ElmSharp/ElmSharp/Conformant.cs b/src/ElmSharp/ElmSharp/Conformant.cs index 8914a70..fc4fd7b 100644 --- a/src/ElmSharp/ElmSharp/Conformant.cs +++ b/src/ElmSharp/ElmSharp/Conformant.cs @@ -8,7 +8,7 @@ namespace ElmSharp { Interop.Evas.evas_object_size_hint_weight_set(Handle, 1.0, 1.0); Interop.Elementary.elm_win_conformant_set(parent.Handle, true); - Interop.Elementary.elm_win_resize_object_add(parent.Handle, Handle); + parent.AddResizeObject(this); } protected override IntPtr CreateHandle(EvasObject parent) diff --git a/src/ElmSharp/ElmSharp/EvasObject.cs b/src/ElmSharp/ElmSharp/EvasObject.cs index 123e635..c373a7d 100644 --- a/src/ElmSharp/ElmSharp/EvasObject.cs +++ b/src/ElmSharp/ElmSharp/EvasObject.cs @@ -26,6 +26,14 @@ namespace ElmSharp OnInstantiated(); } + // C# Finalizer was called on GC thread + // So, We can't access to EFL object + // And When Finalizer was called, Field can be already released. + //~EvasObject() + //{ + // Unrealize(); + //} + public event EventHandler Deleted; public event EventHandler KeyUp; public event EventHandler KeyDown; @@ -212,11 +220,6 @@ namespace ElmSharp Interop.Evas.evas_object_size_hint_weight_set(Handle, x, y); } - ~EvasObject() - { - Unrealize(); - } - public void Show() { Interop.Evas.evas_object_show(Handle); @@ -247,6 +250,9 @@ namespace ElmSharp Deleted?.Invoke(this, EventArgs.Empty); OnInvalidate(); Handle = IntPtr.Zero; + + (Parent as Window)?.RemoveChild(this); + Parent = null; _deleted = null; } @@ -297,6 +303,9 @@ namespace ElmSharp Parent = parent; Handle = CreateHandle(parent); Debug.Assert(Handle != IntPtr.Zero); + + (parent as Window)?.AddChild(this); + OnRealized(); _deleted = new Interop.EvasObjectEvent(this, Handle, Interop.Evas.ObjectCallbackType.Del); _keydown = new Interop.EvasObjectEvent(this, Handle, Interop.Evas.ObjectCallbackType.KeyDown, EvasKeyEventArgs.Create); @@ -317,10 +326,25 @@ namespace ElmSharp if (IsRealized) { OnUnrealize(); - Interop.Evas.evas_object_del(Handle); + IntPtr toBeDeleted = Handle; Handle = IntPtr.Zero; - Parent = null; + _deleted?.Dispose(); _deleted = null; + _keydown?.Dispose(); + _keydown = null; + _keyup?.Dispose(); + _keyup = null; + _moved?.Dispose(); + _moved = null; + _resized?.Dispose(); + _resized = null; + _renderPost?.Dispose(); + _renderPost = null; + + (Parent as Window)?.RemoveChild(this); + + Interop.Evas.evas_object_del(toBeDeleted); + Parent = null; } } } diff --git a/src/ElmSharp/ElmSharp/ItemObject.cs b/src/ElmSharp/ElmSharp/ItemObject.cs index e8c9be7..a3cc6e2 100644 --- a/src/ElmSharp/ElmSharp/ItemObject.cs +++ b/src/ElmSharp/ElmSharp/ItemObject.cs @@ -23,11 +23,14 @@ namespace ElmSharp Handle = handle; } - ~ItemObject() - { - if (Handle != IntPtr.Zero) - Interop.Elementary.elm_object_item_del(Handle); - } + // C# Finalizer was called on GC thread + // So, We can't access to EFL object + // And When Finalizer was called, Field can be already released. + //~ItemObject() + //{ + // if (Handle != IntPtr.Zero) + // Interop.Elementary.elm_object_item_del(Handle); + //} public int Id { get; private set; } diff --git a/src/ElmSharp/ElmSharp/Window.cs b/src/ElmSharp/ElmSharp/Window.cs index efc239d..02933df 100644 --- a/src/ElmSharp/ElmSharp/Window.cs +++ b/src/ElmSharp/ElmSharp/Window.cs @@ -37,6 +37,7 @@ namespace ElmSharp { Interop.SmartEvent _deleteRequest; Interop.SmartEvent _rotationChanged; + HashSet _referenceHolder = new HashSet(); public Window(string name) : this(null, name) { @@ -151,6 +152,16 @@ namespace ElmSharp return Interop.Elementary.elm_win_add(parent != null ? parent.Handle : IntPtr.Zero, Name, 0); } + internal void AddChild(EvasObject obj) + { + _referenceHolder.Add(obj); + } + + internal void RemoveChild(EvasObject obj) + { + _referenceHolder.Remove(obj); + } + static int[] ConvertDegreeArray(DisplayRotation value) { List rotations = new List(); diff --git a/src/ElmSharp/Interop/Interop.EvasObjectEvent.cs b/src/ElmSharp/Interop/Interop.EvasObjectEvent.cs index 9f5cade..3b8b614 100644 --- a/src/ElmSharp/Interop/Interop.EvasObjectEvent.cs +++ b/src/ElmSharp/Interop/Interop.EvasObjectEvent.cs @@ -78,10 +78,11 @@ internal static partial class Interop { if (disposing) { - foreach (var cb in _nativeCallbacks) - { - Evas.evas_object_event_callback_del(_handle, _type, cb.callback); - } + // Place holder to dispose managed state (managed objects). + } + foreach (var cb in _nativeCallbacks) + { + Evas.evas_object_event_callback_del(_handle, _type, cb.callback); } _nativeCallbacks.Clear(); _disposed = true; @@ -152,10 +153,11 @@ internal static partial class Interop { if (disposing) { - foreach (var cb in _nativeCallbacks) - { - Evas.evas_object_event_callback_del(_handle, _type, cb.callback); - } + // Place holder to dispose managed state (managed objects). + } + foreach (var cb in _nativeCallbacks) + { + Evas.evas_object_event_callback_del(_handle, _type, cb.callback); } _nativeCallbacks.Clear(); _disposed = true; diff --git a/src/ElmSharp/Interop/Interop.SmartEvent.cs b/src/ElmSharp/Interop/Interop.SmartEvent.cs index 5116e6a..b83d4b4 100644 --- a/src/ElmSharp/Interop/Interop.SmartEvent.cs +++ b/src/ElmSharp/Interop/Interop.SmartEvent.cs @@ -74,10 +74,11 @@ internal static partial class Interop { if (disposing) { - foreach (var cb in _nativeCallbacks) - { - Evas.evas_object_smart_callback_del(_handle, _eventName, cb.callback); - } + // Place holder to dispose managed state (managed objects). + } + foreach (var cb in _nativeCallbacks) + { + Evas.evas_object_smart_callback_del(_handle, _eventName, cb.callback); } _nativeCallbacks.Clear(); } @@ -85,6 +86,7 @@ internal static partial class Interop public void Dispose() { Dispose(true); + GC.SuppressFinalize(this); } } @@ -142,10 +144,11 @@ internal static partial class Interop { if (disposing) { - foreach (var cb in _nativeCallbacks) - { - Evas.evas_object_smart_callback_del(_handle, _eventName, cb.callback); - } + // Place holder to dispose managed state (managed objects). + } + foreach (var cb in _nativeCallbacks) + { + Evas.evas_object_smart_callback_del(_handle, _eventName, cb.callback); } _nativeCallbacks.Clear(); } @@ -153,6 +156,7 @@ internal static partial class Interop public void Dispose() { Dispose(true); + GC.SuppressFinalize(this); } } } diff --git a/test/ElmSharp.Test/TestRunner.cs b/test/ElmSharp.Test/TestRunner.cs index 4c95734..52fc071 100644 --- a/test/ElmSharp.Test/TestRunner.cs +++ b/test/ElmSharp.Test/TestRunner.cs @@ -96,6 +96,7 @@ namespace ElmSharp.Test if (e.KeyName == EvasKeyEventArgs.PlatformBackButtonName) { window.Hide(); + window.Unrealize(); } }; } -- 2.7.4