2005-03-08 Joe Shaw <joeshaw@novell.com>

	Fix a bunch of lifecycle and memory management problems
	in the mono bindings.

	* mono/Arguments.cs (Arguments): Implement IDisposable

	* mono/Bus.cs (Bus): Don't allow public instantiation.  This is
	strictly a static class.

	* mono/Connection.cs: Move the DBusObjectPathVTable and associated
	delegates into this file.
	(Connection): Implement IDisposable.
	(Dispose): Disconnect the connection and set the raw connection
	pointer to IntPtr.Zero.
	(~Connection): Call Dispose().
	(RegisterObjectPath): Added.  Manages the registration of object
	paths so we can cleanly disconnect them at dispose/finalize time.
	(UnregisterObjectPath): Ditto.
	(set_RawConnection): Unregister all of the object paths when
	changing the underlying DBusConnection.  Add them back onto the
	new connection, if any.

	* mono/Handler.cs: Don't implement IDisposable; it doesn't use any
	more unmanaged resources anymore, so it's not necessary.  Move all
	the DBusObjectPathVTable stuff out of here.
	(Handler): Save references to our delegates so that they don't get
	finalized.  Call Connection.RegisterObjectPath() instead of
	dbus_connection_register_object_path() directly.
	(Message_Called): Dispose the message after we're finished with
	it.

	* mono/Message.cs (Message): Implement IDisposable.
	(Dispose): Dispose the Arguments, and set the RawMessage to
	IntPtr.Zero.
	(SendWithReplyAndBlock): We own the ref to the reply that comes
	back from dbus_connection_send_with_reply_and_block() so add a
	comment about that and unref it after we've constructed a managed
	MethodReturn class around it.  Fixes a big, big leak.

	* mono/ProxyBuilder.cs: Reflect into Message to get the Dispose
	method.
	(BuildSignalHandler): After we've sent the Signal message, dispose
	of it.
	(BuildMethod): Dispose of the method call and reply messages after
	we've sent the message and extracted the data we want from the
	reply.

	* mono/Service.cs (UnregisterObject): Don't call handler.Dispose()
	anymore.
	(Service_FilterCalled): Dispose of the message after we're
	finished with it.

8 files changed, 217 insertions, 111 deletions

diff --git a/ChangeLog b/ChangeLog
index 91153314..844b73ca 100644
--- a/ChangeLog
+++ b/ChangeLog
@@ -1,5 +1,58 @@
 2005-03-08  Joe Shaw  <joeshaw@novell.com>
-                                                                                
+
+	Fix a bunch of lifecycle and memory management problems
+	in the mono bindings.
+
+	* mono/Arguments.cs (Arguments): Implement IDisposable
+
+	* mono/Bus.cs (Bus): Don't allow public instantiation.  This is
+	strictly a static class.
+
+	* mono/Connection.cs: Move the DBusObjectPathVTable and associated
+	delegates into this file.
+	(Connection): Implement IDisposable.
+	(Dispose): Disconnect the connection and set the raw connection
+	pointer to IntPtr.Zero.
+	(~Connection): Call Dispose().
+	(RegisterObjectPath): Added.  Manages the registration of object
+	paths so we can cleanly disconnect them at dispose/finalize time.
+	(UnregisterObjectPath): Ditto.
+	(set_RawConnection): Unregister all of the object paths when
+	changing the underlying DBusConnection.  Add them back onto the
+	new connection, if any.
+
+	* mono/Handler.cs: Don't implement IDisposable; it doesn't use any
+	more unmanaged resources anymore, so it's not necessary.  Move all
+	the DBusObjectPathVTable stuff out of here.
+	(Handler): Save references to our delegates so that they don't get
+	finalized.  Call Connection.RegisterObjectPath() instead of
+	dbus_connection_register_object_path() directly.
+	(Message_Called): Dispose the message after we're finished with
+	it.
+
+	* mono/Message.cs (Message): Implement IDisposable.
+	(Dispose): Dispose the Arguments, and set the RawMessage to
+	IntPtr.Zero.
+	(SendWithReplyAndBlock): We own the ref to the reply that comes
+	back from dbus_connection_send_with_reply_and_block() so add a
+	comment about that and unref it after we've constructed a managed
+	MethodReturn class around it.  Fixes a big, big leak.
+
+	* mono/ProxyBuilder.cs: Reflect into Message to get the Dispose
+	method.
+	(BuildSignalHandler): After we've sent the Signal message, dispose
+	of it.
+	(BuildMethod): Dispose of the method call and reply messages after
+	we've sent the message and extracted the data we want from the
+	reply.
+
+	* mono/Service.cs (UnregisterObject): Don't call handler.Dispose()
+	anymore.
+	(Service_FilterCalled): Dispose of the message after we're
+	finished with it.
+
+2005-03-08  Joe Shaw  <joeshaw@novell.com>
+
         * dbus/dbus-connection.c (dbus_connection_send_with_reply):
         After we attach our pending call to the connection, unref
         it.  Fixes a leak.
