From 7fb112727a40b9b215cc26b81d62f34ea5d0a08e Mon Sep 17 00:00:00 2001 From: Mads Kruse Johnsen Date: Mon, 26 Oct 2020 11:44:13 +0100 Subject: [PATCH 1/6] Prevent double removal of timeout sources See https://github.com/GLibSharp/GtkSharp/pull/49 --- Source/Libs/GLibSharp/Idle.cs | 2 +- Source/Libs/GLibSharp/Timeout.cs | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/Source/Libs/GLibSharp/Idle.cs b/Source/Libs/GLibSharp/Idle.cs index bbdde15f4..db65c5205 100644 --- a/Source/Libs/GLibSharp/Idle.cs +++ b/Source/Libs/GLibSharp/Idle.cs @@ -54,7 +54,7 @@ namespace GLib { { lock (this) { - Remove (); + Dispose(); } } return cont; diff --git a/Source/Libs/GLibSharp/Timeout.cs b/Source/Libs/GLibSharp/Timeout.cs index 8e17e464d..4f3ca34b3 100644 --- a/Source/Libs/GLibSharp/Timeout.cs +++ b/Source/Libs/GLibSharp/Timeout.cs @@ -52,7 +52,7 @@ namespace GLib { { lock (this) { - Remove (); + Dispose(); } } return cont; From c3364fd338d7642b0138770ad8c5b6e5e4ed6ae4 Mon Sep 17 00:00:00 2001 From: Mads Kruse Johnsen Date: Thu, 5 Nov 2020 12:33:43 +0100 Subject: [PATCH 2/6] Fix MessageDialog effectively creating an additional Dialog due to wrong base call --- Source/Libs/GtkSharp/MessageDialog.cs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/Source/Libs/GtkSharp/MessageDialog.cs b/Source/Libs/GtkSharp/MessageDialog.cs index 5034aa5ad..f3cce9270 100644 --- a/Source/Libs/GtkSharp/MessageDialog.cs +++ b/Source/Libs/GtkSharp/MessageDialog.cs @@ -26,7 +26,7 @@ namespace Gtk { delegate IntPtr d_gtk_message_dialog_new_with_markup(IntPtr parent_window, DialogFlags flags, MessageType type, ButtonsType bt, IntPtr msg, IntPtr args); static d_gtk_message_dialog_new_with_markup gtk_message_dialog_new_with_markup = FuncLoader.LoadFunction(FuncLoader.GetProcAddress(GLibrary.Load(Library.Gtk), "gtk_message_dialog_new_with_markup")); - public MessageDialog (Gtk.Window parent_window, DialogFlags flags, MessageType type, ButtonsType bt, bool use_markup, string format, params object[] args) + public MessageDialog (Gtk.Window parent_window, DialogFlags flags, MessageType type, ButtonsType bt, bool use_markup, string format, params object[] args) : base (IntPtr.Zero) { IntPtr p = (parent_window != null) ? parent_window.Handle : IntPtr.Zero; From 470ce6cff70e9b4248e9b105c9c060eeb6a37ae2 Mon Sep 17 00:00:00 2001 From: Mads Kruse Johnsen Date: Thu, 19 Nov 2020 16:22:47 +0100 Subject: [PATCH 3/6] Invoke Destroy in Dispose if the Widget IsToplevel and it is not already destroyed If the Widget is a toplevel then we have not ref'ed the Object, so ref before destroying it, and let the freeing of the ToggleRef undo the ref. --- Source/Libs/GtkSharp/Widget.cs | 31 ++++++++++++++++++++++++++++++- 1 file changed, 30 insertions(+), 1 deletion(-) diff --git a/Source/Libs/GtkSharp/Widget.cs b/Source/Libs/GtkSharp/Widget.cs index 845495fba..78c74abd0 100644 --- a/Source/Libs/GtkSharp/Widget.cs +++ b/Source/Libs/GtkSharp/Widget.cs @@ -370,6 +370,7 @@ namespace Gtk { Gtk.Widget widget = o as Gtk.Widget; if (widget == null) return; + widget.OnDestroyed (); } @@ -387,20 +388,41 @@ namespace Gtk { base.CreateNativeObject (names, vals); } + [UnmanagedFunctionPointer(CallingConvention.Cdecl)] + delegate IntPtr d_g_object_ref(IntPtr raw); + static d_g_object_ref g_object_ref = FuncLoader.LoadFunction(FuncLoader.GetProcAddress(GLibrary.Load(Library.GObject), "g_object_ref")); + + private bool destroyed; protected override void Dispose (bool disposing) { if (Handle == IntPtr.Zero) return; + + if (disposing && !destroyed && IsToplevel) + { + //If this is a TopLevel widget, then we do not hold a ref, only a toggle ref. + //Freeing our toggle ref expects a normal ref to exist, and therefore does not check if the object still exists. + //Take a ref here and let our toggle ref unref it. + g_object_ref (Handle); + gtk_widget_destroy (Handle); + destroyed = true; + } + InternalDestroyed -= NativeDestroyHandler; + base.Dispose (disposing); } - protected override IntPtr Raw { + protected override IntPtr Raw { get { return base.Raw; } set { + if (Handle == value) + return; + base.Raw = value; + if (value != IntPtr.Zero) InternalDestroyed += NativeDestroyHandler; } @@ -409,11 +431,18 @@ namespace Gtk { delegate void d_gtk_widget_destroy(IntPtr raw); static d_gtk_widget_destroy gtk_widget_destroy = FuncLoader.LoadFunction(FuncLoader.GetProcAddress(GLibrary.Load(Library.Gtk), "gtk_widget_destroy")); + public virtual void Destroy () { if (Handle == IntPtr.Zero) return; + + if (destroyed) + return; + gtk_widget_destroy (Handle); + destroyed = true; + InternalDestroyed -= NativeDestroyHandler; } } From 06b88bdbc5ceb7abe4d9185a30cc474bbcd3ec74 Mon Sep 17 00:00:00 2001 From: Mads Kruse Johnsen Date: Thu, 19 Nov 2020 16:41:06 +0100 Subject: [PATCH 4/6] Queue freeing of signals on the main-thread instead of on the Finalizer thread. --- Source/Libs/GLibSharp/Object.cs | 62 ++++++++++++++++++++++++++------- 1 file changed, 50 insertions(+), 12 deletions(-) diff --git a/Source/Libs/GLibSharp/Object.cs b/Source/Libs/GLibSharp/Object.cs index 3ab7f12f1..833b8e513 100644 --- a/Source/Libs/GLibSharp/Object.cs +++ b/Source/Libs/GLibSharp/Object.cs @@ -33,7 +33,7 @@ namespace GLib { IntPtr handle; ToggleRef tref; - bool disposed = false; + bool disposed; static uint idx = 1; static Dictionary Objects = new Dictionary(); static Dictionary> PropertiesToSet = new Dictionary>(); @@ -41,7 +41,7 @@ namespace GLib { ~Object () { if (WarnOnFinalize) - Console.Error.WriteLine ("Unexpected finalization of " + GetType() + " instance. Consider calling Dispose."); + Console.Error.WriteLine ("Unexpected finalization of " + GetType() + " instance. Consider calling Dispose. (" + handle.ToInt64 () + ")"); Dispose (false); } @@ -51,9 +51,10 @@ namespace GLib { if (disposed) return; + GC.SuppressFinalize (this); + Dispose (true); disposed = true; - GC.SuppressFinalize (this); } protected virtual void Dispose (bool disposing) @@ -70,19 +71,25 @@ namespace GLib { return; if (disposing) - tref.Dispose (); - else - tref.QueueUnref (); - - // Free all internal signals, else the garbage collector is not - // able to free the object. - if (signals != null) { - foreach (var sig in signals.Keys) - signals[sig].Free (); + tref.Dispose (); + + if (signals != null) + { + foreach (var sig in signals.Keys) + signals[sig].Free (); + } + } + else + { + if (signals != null) + QueueSignalFree (); + + tref.QueueUnref (); } signals = null; + disposed = true; } public static bool WarnOnFinalize { get; set; } @@ -807,6 +814,9 @@ namespace GLib { GLib.Marshaller.Free (native_name); } + public static List PendingSignalFrees = new List (); + static bool idle_queued; + Dictionary signals; Dictionary Signals { get { @@ -853,6 +863,34 @@ namespace GLib { sig.RemoveDelegate (handler); } + public void QueueSignalFree () + { + lock (PendingSignalFrees) { + PendingSignalFrees.AddRange (signals.Values); + if (!idle_queued){ + Timeout.Add (50, new TimeoutHandler (PerformQueuedSignalFrees)); + idle_queued = true; + } + } + } + + static bool PerformQueuedSignalFrees () + { + Signal[] qsignals; + + lock (PendingSignalFrees){ + qsignals = new Signal[PendingSignalFrees.Count]; + PendingSignalFrees.CopyTo (qsignals, 0); + PendingSignalFrees.Clear (); + idle_queued = false; + } + + foreach (Signal s in qsignals) + s.Free (); + + return false; + } + protected static void OverrideVirtualMethod (GType gtype, string name, Delegate cb) { Signal.OverrideDefaultHandler (gtype, name, cb); From 41d6f0bf284977e39519eb735ac5ca2e2bbf2bec Mon Sep 17 00:00:00 2001 From: Mads Kruse Johnsen Date: Thu, 19 Nov 2020 16:48:29 +0100 Subject: [PATCH 5/6] Queue freeing of GCHandles instead of freeing them immidiately --- Source/Libs/GLibSharp/ToggleRef.cs | 40 ++++++++++++++++++++++++++++-- 1 file changed, 38 insertions(+), 2 deletions(-) diff --git a/Source/Libs/GLibSharp/ToggleRef.cs b/Source/Libs/GLibSharp/ToggleRef.cs index 20517ac99..432257f69 100644 --- a/Source/Libs/GLibSharp/ToggleRef.cs +++ b/Source/Libs/GLibSharp/ToggleRef.cs @@ -71,8 +71,12 @@ namespace GLib { g_object_unref (handle); else g_object_remove_toggle_ref (handle, ToggleNotifyCallback, (IntPtr) gch); + reference = null; - gch.Free (); + + QueueGCHandleFree (); + + handle = IntPtr.Zero; } internal void Harden () @@ -97,7 +101,8 @@ namespace GLib { reference = new WeakReference (reference); else if (!is_last_ref && reference is WeakReference) { WeakReference weak = reference as WeakReference; - reference = weak.Target; + if (weak.IsAlive) + reference = weak.Target; } } @@ -124,6 +129,37 @@ namespace GLib { } } + static List PendingGCHandleFrees = new List (); + static bool gc_idle_queued; + + public void QueueGCHandleFree () + { + lock (PendingGCHandleFrees) { + PendingGCHandleFrees.Add (gch); + if (!gc_idle_queued){ + Timeout.Add (50, new TimeoutHandler (PerformGCHandleFrees)); + gc_idle_queued = true; + } + } + } + + static bool PerformGCHandleFrees () + { + GCHandle[] handles; + + lock (PendingGCHandleFrees){ + handles = new GCHandle [PendingGCHandleFrees.Count]; + PendingGCHandleFrees.CopyTo (handles, 0); + PendingGCHandleFrees.Clear (); + gc_idle_queued = false; + } + + foreach (GCHandle r in handles) + r.Free (); + + return false; + } + static List PendingDestroys = new List (); static bool idle_queued; From 0daa9853166a886005a6219862da5a4c9ce1c296 Mon Sep 17 00:00:00 2001 From: Mads Kruse Johnsen Date: Mon, 25 Jan 2021 12:29:04 +0100 Subject: [PATCH 6/6] Add method GetRawOwnedObject to Builder. This method performs a GetRawObject, but also increases the refcount. This is (at least) needed in cases where you wish to have a class inherting a widget, and use a builder in the constructor : base (builder.GetRawObject("name")) Because no ref is taken the eventual freeing of the builder will cause the our object (this) to have a refcount 0. This is an issue since the freeing of our ToggleRef (which should happen later) performs an unref. --- Source/Libs/GtkSharp/Builder.cs | 10 ++++++++++ 1 file changed, 10 insertions(+) diff --git a/Source/Libs/GtkSharp/Builder.cs b/Source/Libs/GtkSharp/Builder.cs index c09fff6cd..9e6234d5c 100644 --- a/Source/Libs/GtkSharp/Builder.cs +++ b/Source/Libs/GtkSharp/Builder.cs @@ -151,6 +151,16 @@ namespace Gtk { GLib.Marshaller.Free (native_name); return raw_ret; } + + [UnmanagedFunctionPointer(CallingConvention.Cdecl)] + delegate IntPtr d_g_object_ref(IntPtr raw); + static d_g_object_ref g_object_ref = FuncLoader.LoadFunction(FuncLoader.GetProcAddress(GLibrary.Load(Library.GObject), "g_object_ref")); + + public IntPtr GetRawOwnedObject(string name) { + IntPtr raw_ret = GetRawObject (name); + g_object_ref (raw_ret); + return raw_ret; + } public Builder (System.IO.Stream s) : this (s, null) {