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
This commit is contained in:
Thiago Santos 2016-02-24 10:56:24 -03:00
parent e953f20786
commit 894c67e642
2 changed files with 45 additions and 28 deletions

View file

@ -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;
return bsrc->bus->priv->pollfd.revents & (G_IO_IN | G_IO_HUP | G_IO_ERR);
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 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;

View file

@ -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);