diff --git a/mono/Arguments.cs b/mono/Arguments.cs
index 41e6d15d..61ae443f 100644
--- a/mono/Arguments.cs
+++ b/mono/Arguments.cs
@@ -7,29 +7,37 @@ namespace DBus
 {
   // Holds the arguments of a message. Provides methods for appending
   // arguments and to assist in matching .NET types with D-BUS types.
-  public class Arguments : IEnumerable
+	public class Arguments : IEnumerable, IDisposable
   {
     // Must follow sizeof(DBusMessageIter)
     internal const int DBusMessageIterSize = 14*4;
     private static Hashtable dbusTypes = null;
     private Message message;
-    private IntPtr appenderIter = Marshal.AllocCoTaskMem(DBusMessageIterSize);
+    private IntPtr appenderIter;
     private IEnumerator enumerator = null;
     
-    internal Arguments()
+    internal Arguments (Message message)
     {
+      this.appenderIter = Marshal.AllocCoTaskMem(DBusMessageIterSize);
+      this.message = message;
     }
 
-    ~Arguments()
+    private void Dispose (bool disposing)
     {
       Marshal.FreeCoTaskMem(appenderIter);
     }
 
-    internal Arguments(Message message)
+    public void Dispose ()
     {
-      this.message = message;
+      Dispose (true);
+      GC.SuppressFinalize (this);
     }
-    
+
+    ~Arguments()
+    {
+      Dispose (false);
+    }
+
     // Checks the suitability of a D-BUS type for supporting a .NET
     // type.
     public static bool Suits(Type dbusType, Type type) 
diff --git a/mono/Bus.cs b/mono/Bus.cs
index 963e8195..05cf24c2 100644
--- a/mono/Bus.cs
+++ b/mono/Bus.cs
@@ -14,6 +14,9 @@ namespace DBus
       Activation = 2
     }
 
+    // Don't allow instantiation
+    private Bus () { }
+
     public static Connection GetSessionBus() 
     {
       return GetBus(BusType.Session);
diff --git a/mono/Connection.cs b/mono/Connection.cs
index 6abd7e80..af0764db 100644
--- a/mono/Connection.cs
+++ b/mono/Connection.cs
@@ -12,7 +12,36 @@ namespace DBus
 						 IntPtr rawMessage,
 						 IntPtr userData);
 
-  public class Connection 
+  internal delegate void DBusObjectPathUnregisterFunction(IntPtr rawConnection,
+							  IntPtr userData);
+
+  internal delegate int DBusObjectPathMessageFunction(IntPtr rawConnection,
+						      IntPtr rawMessage,
+						      IntPtr userData);
+
+  [StructLayout (LayoutKind.Sequential)]
+  internal struct DBusObjectPathVTable
+  {
+    public DBusObjectPathUnregisterFunction unregisterFunction;
+    public DBusObjectPathMessageFunction messageFunction;
+    public IntPtr padding1;
+    public IntPtr padding2;
+    public IntPtr padding3;
+    public IntPtr padding4;
+    
+    public DBusObjectPathVTable(DBusObjectPathUnregisterFunction unregisterFunction,
+				DBusObjectPathMessageFunction messageFunction) 
+    {
+      this.unregisterFunction = unregisterFunction;
+      this.messageFunction = messageFunction;
+      this.padding1 = IntPtr.Zero;
+      this.padding2 = IntPtr.Zero;
+      this.padding3 = IntPtr.Zero;
+      this.padding4 = IntPtr.Zero;
+    }
+  }
+
+  public class Connection : IDisposable
   {
     /// <summary>
     /// A pointer to the underlying Connection structure
@@ -26,8 +55,9 @@ namespace DBus
     
     private int timeout = -1;
 
-    private ArrayList filters = new ArrayList (); // of DBusHandleMessageFunction
-    private ArrayList matches = new ArrayList (); // of string
+    private ArrayList filters = new ArrayList ();      // of DBusHandleMessageFunction
+    private ArrayList matches = new ArrayList ();      // of string
+    private Hashtable object_paths = new Hashtable (); // key: string  value: DBusObjectPathVTable
 
     internal Connection(IntPtr rawConnection)
     {
@@ -49,6 +79,22 @@ namespace DBus
       SetupWithMain();
     }
 
+    public void Dispose() 
+    {
+      Dispose(true);
+      GC.SuppressFinalize(this);
+    }
+    
+    public void Dispose (bool disposing) 
+    {
+      if (disposing && RawConnection != IntPtr.Zero) 
+	{
+	  dbus_connection_disconnect(rawConnection);
+
+	  RawConnection = IntPtr.Zero; // free the native object
+	}
+    }
+
     public void Flush()
     {
       dbus_connection_flush(RawConnection);
@@ -61,17 +107,7 @@ namespace DBus
     
     ~Connection () 
     {
-      if (RawConnection != IntPtr.Zero) 
-	{
-          foreach (DBusHandleMessageFunction func in this.filters)
-            RemoveFilter (func);
-
-          foreach (string match_rule in this.matches)
-            RemoveMatch (match_rule);
-
-	  dbus_connection_disconnect(rawConnection);
-	}
-      RawConnection = IntPtr.Zero; // free the native object
+      Dispose (false);
     }
     
     internal static Connection Wrap(IntPtr rawConnection) 
@@ -121,6 +157,22 @@ namespace DBus
       this.matches.Remove (match_rule);
     }
 
+    internal void RegisterObjectPath (string path, DBusObjectPathVTable vtable)
+    {
+      if (!dbus_connection_register_object_path (RawConnection, path, ref vtable, IntPtr.Zero))
+        throw new OutOfMemoryException ();
+ 
+      this.object_paths[path] = vtable;
+    }
+ 
+    internal void UnregisterObjectPath (string path)
+    {
+      dbus_connection_unregister_object_path (RawConnection, path);
+ 
+      this.object_paths.Remove (path);
+    }
+
+
     public string UniqueName
     {
       get
@@ -178,6 +230,9 @@ namespace DBus
               foreach (string match_rule in this.matches)
                 dbus_bus_remove_match (rawConnection, match_rule, IntPtr.Zero);
 
+              foreach (string path in this.object_paths.Keys)
+                dbus_connection_unregister_object_path (rawConnection, path);
+
 	      // Get the reference to this
 	      IntPtr rawThis = dbus_connection_get_data (rawConnection, Slot);
 	      Debug.Assert (rawThis != IntPtr.Zero);
@@ -211,11 +266,17 @@ namespace DBus
 
               foreach (string match_rule in this.matches)
                 dbus_bus_add_match (rawConnection, match_rule, IntPtr.Zero);
+
+              foreach (string path in this.object_paths.Keys) {
+                DBusObjectPathVTable vtable = (DBusObjectPathVTable) this.object_paths[path];
+                dbus_connection_register_object_path (rawConnection, path, ref vtable, IntPtr.Zero);
+	      }
 	    }
 	  else
 	    {
 	      this.filters.Clear ();
               this.matches.Clear ();
+	      this.object_paths.Clear ();
 	    }
 	}
     }
