[Gtk-sharp-list] Delegate wrappers and GC [Patch]

Gonzalo Paniagua Javier gonzalo@ximian.com
03 Apr 2003 03:11:29 +0200


--=-Mrp6aQ918sheyF9N1qy7
Content-Type: text/plain
Content-Transfer-Encoding: 7bit

Hi!

While working on nunit-gtk (which it's now in its own CVS module), I had
some troubles with a NullReference being thrown in the callback for the
cell renderer function.

The problem *was* that the delegate wrapper itself was garbage
collected, so the _managed field was not there when the callback is
called from unmanaged code.

Attached you can get a patch that fixes this issue.

The patch modifies DelegateWrapper to hold a Hashtable containing
DelegateWrappers as keys and a WeakReference to the object that creates
them as values.

It also contains a new method (RemoveIfNotAlive) which is now called at
the beginning of the callback method. If the target object has been
collected, returns true, preventing the NullReferenceException and also
removes the delegate wrapper from the hash table (which will allow the
GC to finalize it).

The changes to the generator just use the new constructors needed to
pass an object (the creator of the wrapper) or null (when created in
static methods). Note that the wrappers created in static methods are
never freed.

Ok to apply?

-Gonzalo


--=-Mrp6aQ918sheyF9N1qy7
Content-Disposition: attachment; filename=entry
Content-Type: text/plain; name=entry; charset=ISO-8859-1
Content-Transfer-Encoding: 7bit

2003-04-03  Gonzalo Paniagua Javier <gonzalo@ximian.com>

	* generator/CallbackGen.cs: the new generated wrappers have:
		-(optional) Field of the same type returned by the callback.
		-A call to RemoveIfNotAlive at the beginning. It returns true,
		return the dummy field.
		-Added an object to the ctor signature and pass it to the base
		class.

	* generator/Ctor.cs: added a Params property.

	* generator/Method.cs: set Static property in Parameters if the method
	is static.

	* generator/Parameters.cs: added Static property. The call creation of
	the delegate wrapper (if applicable) uses the new signature. Pass a null
	as object is the method is static.

	* generator/StructBase.cs: set Static for the parameters of the ctors.

	* glib/DelegateWrapper.cs: the ctor takes an object (the one creating
	the wrapper or null) and creates a weak reference to it. Store it in
	a static Hashtable (this way the wrapper itself is not garbage
	collected).
	(RemoveIfNotAlive): called from the native delegate callbacks. If the
	target of the weak reference has been garbage collected, removes itself
	from the hashtable to let the GC dispose this instance and returns true.

	* gdk/Pixbuf.custom:
	* gtk/Clipboard.custom:
	* gtk/GtkSharp.GtkClipboardClearFuncNative.cs:
	* gtk/GtkSharp.GtkClipboardGetFuncNative.cs:
	* glade/XML.custom: changed delegate wrappers to match the new
	signature.


--=-Mrp6aQ918sheyF9N1qy7
Content-Disposition: attachment; filename=wrappers.patch
Content-Type: text/x-patch; name=wrappers.patch; charset=ISO-8859-1
Content-Transfer-Encoding: 7bit

Index: gdk/Pixbuf.custom
===================================================================
RCS file: /cvs/public/gtk-sharp/gdk/Pixbuf.custom,v
retrieving revision 1.4
diff -u -r1.4 Pixbuf.custom
--- gdk/Pixbuf.custom	14 Mar 2003 03:04:15 -0000	1.4
+++ gdk/Pixbuf.custom	3 Apr 2003 01:05:14 -0000
@@ -82,7 +82,7 @@
 		public Pixbuf(byte [] data, bool has_alpha, int bits_per_sample, int width, int height, int rowstride, Gdk.PixbufDestroyNotify destroy_fn)
 		{
 			GdkSharp.PixbufDestroyNotifyWrapper destroy_fn_wrapper = null;
-			destroy_fn_wrapper = new GdkSharp.PixbufDestroyNotifyWrapper (destroy_fn);
+			destroy_fn_wrapper = new GdkSharp.PixbufDestroyNotifyWrapper (destroy_fn, this);
 			Raw = gdk_pixbuf_new_from_data(data, (int) Gdk.Colorspace.Rgb, has_alpha, bits_per_sample, width, height, rowstride, destroy_fn_wrapper.NativeDelegate, IntPtr.Zero);
 		}
 
Index: generator/CallbackGen.cs
===================================================================
RCS file: /cvs/public/gtk-sharp/generator/CallbackGen.cs,v
retrieving revision 1.18
diff -u -r1.18 CallbackGen.cs
--- generator/CallbackGen.cs	24 Feb 2003 03:13:07 -0000	1.18
+++ generator/CallbackGen.cs	3 Apr 2003 01:05:14 -0000
@@ -84,15 +84,34 @@
 			XmlElement ret_elem = Elem["return-type"];
 			string rettype = ret_elem.GetAttribute("type");
 			string m_ret = SymbolTable.GetMarshalReturnType (rettype);
+			ClassBase ret_wrapper = SymbolTable.GetClassGen (rettype);
 
 			sw.WriteLine ("\tpublic delegate " + m_ret + " " + wrapper + "(" + import_sig + ");");
 			sw.WriteLine ();
 			
 			sw.WriteLine ("\tpublic class " + Name + "Wrapper : GLib.DelegateWrapper {");
+			if (m_ret != "void") {
+				if (SymbolTable.IsEnum (rettype)) {
+					sw.WriteLine ("\t\tstatic int _dummy;");
+				} else if (ret_wrapper != null && (ret_wrapper is ObjectGen || ret_wrapper is OpaqueGen)) {
+					// Do nothing
+				} else if (!SymbolTable.IsStruct (rettype) && !SymbolTable.IsBoxed (rettype)) {
+					sw.WriteLine ("\t\tstatic {0} _dummy;", s_ret);
+				}
+			}
+			
 			sw.WriteLine ();
 
 			sw.WriteLine ("\t\tpublic " + m_ret + " NativeCallback (" + import_sig + ")");
 			sw.WriteLine ("\t\t{");
+			sw.Write ("\t\t\tif (RemoveIfNotAlive ()) return ");
+			if (SymbolTable.IsStruct (rettype) || SymbolTable.IsBoxed (rettype))
+				sw.WriteLine ("IntPtr.Zero;");
+			else if (ret_wrapper != null && (ret_wrapper is ObjectGen || ret_wrapper is OpaqueGen))
+				sw.WriteLine ("IntPtr.Zero;");
+			else
+				sw.WriteLine (m_ret != "void" ? "_dummy;" : ";");
+
 			int count = (parms != null) ? parms.Count : 0;
 			if (count > 0)
 				sw.WriteLine ("\t\t\tobject[] _args = new object[{0}];", count);
@@ -129,8 +148,7 @@
 			sw.Write ("\t\t\t");
 			string invoke = "_managed (" + call_str + ")";
 			if (m_ret != "void") {
-					ClassBase parm_wrapper = SymbolTable.GetClassGen (rettype);
-					if (parm_wrapper != null && (parm_wrapper is ObjectGen || parm_wrapper is OpaqueGen))
+					if (ret_wrapper != null && (ret_wrapper is ObjectGen || ret_wrapper is OpaqueGen))
 						sw.WriteLine ("return (({0}) {1}).Handle;", s_ret, invoke);
 					else if (SymbolTable.IsStruct (rettype) || SymbolTable.IsBoxed (rettype)) {
 						// Shoot. I have no idea what to do here.
@@ -150,7 +168,7 @@
 			sw.WriteLine ("\t\tprotected {0} _managed;", NS + "." + Name);
 			sw.WriteLine ();
 
-			sw.WriteLine ("\t\tpublic {0} ({1} managed) : base ()", Name + "Wrapper", NS + "." + Name);
+			sw.WriteLine ("\t\tpublic {0} ({1} managed, object o) : base (o)", Name + "Wrapper", NS + "." + Name);
 			sw.WriteLine ("\t\t{");
 
 			sw.WriteLine ("\t\t\tNativeDelegate = new {0} (NativeCallback);", wrapper);
Index: generator/Ctor.cs
===================================================================
RCS file: /cvs/public/gtk-sharp/generator/Ctor.cs,v
retrieving revision 1.10
diff -u -r1.10 Ctor.cs
--- generator/Ctor.cs	1 Nov 2002 05:01:22 -0000	1.10
+++ generator/Ctor.cs	3 Apr 2003 01:05:14 -0000
@@ -31,6 +31,10 @@
 			set { force_static = value; }
 		}
 	
+		public Parameters Params {
+			get { return parms; }
+		}
+
 		public Ctor (string libname, XmlElement elem, ClassBase container_type) {
 			this.libname = libname;
 			this.elem = elem;
Index: generator/Method.cs
===================================================================
RCS file: /cvs/public/gtk-sharp/generator/Method.cs,v
retrieving revision 1.26
diff -u -r1.26 Method.cs
--- generator/Method.cs	1 Nov 2002 05:01:22 -0000	1.26
+++ generator/Method.cs	3 Apr 2003 01:05:14 -0000
@@ -170,6 +170,7 @@
 			is_set = ((parms != null && (parms.IsAccessor || (parms.Count == 1 && s_ret == "void"))) && (Name.Length > 3 && Name.Substring(0, 3) == "Set"));
 			
 			if (parms != null) {
+				parms.Static = is_shared;
 				parms.CreateSignature (is_set);
 				sig = "(" + parms.Signature + ")";
 				isig = "(" + (is_shared ? "" : container_type.MarshalType + " raw, ") + parms.ImportSig + ");";
Index: generator/Parameters.cs
===================================================================
RCS file: /cvs/public/gtk-sharp/generator/Parameters.cs,v
retrieving revision 1.27
diff -u -r1.27 Parameters.cs
--- generator/Parameters.cs	24 Feb 2003 07:36:30 -0000	1.27
+++ generator/Parameters.cs	3 Apr 2003 01:05:15 -0000
@@ -123,6 +123,7 @@
 		private string signature;
 		private string signature_types;
 		private bool hide_data;
+		private bool is_static;
 
 		public Parameters (XmlElement elem) {
 			
@@ -169,6 +170,10 @@
 			set { hide_data = value; }
 		}
 
+		public bool Static {
+			set { is_static = value; }
+		}
+
 		public bool Validate ()
 		{
 			foreach (XmlNode parm in elem.ChildNodes) {
@@ -394,7 +399,7 @@
 					{
 						sw.Write ("if ({0} != null) ", name);
 					}
-					sw.WriteLine ("{1}_wrapper = new {0} ({1});", type, name);
+					sw.WriteLine ("{1}_wrapper = new {0} ({1}, {2});", type, name, is_static ? "null" : "this");
 				}
 			}
 		}
Index: generator/StructBase.cs
===================================================================
RCS file: /cvs/public/gtk-sharp/generator/StructBase.cs,v
retrieving revision 1.32
diff -u -r1.32 StructBase.cs
--- generator/StructBase.cs	5 Jan 2003 23:51:37 -0000	1.32
+++ generator/StructBase.cs	3 Apr 2003 01:05:15 -0000
@@ -262,7 +262,10 @@
 
 			foreach (Ctor ctor in Ctors) {
 				ctor.ForceStatic = true;
+				if (ctor.Params != null)
+					ctor.Params.Static = true;
 			}
+
 			base.GenCtors (sw);
 		}
 
Index: glade/XML.custom
===================================================================
RCS file: /cvs/public/gtk-sharp/glade/XML.custom,v
retrieving revision 1.10
diff -u -r1.10 XML.custom
--- glade/XML.custom	15 Mar 2003 21:02:25 -0000	1.10
+++ glade/XML.custom	3 Apr 2003 01:05:15 -0000
@@ -18,7 +18,7 @@
 
 		static public void SetCustomHandler (Glade.XMLCustomWidgetHandler handler)
 		{
-			callback_wrapper = new GladeSharp.XMLCustomWidgetHandlerWrapper (handler);
+			callback_wrapper = new GladeSharp.XMLCustomWidgetHandlerWrapper (handler, null);
 			glade_set_custom_handler (callback_wrapper.NativeDelegate, IntPtr.Zero);
 		}
 
Index: glib/DelegateWrapper.cs
===================================================================
RCS file: /cvs/public/gtk-sharp/glib/DelegateWrapper.cs,v
retrieving revision 1.1
diff -u -r1.1 DelegateWrapper.cs
--- glib/DelegateWrapper.cs	29 Aug 2002 21:45:07 -0000	1.1
+++ glib/DelegateWrapper.cs	3 Apr 2003 01:05:15 -0000
@@ -17,10 +17,29 @@
 	///	Wrapper class for delegates.
 	/// </remarks>
 
-	public class DelegateWrapper {
-		static ArrayList _instances = new ArrayList ();
+	public class DelegateWrapper
+	{
+		static Hashtable weakReferences = new Hashtable ();
+		
+		protected DelegateWrapper (object o)
+		{
+			if (o == null)
+				o = this; // Never expires. Used in static methods.
 
-		protected DelegateWrapper () {
+			weakReferences [this] = new WeakReference (o);
+		}
+
+		protected bool RemoveIfNotAlive ()
+		{
+			WeakReference r = null;
+			r = weakReferences [this] as WeakReference;
+			if (r != null && !r.IsAlive) {
+				weakReferences.Remove (this);
+				r = null;
+			}
+
+			return (r == null);
 		}
 	}
 }
+
Index: gtk/Clipboard.custom
===================================================================
RCS file: /cvs/public/gtk-sharp/gtk/Clipboard.custom,v
retrieving revision 1.4
diff -u -r1.4 Clipboard.custom
--- gtk/Clipboard.custom	24 Feb 2003 03:13:08 -0000	1.4
+++ gtk/Clipboard.custom	3 Apr 2003 01:05:15 -0000
@@ -32,8 +32,8 @@
 				clipboard_objects[this_id] = data;
 			}
 
-			get_func_wrapper = new GtkSharp.GtkClipboardGetFuncWrapper (get_func);
-			clear_func_wrapper = new GtkSharp.GtkClipboardClearFuncWrapper (clear_func);
+			get_func_wrapper = new GtkSharp.GtkClipboardGetFuncWrapper (get_func, this);
+			clear_func_wrapper = new GtkSharp.GtkClipboardClearFuncWrapper (clear_func, this);
 
 			IntPtr list = IntPtr.Zero;
 
Index: gtk/GtkSharp.GtkClipboardClearFuncNative.cs
===================================================================
RCS file: /cvs/public/gtk-sharp/gtk/GtkSharp.GtkClipboardClearFuncNative.cs,v
retrieving revision 1.1
diff -u -r1.1 GtkSharp.GtkClipboardClearFuncNative.cs
--- gtk/GtkSharp.GtkClipboardClearFuncNative.cs	10 Nov 2002 10:03:50 -0000	1.1
+++ gtk/GtkSharp.GtkClipboardClearFuncNative.cs	3 Apr 2003 01:05:15 -0000
@@ -9,6 +9,7 @@
 
 		public void NativeCallback (IntPtr clipboard, uint objid)
 		{
+			if (RemoveIfNotAlive ()) return;
 			object[] _args = new object[2];
 			_args[0] = (Gtk.Clipboard) GLib.Opaque.GetOpaque(clipboard);
 			if (_args[0] == null)
@@ -21,7 +22,7 @@
 		public GtkClipboardClearFuncNative NativeDelegate;
 		protected Gtk.ClipboardClearFunc _managed;
 
-		public GtkClipboardClearFuncWrapper (Gtk.ClipboardClearFunc managed) : base ()
+		public GtkClipboardClearFuncWrapper (Gtk.ClipboardClearFunc managed, object o) : base (o)
 		{
 			NativeDelegate = new GtkClipboardClearFuncNative (NativeCallback);
 			_managed = managed;
Index: gtk/GtkSharp.GtkClipboardGetFuncNative.cs
===================================================================
RCS file: /cvs/public/gtk-sharp/gtk/GtkSharp.GtkClipboardGetFuncNative.cs,v
retrieving revision 1.1
diff -u -r1.1 GtkSharp.GtkClipboardGetFuncNative.cs
--- gtk/GtkSharp.GtkClipboardGetFuncNative.cs	10 Nov 2002 10:03:50 -0000	1.1
+++ gtk/GtkSharp.GtkClipboardGetFuncNative.cs	3 Apr 2003 01:05:15 -0000
@@ -9,6 +9,7 @@
 
 		public void NativeCallback (IntPtr clipboard, ref Gtk.SelectionData selection_data, uint info, uint obj_id)
 		{
+			if (RemoveIfNotAlive ()) return;
 			object[] _args = new object[4];
 			_args[0] = (Gtk.Clipboard) GLib.Opaque.GetOpaque(clipboard);
 			if (_args[0] == null)
@@ -23,7 +24,7 @@
 		public GtkClipboardGetFuncNative NativeDelegate;
 		protected Gtk.ClipboardGetFunc _managed;
 
-		public GtkClipboardGetFuncWrapper (Gtk.ClipboardGetFunc managed) : base ()
+		public GtkClipboardGetFuncWrapper (Gtk.ClipboardGetFunc managed, object o) : base (o)
 		{
 			NativeDelegate = new GtkClipboardGetFuncNative (NativeCallback);
 			_managed = managed;

--=-Mrp6aQ918sheyF9N1qy7--