bus: change GstBusSource to hold a weak ref to GstBus

When holding a regular ref it will cause the GstBus to never
reach 0 references and it won't be destroyed unless the application
explicitly calls gst_bus_remove_signal_watch().

Switching to weakref will allow the GstBus to be destroyed.
The application is still responsible for destroying the
GSource.

https://bugzilla.gnome.org/show_bug.cgi?id=762552

diff --git a/gst/gstbus.c b/gst/gstbus.c
index 9936290..237507f 100644 (file)
--- a/gst/gstbus.c
+++ b/gst/gstbus.c
@@ -741,7 +741,7 @@ no_replace:
 typedef struct
 {
   GSource source;
-  GstBus *bus;
+  GWeakRef bus_ref;
 } GstBusSource;
 
 static gboolean
@@ -755,8 +755,16 @@ static gboolean
 gst_bus_source_check (GSource * source)
 {
   GstBusSource *bsrc = (GstBusSource *) source;
+  GstBus *bus;
+  gboolean ret = FALSE;
+
+  bus = g_weak_ref_get (&bsrc->bus_ref);
+  if (bus) {
+    ret = bus->priv->pollfd.revents & (G_IO_IN | G_IO_HUP | G_IO_ERR);
+    g_object_unref (bus);
+  }
 
-  return bsrc->bus->priv->pollfd.revents & (G_IO_IN | G_IO_HUP | G_IO_ERR);
+  return ret;
 }
 
 static gboolean
@@ -766,32 +774,39 @@ gst_bus_source_dispatch (GSource * source, GSourceFunc callback,
   GstBusFunc handler = (GstBusFunc) callback;
   GstBusSource *bsource = (GstBusSource *) source;
   GstMessage *message;
-  gboolean keep;
+  gboolean keep = TRUE;
   GstBus *bus;
 
   g_return_val_if_fail (bsource != NULL, FALSE);
 
-  bus = bsource->bus;
+  bus = g_weak_ref_get (&bsource->bus_ref);
 
-  g_return_val_if_fail (GST_IS_BUS (bus), FALSE);
+  if (bus) {
+    g_return_val_if_fail (GST_IS_BUS (bus), FALSE);
 
-  message = gst_bus_pop (bus);
+    message = gst_bus_pop (bus);
 
-  /* The message queue might be empty if some other thread or callback set
-   * the bus to flushing between check/prepare and dispatch */
-  if (G_UNLIKELY (message == NULL))
-    return TRUE;
+    /* The message queue might be empty if some other thread or callback set
+     * the bus to flushing between check/prepare and dispatch */
+    if (G_UNLIKELY (message == NULL))
+      return TRUE;
 
-  if (!handler)
-    goto no_handler;
+    if (!handler)
+      goto no_handler;
 
-  GST_DEBUG_OBJECT (bus, "source %p calling dispatch with %" GST_PTR_FORMAT,
-      source, message);
+    GST_DEBUG_OBJECT (bus, "source %p calling dispatch with %" GST_PTR_FORMAT,
+        source, message);
 
-  keep = handler (bus, message, user_data);
-  gst_message_unref (message);
+    keep = handler (bus, message, user_data);
+    gst_message_unref (message);
 
-  GST_DEBUG_OBJECT (bus, "source %p handler returns %d", source, keep);
+    GST_DEBUG_OBJECT (bus, "source %p handler returns %d", source, keep);
+    g_object_unref (bus);
+  } else {
+    GST_WARNING ("GstBusSource without a bus and still attached to a context."
+        " The application is responsible for removing the GstBus"
+        " watch when it isn't needed anymore.");
+  }
 
   return keep;
 
@@ -810,17 +825,19 @@ gst_bus_source_finalize (GSource * source)
   GstBusSource *bsource = (GstBusSource *) source;
   GstBus *bus;
 
-  bus = bsource->bus;
+  bus = g_weak_ref_get (&bsource->bus_ref);
 
-  GST_DEBUG_OBJECT (bus, "finalize source %p", source);
+  if (bus) {
+    GST_DEBUG_OBJECT (bus, "finalize source %p", source);
 
-  GST_OBJECT_LOCK (bus);
-  if (bus->priv->signal_watch == source)
-    bus->priv->signal_watch = NULL;
-  GST_OBJECT_UNLOCK (bus);
+    GST_OBJECT_LOCK (bus);
+    if (bus->priv->signal_watch == source)
+      bus->priv->signal_watch = NULL;
+    GST_OBJECT_UNLOCK (bus);
 
-  gst_object_unref (bsource->bus);
-  bsource->bus = NULL;
+    g_object_unref (bus);
+  }
+  g_weak_ref_clear (&bsource->bus_ref);
 }
 
 static GSourceFuncs gst_bus_source_funcs = {
@@ -853,7 +870,7 @@ gst_bus_create_watch (GstBus * bus)
 
   g_source_set_name ((GSource *) source, "GStreamer message bus watch");
 
-  source->bus = gst_object_ref (bus);
+  g_weak_ref_init (&source->bus_ref, (GObject *) bus);
   g_source_add_poll ((GSource *) source, &bus->priv->pollfd);
 
   return (GSource *) source;

diff --git a/tests/check/gst/gstpipeline.c b/tests/check/gst/gstpipeline.c
index d186bc7..68a21a9 100644 (file)
--- a/tests/check/gst/gstpipeline.c
+++ b/tests/check/gst/gstpipeline.c
@@ -200,7 +200,7 @@ GST_START_TEST (test_bus)
 
   id = gst_bus_add_watch (bus, message_received, pipeline);
   ASSERT_OBJECT_REFCOUNT (pipeline, "pipeline after add_watch", 1);
-  ASSERT_OBJECT_REFCOUNT (bus, "bus after add_watch", 3);
+  ASSERT_OBJECT_REFCOUNT (bus, "bus after add_watch", 2);
 
   ret = gst_element_set_state (pipeline, GST_STATE_PLAYING);
   fail_unless (ret == GST_STATE_CHANGE_ASYNC);
@@ -225,7 +225,7 @@ GST_START_TEST (test_bus)
   fail_unless (current == GST_STATE_NULL, "state is not NULL but %d", current);
 
   ASSERT_OBJECT_REFCOUNT (pipeline, "pipeline at start of cleanup", 1);
-  ASSERT_OBJECT_REFCOUNT (bus, "bus at start of cleanup", 3);
+  ASSERT_OBJECT_REFCOUNT (bus, "bus at start of cleanup", 2);
 
   fail_unless (g_source_remove (id));
   ASSERT_OBJECT_REFCOUNT (bus, "bus after removing source", 2);