@@ -278,5 +339,16 @@ namespace DBus
     private extern static void dbus_bus_remove_match(IntPtr rawConnection,
 						     string rule,
 						     IntPtr erro);
+
+    [DllImport ("dbus-1")]
+    private extern static bool dbus_connection_register_object_path (IntPtr rawConnection,
+								     string path,
+								     ref DBusObjectPathVTable vTable,
+								     IntPtr userData);
+
+    [DllImport ("dbus-1")]
+    private extern static void dbus_connection_unregister_object_path (IntPtr rawConnection,
+								       string path);
+
   }
 }
diff --git a/mono/Handler.cs b/mono/Handler.cs
index 9696a4d3..87092f90 100644
--- a/mono/Handler.cs
+++ b/mono/Handler.cs
@@ -13,7 +13,7 @@ namespace DBus
     NeedMemory = 2
   }
 
-  internal class Handler : IDisposable
+  internal class Handler
   {
     private string path = null;
     private Introspector introspector = null;
@@ -21,70 +21,12 @@ namespace DBus
     private DBusObjectPathVTable vTable;
     private Connection connection;
     private Service service;
-    private bool disposed = false;
-    
-    internal delegate void DBusObjectPathUnregisterFunction(IntPtr rawConnection,
-							    IntPtr userData);
-
-    internal delegate int DBusObjectPathMessageFunction(IntPtr rawConnection,
-							IntPtr rawMessage,
-							IntPtr userData);
-
-    [StructLayout (LayoutKind.Sequential)]
-    private struct DBusObjectPathVTable 
-    {
-      public DBusObjectPathUnregisterFunction unregisterFunction;
-      public DBusObjectPathMessageFunction messageFunction;
-      public IntPtr padding1;
-      public IntPtr padding2;
-      public IntPtr padding3;
-      public IntPtr padding4;
-    
-      public DBusObjectPathVTable(DBusObjectPathUnregisterFunction unregisterFunction,
-				  DBusObjectPathMessageFunction messageFunction) 
-      {
-	this.unregisterFunction = unregisterFunction;
-	this.messageFunction = messageFunction;
-	this.padding1 = IntPtr.Zero;
-	this.padding2 = IntPtr.Zero;
-	this.padding3 = IntPtr.Zero;
-	this.padding4 = IntPtr.Zero;
-      }
-    }
-
-    public void Dispose() 
-    {
-      Dispose(true);
-      GC.SuppressFinalize(this);
-    }
-    
-    private void Dispose(bool disposing) 
-    {
-      if (!disposed) {	
-	if (disposing) {
-	  // Clean up managed resources
-	}
-
-	service = null;
 
-	// Clean up unmanaged resources
-	if (Connection != null && Connection.RawConnection != IntPtr.Zero && Path != null) {
-	  dbus_connection_unregister_object_path(Connection.RawConnection,
-						 Path);
-	}	
-
-	connection = null;
-	introspector = null;
-	handledObject = null;
-      }
-      
-      disposed = true;    
-    }
-
-    ~Handler() 
-    {
-      Dispose(false);
-    }
+    // We need to hold extra references to these callbacks so that they don't
+    // get garbage collected before they are called back into from unmanaged
+    // code.
+    private DBusObjectPathUnregisterFunction unregister_func;
+    private DBusObjectPathMessageFunction message_func;
 
     public Handler(object handledObject, 
 		   string path, 
@@ -96,15 +38,11 @@ namespace DBus
       this.path = path;
       
       // Create the vTable and register the path
-      vTable = new DBusObjectPathVTable(new DBusObjectPathUnregisterFunction(Unregister_Called), 
-					new DBusObjectPathMessageFunction(Message_Called));
-      
-      if (!dbus_connection_register_object_path(Connection.RawConnection,
-						Path,
-						ref vTable,
-						IntPtr.Zero))
-	throw new OutOfMemoryException();
+      this.unregister_func = new DBusObjectPathUnregisterFunction (Unregister_Called);
+      this.message_func = new DBusObjectPathMessageFunction (Message_Called);
 
+      vTable = new DBusObjectPathVTable (this.unregister_func, this.message_func);
+      Connection.RegisterObjectPath (Path, vTable);
       RegisterSignalHandlers();
     }
 
@@ -131,8 +69,6 @@ namespace DBus
       set {
 	this.handledObject = value;
 	
-	object[] attributes;
-	
 	// Register the methods
 	this.introspector = Introspector.GetIntrospector(value.GetType());	  
       }
@@ -153,17 +89,22 @@ namespace DBus
 			       IntPtr userData) 
     {
       Message message = Message.Wrap(rawMessage, Service);
+      Result res = Result.NotYetHandled;
 
       switch (message.Type) {
+      case Message.MessageType.MethodCall:
+        res = HandleMethod ((MethodCall) message);
+	break;
+
       case Message.MessageType.Signal:
 	// We're not interested in signals here because we're the ones
 	// that generate them!
 	break;
-      case Message.MessageType.MethodCall:
-	return (int) HandleMethod((MethodCall) message);
       }
 
-      return (int) Result.NotYetHandled;
+      message.Dispose ();
+
+      return (int) res;
     }
     
     private Result HandleMethod(MethodCall methodCall)
