[patch] dbus_connection_send_with_reply() leaking DBusPendingCalls

[Timo Teräs]

This is a multi-part message in MIME format.
--------------060501080503060300070401
Content-Type: text/plain; charset=us-ascii; format=flowed
Content-Transfer-Encoding: 7bit

Hi all.

I have implemented a small test server/client using the pending call 
system for getting replies. Then I valgrinded it and it seemed to have 
some odd memory leaks so I started digging.

It seems that the DBusPendingCall -struct isn't initialized properly in 
dbus_connection_send_with_reply(). It is first allocated using 
_dbus_pending_call_new (refcount=1). Then it is attached to the 
onnection using _dbus_connection_attach_pending_call_unlocked() which 
increases the refcount to 2. And finally if the DBusPendingCall is 
returned to caller the refcount is yet increased (to 3).

When the pending call is removed it's refcount is only decreased by one 
in free_pending_call_on_hash_removal(). Plus the caller should unref 
once if it received the pointer. So the DBusPendingCalls allocated in 
dbus_connection_send_with_reply() doesn't get freed.

See attached patch pending_call_fix.diff.


I've also been using dbus-send and noticed it lacks support to send 
boolean arguments. It was also a bit weird that --print-reply doesn't 
set the message type to method call since you propably won't otherwise 
get any reply. A small patch for those is as follows:

See attached patch dbus_send.diff.


Cheers,
   Timo

--------------060501080503060300070401
Content-Type: text/x-patch;
 name="pending_call_fix.diff"
Content-Transfer-Encoding: 7bit
Content-Disposition: inline;
 filename="pending_call_fix.diff"

diff -u -r1.75 dbus-connection.c
--- dbus/dbus-connection.c	2 Dec 2003 10:44:21 -0000	1.75
+++ dbus/dbus-connection.c	15 Feb 2004 16:08:45 -0000
@@ -1787,10 +1787,9 @@
     }
 
   if (pending_return)
-    {
-      dbus_pending_call_ref (pending);
-      *pending_return = pending;
-    }
+    *pending_return = pending;
+  else
+    dbus_pending_call_unref (pending);
 
   CONNECTION_UNLOCK (connection);

--------------060501080503060300070401
Content-Type: text/x-patch;
 name="dbus_send.diff"
Content-Transfer-Encoding: 7bit
Content-Disposition: inline;
 filename="dbus_send.diff"

diff -u -r1.7 dbus-send.c
--- tools/dbus-send.c	10 Oct 2003 02:42:21 -0000	1.7
+++ tools/dbus-send.c	15 Feb 2004 16:08:48 -0000
@@ -64,7 +64,7 @@
       else if (strcmp (arg, "--session") == 0)
 	type = DBUS_BUS_SESSION;
       else if (strcmp (arg, "--print-reply") == 0)
-        print_reply = TRUE;
+        print_reply = TRUE, message_type = DBUS_MESSAGE_TYPE_METHOD_CALL;
       else if (strstr (arg, "--dest=") == arg)
 	dest = strchr (arg, '=') + 1;
       else if (strstr (arg, "--type=") == arg)
@@ -227,6 +227,18 @@
 	  dbus_message_iter_append_string (&iter, c);
 	  break;
 
+	case DBUS_TYPE_BOOLEAN:
+         if (strcasecmp(c, "true") == 0)
+           dbus_message_iter_append_boolean (&iter, TRUE);
+	  else if (strcasecmp(c, "false") == 0)
+           dbus_message_iter_append_boolean (&iter, FALSE);
+	  else
+	    {
+	      fprintf (stderr, "%s: Expected \"true\" or \"false\" instead of \"%s\"\n", argv[0], c);
+	      exit (1);
+	    }
+	  break;
+
 	default:
 	  fprintf (stderr, "%s: Unsupported data type\n", argv[0]);
 	  exit (1);

--------------060501080503060300070401--