From f4f56d0ac67df76525715a1eaf9fedcb720e9297 Mon Sep 17 00:00:00 2001 From: Seungkeun Lee Date: Fri, 22 Feb 2019 09:00:50 +0900 Subject: [PATCH] [ElmSharp] Fix memory leak on Image.LoadAsync method (#720) --- src/ElmSharp/ElmSharp/EvasObject.cs | 8 +++++- src/ElmSharp/ElmSharp/EvasObjectEvent.cs | 2 +- src/ElmSharp/ElmSharp/Image.cs | 42 +++++++++++++++++--------------- src/ElmSharp/ElmSharp/SmartEvent.cs | 2 +- 4 files changed, 31 insertions(+), 23 deletions(-) diff --git a/src/ElmSharp/ElmSharp/EvasObject.cs b/src/ElmSharp/ElmSharp/EvasObject.cs index 049bc10..20b8b87 100644 --- a/src/ElmSharp/ElmSharp/EvasObject.cs +++ b/src/ElmSharp/ElmSharp/EvasObject.cs @@ -1277,7 +1277,8 @@ namespace ElmSharp private void DisposeEvent() { - foreach (var evt in _eventStore) + var events = new List(_eventStore); + foreach (var evt in events) { evt.Dispose(); } @@ -1297,5 +1298,10 @@ namespace ElmSharp { _eventStore.Add(item); } + + internal void RemoveFromEventLifeTracker(IInvalidatable item) + { + _eventStore.Remove(item); + } } } diff --git a/src/ElmSharp/ElmSharp/EvasObjectEvent.cs b/src/ElmSharp/ElmSharp/EvasObjectEvent.cs index f551fa2..cfe8db1 100644 --- a/src/ElmSharp/ElmSharp/EvasObjectEvent.cs +++ b/src/ElmSharp/ElmSharp/EvasObjectEvent.cs @@ -351,7 +351,7 @@ namespace ElmSharp { if (disposing) { - // Place holder to dispose managed state (managed objects). + _sender.RemoveFromEventLifeTracker(this); } if (_handle != IntPtr.Zero) { diff --git a/src/ElmSharp/ElmSharp/Image.cs b/src/ElmSharp/ElmSharp/Image.cs index c740e1d..f6403dd 100644 --- a/src/ElmSharp/ElmSharp/Image.cs +++ b/src/ElmSharp/ElmSharp/Image.cs @@ -431,7 +431,7 @@ namespace ElmSharp public bool Load(string file) { if (file == null) - throw new ArgumentNullException("file"); + throw new ArgumentNullException(nameof(file)); Interop.Elementary.elm_image_async_open_set(RealHandle, false); Interop.Elementary.elm_image_preload_disabled_set(RealHandle, true); @@ -447,7 +447,7 @@ namespace ElmSharp public bool Load(Uri uri) { if (uri == null) - throw new ArgumentNullException("uri"); + throw new ArgumentNullException(nameof(uri)); return Load(uri.IsFile ? uri.LocalPath : uri.AbsoluteUri); } @@ -468,7 +468,7 @@ namespace ElmSharp public unsafe bool Load(byte* img, long size) { if (img == null) - throw new ArgumentNullException("img"); + throw new ArgumentNullException(nameof(img)); Interop.Elementary.elm_image_async_open_set(RealHandle, false); Interop.Elementary.elm_image_preload_disabled_set(RealHandle, true); @@ -484,7 +484,7 @@ namespace ElmSharp public bool Load(Stream stream) { if (stream == null) - throw new ArgumentNullException("stream"); + throw new ArgumentNullException(nameof(stream)); Interop.Elementary.elm_image_async_open_set(RealHandle, false); Interop.Elementary.elm_image_preload_disabled_set(RealHandle, true); @@ -507,10 +507,10 @@ namespace ElmSharp /// The cancellation token. /// (true = success, false = error) /// preview - public Task LoadAsync(string file, CancellationToken cancellationToken = default(CancellationToken)) + public async Task LoadAsync(string file, CancellationToken cancellationToken = default(CancellationToken)) { if (file == null) - throw new ArgumentNullException("file"); + throw new ArgumentNullException(nameof(file)); Interop.Elementary.elm_image_async_open_set(RealHandle, true); Interop.Elementary.elm_image_preload_disabled_set(RealHandle, false); @@ -528,7 +528,6 @@ namespace ElmSharp SmartEvent loadReady = new SmartEvent(this, RealHandle, "load,ready"); loadReady.On += (s, e) => { - loadReady.Dispose(); LoadingCompleted?.Invoke(this, EventArgs.Empty); if (tcs != null && !tcs.Task.IsCompleted) { @@ -539,7 +538,6 @@ namespace ElmSharp SmartEvent loadError = new SmartEvent(this, RealHandle, "load,error"); loadError.On += (s, e) => { - loadError.Dispose(); LoadingFailed?.Invoke(this, EventArgs.Empty); if (tcs != null && !tcs.Task.IsCompleted) { @@ -547,13 +545,17 @@ namespace ElmSharp } }; - bool ret = Interop.Elementary.elm_image_file_set(RealHandle, file, null); - if (!ret) + using (loadReady) + using (loadError) { - throw new InvalidOperationException("Failed to set file to Image"); + bool ret = Interop.Elementary.elm_image_file_set(RealHandle, file, null); + if (!ret) + { + throw new InvalidOperationException("Failed to set file to Image"); + } + // it should be return on main thread, because SmartEvent should be disposed on main thread + return await tcs.Task.ConfigureAwait(true); } - - return tcs.Task; } /// @@ -566,7 +568,7 @@ namespace ElmSharp public Task LoadAsync(Uri uri, CancellationToken cancellationToken = default(CancellationToken)) { if (uri == null) - throw new ArgumentNullException("uri"); + throw new ArgumentNullException(nameof(uri)); return LoadAsync(uri.IsFile ? uri.LocalPath : uri.AbsoluteUri, cancellationToken); } @@ -581,7 +583,7 @@ namespace ElmSharp public async Task LoadAsync(Stream stream, CancellationToken cancellationToken = default(CancellationToken)) { if (stream == null) - throw new ArgumentNullException("stream"); + throw new ArgumentNullException(nameof(stream)); Interop.Elementary.elm_image_async_open_set(RealHandle, true); Interop.Elementary.elm_image_preload_disabled_set(RealHandle, false); @@ -599,7 +601,6 @@ namespace ElmSharp SmartEvent loadReady = new SmartEvent(this, RealHandle, "load,ready"); loadReady.On += (s, e) => { - loadReady.Dispose(); LoadingCompleted?.Invoke(this, EventArgs.Empty); if (tcs != null && !tcs.Task.IsCompleted) { @@ -610,7 +611,6 @@ namespace ElmSharp SmartEvent loadError = new SmartEvent(this, RealHandle, "load,error"); loadError.On += (s, e) => { - loadError.Dispose(); LoadingFailed?.Invoke(this, EventArgs.Empty); if (tcs != null && !tcs.Task.IsCompleted) { @@ -619,8 +619,10 @@ namespace ElmSharp }; using (MemoryStream memstream = new MemoryStream()) + using (loadReady) + using (loadError) { - await stream.CopyToAsync(memstream); + await stream.CopyToAsync(memstream).ConfigureAwait(true); unsafe { @@ -634,9 +636,9 @@ namespace ElmSharp } } } + // it should be return on main thread, because SmartEvent should be disposed on main thread + return await tcs.Task.ConfigureAwait(true); } - - return await tcs.Task; } /// diff --git a/src/ElmSharp/ElmSharp/SmartEvent.cs b/src/ElmSharp/ElmSharp/SmartEvent.cs index e52aeec..1200e0d 100644 --- a/src/ElmSharp/ElmSharp/SmartEvent.cs +++ b/src/ElmSharp/ElmSharp/SmartEvent.cs @@ -171,7 +171,7 @@ namespace ElmSharp { if (disposing) { - // Place holder to dispose managed state (managed objects). + _sender.RemoveFromEventLifeTracker(this); } if (_handle != IntPtr.Zero) { -- 2.7.4