@@ -227,12 +168,5 @@ namespace DBus
 	  this.service = value;
 	}
     }
-
-    [DllImport ("dbus-1")]
-    private extern static bool dbus_connection_register_object_path (IntPtr rawConnection, string path, ref DBusObjectPathVTable vTable, IntPtr userData);
-
-    [DllImport ("dbus-1")]
-    private extern static void dbus_connection_unregister_object_path (IntPtr rawConnection, string path);
-
   }
 }
diff --git a/mono/Message.cs b/mono/Message.cs
index 5aa3542f..944e3f92 100644
--- a/mono/Message.cs
+++ b/mono/Message.cs
@@ -6,7 +6,7 @@ namespace DBus
   using System.Diagnostics;
   using System.Collections;
   
-  public class Message 
+  public class Message : IDisposable
   {
     private static Stack stack = new Stack ();
 	  
@@ -83,10 +83,26 @@ namespace DBus
     {
       this.service = service;
     }
+
+    public void Dispose() 
+    {
+      Dispose(true);
+      GC.SuppressFinalize(this);
+    }
     
-    ~Message() 
+    public void Dispose (bool disposing) 
     {
+      if (disposing) {
+        if (this.arguments != null)
+	  this.arguments.Dispose ();
+      }
+
       RawMessage = IntPtr.Zero; // free the native object
+    }     
+
+    ~Message() 
+    {
+      Dispose (false);
     }
     
     public static Message Wrap(IntPtr rawMessage, Service service) 
@@ -198,6 +214,12 @@ namespace DBus
 
       if (rawMessage != IntPtr.Zero) {
 	MethodReturn methodReturn = new MethodReturn(rawMessage, Service);
+	// Ownership of a ref is passed onto us from
+	// dbus_connection_send_with_reply_and_block().  It gets reffed as
+	// a result of being passed into the MethodReturn ctor, so unref
+	// the extra one here.
+	dbus_message_unref (rawMessage);
+
 	return methodReturn;
       } else {
 	throw new DBusException(error);
diff --git a/mono/ProxyBuilder.cs b/mono/ProxyBuilder.cs
index 80449093..fefac473 100644
--- a/mono/ProxyBuilder.cs
+++ b/mono/ProxyBuilder.cs
@@ -35,6 +35,8 @@ namespace DBus
 											     new Type[0]);
     private static MethodInfo Message_SendMI = typeof(Message).GetMethod("Send",
 									 new Type[0]);
+    private static MethodInfo Message_DisposeMI = typeof(Message).GetMethod("Dispose",
+									    new Type[0]);
     private static MethodInfo Arguments_GetEnumeratorMI = typeof(Arguments).GetMethod("GetEnumerator",
 											  new Type[0]);
     private static MethodInfo IEnumerator_MoveNextMI = typeof(System.Collections.IEnumerator).GetMethod("MoveNext",
@@ -197,7 +199,6 @@ namespace DBus
       // Generate the locals
       LocalBuilder methodCallL = generator.DeclareLocal(typeof(MethodCall));
       methodCallL.SetLocalSymInfo("signal");
-      LocalBuilder replyL = generator.DeclareLocal(typeof(MethodReturn));
 
       //generator.EmitWriteLine("Signal signal = new Signal(...)");
       generator.Emit(OpCodes.Ldsfld, serviceF);
@@ -224,6 +225,10 @@ namespace DBus
       generator.Emit(OpCodes.Ldloc_0);
       generator.EmitCall(OpCodes.Callvirt, Message_SendMI, null); 
 
+      //generator.EmitWriteLine("signal.Dispose()");
+      generator.Emit(OpCodes.Ldloc_0);
+      generator.EmitCall(OpCodes.Callvirt, Message_DisposeMI, null);
+
       //generator.EmitWriteLine("return");
       generator.Emit(OpCodes.Ret);
     }
@@ -310,6 +315,15 @@ namespace DBus
 	}
       }
 
+      // Clean up after ourselves
+      //generator.EmitWriteLine("methodCall.Dispose()");
+      generator.Emit(OpCodes.Ldloc_0);
+      generator.EmitCall(OpCodes.Callvirt, Message_DisposeMI, null);
+
+      //generator.EmitWriteLine("reply.Dispose()");
+      generator.Emit(OpCodes.Ldloc_1);
+      generator.EmitCall(OpCodes.Callvirt, Message_DisposeMI, null);
+
       if (method.ReturnType != typeof(void)) {
 	generator.Emit(OpCodes.Ldloc_3);
       }
diff --git a/mono/Service.cs b/mono/Service.cs
index 4280c6b3..40703a53 100644
--- a/mono/Service.cs
+++ b/mono/Service.cs
@@ -75,9 +75,7 @@ namespace DBus
 
     public void UnregisterObject(object handledObject) 
     {
-      Handler handler = (Handler) registeredHandlers[handledObject];
       registeredHandlers.Remove(handledObject);
-      handler.Dispose();
     }
 
     public void RegisterObject(object handledObject, 
@@ -128,6 +126,8 @@ namespace DBus
 	}
       }
       
+      message.Dispose ();
+
       return (int) Result.NotYetHandled;
     }