From 0c5bd3f47106f22b6407be0f28c60ca19bc533f0 Mon Sep 17 00:00:00 2001 From: zii-dmg Date: Thu, 27 Jan 2022 23:49:10 +0300 Subject: [PATCH] Fixed glib source double removal (#327) Fixed glib source double removal then using Source.Remove (Timeout.Remove, Idle.Remove). I don't know if fix is correct or safe, but it solves problem that you can test in TimerDemo section from samples. Repro: If on Windows you should enable console in samples: Exe Go to TimerDemo section and press buttons: 1. Add timer 2. Remove timer by handler 3. GC - no error in console 1. Add timer 2. Remove timer 3. GC - error in console "GLib-CRITICAL **: 20:29:41.579: Source ID 123 was not found when attempting to remove it" --- Source/Libs/GLibSharp/DestroyNotify.cs | 23 ++++- Source/Libs/GLibSharp/Idle.cs | 10 +-- Source/Libs/GLibSharp/Timeout.cs | 12 +-- .../Sections/Miscellaneous/TimerSection.cs | 84 +++++++++++++++++++ 4 files changed, 109 insertions(+), 20 deletions(-) create mode 100644 Source/Samples/Sections/Miscellaneous/TimerSection.cs diff --git a/Source/Libs/GLibSharp/DestroyNotify.cs b/Source/Libs/GLibSharp/DestroyNotify.cs index f11dbc5dc..67c8e9f9e 100644 --- a/Source/Libs/GLibSharp/DestroyNotify.cs +++ b/Source/Libs/GLibSharp/DestroyNotify.cs @@ -47,6 +47,27 @@ namespace GLib { return release_gchandle; } } + + static void ReleaseSourceProxy (IntPtr data) + { + if (data == IntPtr.Zero) + return; + GCHandle gch = (GCHandle)data; + if (gch.Target is SourceProxy proxy) { + lock (proxy) + proxy.Dispose (); + } + gch.Free(); + } + + static DestroyNotify release_sourceproxy; + + public static DestroyNotify SourceProxyNotifyHandler { + get { + if (release_sourceproxy == null) + release_sourceproxy = new DestroyNotify (ReleaseSourceProxy); + return release_sourceproxy; + } + } } } - diff --git a/Source/Libs/GLibSharp/Idle.cs b/Source/Libs/GLibSharp/Idle.cs index 756cbc7fb..7aa081089 100644 --- a/Source/Libs/GLibSharp/Idle.cs +++ b/Source/Libs/GLibSharp/Idle.cs @@ -48,15 +48,7 @@ namespace GLib { { try { IdleHandler idle_handler = (IdleHandler) real_handler; - bool cont = idle_handler (); - if (!cont) - { - lock (this) - { - Dispose(); - } - } return cont; } catch (Exception e) { ExceptionManager.RaiseUnhandledException (e, false); @@ -80,7 +72,7 @@ namespace GLib { { var gch = GCHandle.Alloc(p); var userData = GCHandle.ToIntPtr(gch); - p.ID = g_idle_add_full (priority, (IdleHandlerInternal) p.proxy_handler, userData, DestroyHelper.NotifyHandler); + p.ID = g_idle_add_full (priority, (IdleHandlerInternal) p.proxy_handler, userData, DestroyHelper.SourceProxyNotifyHandler); } return p.ID; diff --git a/Source/Libs/GLibSharp/Timeout.cs b/Source/Libs/GLibSharp/Timeout.cs index 19c1d49ef..46cff9d76 100644 --- a/Source/Libs/GLibSharp/Timeout.cs +++ b/Source/Libs/GLibSharp/Timeout.cs @@ -46,15 +46,7 @@ namespace GLib { { try { TimeoutHandler timeout_handler = (TimeoutHandler) real_handler; - bool cont = timeout_handler (); - if (!cont) - { - lock (this) - { - Dispose(); - } - } return cont; } catch (Exception e) { ExceptionManager.RaiseUnhandledException (e, false); @@ -76,7 +68,7 @@ namespace GLib { { var gch = GCHandle.Alloc(p); var userData = GCHandle.ToIntPtr(gch); - p.ID = g_timeout_add_full (priority, interval, (TimeoutHandlerInternal) p.proxy_handler, userData, DestroyHelper.NotifyHandler); + p.ID = g_timeout_add_full (priority, interval, (TimeoutHandlerInternal) p.proxy_handler, userData, DestroyHelper.SourceProxyNotifyHandler); } return p.ID; @@ -103,7 +95,7 @@ namespace GLib { { var gch = GCHandle.Alloc(p); var userData = GCHandle.ToIntPtr(gch); - p.ID = g_timeout_add_seconds_full (priority, interval, (TimeoutHandlerInternal) p.proxy_handler, userData, DestroyHelper.NotifyHandler); + p.ID = g_timeout_add_seconds_full (priority, interval, (TimeoutHandlerInternal) p.proxy_handler, userData, DestroyHelper.SourceProxyNotifyHandler); } return p.ID; diff --git a/Source/Samples/Sections/Miscellaneous/TimerSection.cs b/Source/Samples/Sections/Miscellaneous/TimerSection.cs new file mode 100644 index 000000000..d2dd8144c --- /dev/null +++ b/Source/Samples/Sections/Miscellaneous/TimerSection.cs @@ -0,0 +1,84 @@ +using System; +using System.Collections.Generic; +using Gtk; + +namespace Samples +{ + [Section(ContentType = typeof(TimerDemo), Category = Category.Miscellaneous)] + class TimerSection : Box + { + public TimerSection() : base(Orientation.Horizontal, 3) + { + Valign = Align.Start; + TimerDemo.Create(this); + } + } + + static class TimerDemo + { + public static void Create(Box box) + { + List timers = new List(); + bool removeByHandler = false; + + var btnAddTimer = new Button() { Label = "Add timer" }; + btnAddTimer.Clicked += delegate + { + uint id = 0; + id = GLib.Timeout.Add(500, () => + { + ApplicationOutput.WriteLine("Timer tick " + id); + + if (removeByHandler) + { + removeByHandler = false; + timers.Remove(id); + ApplicationOutput.WriteLine("Remove timer from handler " + id); + return false; + } + + return true; + }); + + timers.Add(id); + ApplicationOutput.WriteLine("Add timer " + id); + }; + + var btnRemoveTimer = new Button() { Label = "Remove timer" }; + btnRemoveTimer.Clicked += delegate + { + if (timers.Count == 0) + return; + + uint id = timers[0]; + timers.RemoveAt(0); + GLib.Timeout.Remove(id); + ApplicationOutput.WriteLine("Remove timer " + id); + }; + + var btnRemoveTimerByHandler = new Button() { Label = "Remove timer by handler" }; + btnRemoveTimerByHandler.Clicked += delegate + { + if (timers.Count == 0) + return; + + removeByHandler = true; + ApplicationOutput.WriteLine("Remove timer by handler"); + }; + + var btnGc = new Button() { Label = "GC" }; + btnGc.Clicked += delegate + { + ApplicationOutput.WriteLine("GC"); + GC.Collect(); + GC.WaitForPendingFinalizers(); + GC.Collect(); + }; + + box.PackStart(btnAddTimer, false, false, 0); + box.PackStart(btnRemoveTimer, false, false, 0); + box.PackStart(btnRemoveTimerByHandler, false, false, 0); + box.PackStart(btnGc, false, false, 0); + } + } +}