agent: Fix a leak in nice_output_stream_write() with a NULL cancellable
authorPhilip Withnall <philip.withnall@collabora.co.uk>
Tue, 15 Apr 2014 17:06:44 +0000 (18:06 +0100)
committerPhilip Withnall <philip.withnall@collabora.co.uk>
Tue, 15 Apr 2014 17:09:08 +0000 (18:09 +0100)
If @cancellable is NULL in a call to nice_output_stream_write(), the
WriteData struct is created with a reference count of 4, but only two
operations are scheduled which will result in its reference count being
decremented. The third operation is only scheduled if @cancellable is
non-NULL (and the final reference is dropped unconditionally at the end
of the function).

Fix this by properly implementing reference counting for WriteData,
rather than hard-coding the expected number of references in a fragile
and unmaintainable way.

agent/outputstream.c

index c2e0211..74b567b 100644 (file)
@@ -314,6 +314,13 @@ typedef struct {
   gboolean cancelled;
 } WriteData;
 
+static WriteData *
+write_data_ref (WriteData *write_data)
+{
+  g_atomic_int_inc (&write_data->ref_count);
+  return write_data;
+}
+
 static void
 write_data_unref (WriteData *write_data)
 {
@@ -384,26 +391,25 @@ nice_output_stream_write (GOutputStream *stream, const void *buffer, gsize count
    * GCond solution; would be much better for nice_agent_send() to block
    * properly in the main loop. */
   write_data = g_slice_new0 (WriteData);
-  g_atomic_int_set (&write_data->ref_count, 4);
-
+  write_data->ref_count = 1;
   g_mutex_init (&write_data->mutex);
   g_cond_init (&write_data->cond);
 
   if (cancellable != NULL) {
     cancel_id = g_cancellable_connect (cancellable,
-        (GCallback) write_cancelled_cb, write_data,
+        (GCallback) write_cancelled_cb, write_data_ref (write_data),
         (GDestroyNotify) write_data_unref);
   }
 
   closed_cancel_id = g_cancellable_connect (self->priv->closed_cancellable,
-      (GCallback) write_cancelled_cb, write_data,
+      (GCallback) write_cancelled_cb, write_data_ref (write_data),
       (GDestroyNotify) write_data_unref);
 
   g_mutex_lock (&write_data->mutex);
 
   writeable_id = g_signal_connect_data (G_OBJECT (agent),
       "reliable-transport-writable",
-      (GCallback) reliable_transport_writeable_cb, write_data,
+      (GCallback) reliable_transport_writeable_cb, write_data_ref (write_data),
       (GClosureNotify) write_data_unref, 0);