Hi,
In libvirt-glib, we call virStreamEventAddCallback() and give it a
ref, that is supposed to be unref when the stream event is removed. But
this doesn't happen! I tracked it down to:
static void remoteStreamCallbackFree(void *opaque)
{
struct remoteStreamCallbackData *cbdata = opaque;
if (!cbdata->cb && cbdata->ff)
(cbdata->ff)(cbdata->opaque);
Why are we checking for cbdata->cb here? That gives us a leak
when taking screenshots. So far I solved it
with the attached patch for libvirt-glib. I noticed it because that
of a resulting process & fd leakage in the libvirtd side.
It might be that the fix should be in libvirt, but anyway, the proposed
patch doesn't need a libvirt depedency update and also keeps the
object reference manangement at the libvirt-glib level, which is nice.
--
Marc-André Lureau
From e3ff31c92edd2390def3226647681cc7252c1a1a Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Marc-Andr=C3=A9=20Lureau?= <marcandre.lureau@xxxxxxxxxx>
Date: Thu, 2 Feb 2012 21:22:42 +0100
Subject: [PATCH libvirt-glib] Don't leak GVirStream
Do not give away a reference to virStreamEventAddCallback, but manage
stream life-time and event instead.
It seems to be a bug in current implementation of libvirt
remoteStreamCallbackFree() that doesn't call the free/notify cb for
some reason. (even if it did, I am not sure it would work correctly,
so I prefer that patch)
---
libvirt-gobject/libvirt-gobject-stream.c | 29 ++++++++++++++++++-----------
1 files changed, 18 insertions(+), 11 deletions(-)
diff --git a/libvirt-gobject/libvirt-gobject-stream.c b/libvirt-gobject/libvirt-gobject-stream.c
index a1039b1..5486b95 100644
--- a/libvirt-gobject/libvirt-gobject-stream.c
+++ b/libvirt-gobject/libvirt-gobject-stream.c
@@ -67,6 +67,7 @@ enum {
#define GVIR_STREAM_ERROR gvir_stream_error_quark()
+static void gvir_stream_update_events(GVirStream *stream);
static GQuark
gvir_stream_error_quark(void)
@@ -204,13 +205,6 @@ static void gvir_stream_finalize(GObject *object)
if (self->priv->input_stream)
g_object_unref(self->priv->input_stream);
- if (priv->handle) {
- if (virStreamFinish(priv->handle) < 0)
- g_critical("cannot finish stream");
-
- virStreamFree(priv->handle);
- }
-
tmp = priv->sources;
while (tmp) {
GVirStreamSource *source = tmp->data;
@@ -218,6 +212,16 @@ static void gvir_stream_finalize(GObject *object)
tmp = tmp->next;
}
g_list_free(priv->sources);
+ priv->sources = NULL;
+
+ if (priv->handle) {
+ gvir_stream_update_events(self);
+
+ if (virStreamFinish(priv->handle) < 0)
+ g_critical("cannot finish stream");
+
+ virStreamFree(priv->handle);
+ }
G_OBJECT_CLASS(gvir_stream_parent_class)->finalize(object);
}
@@ -550,8 +554,8 @@ static void gvir_stream_update_events(GVirStream *stream)
} else {
virStreamEventAddCallback(priv->handle, mask,
gvir_stream_handle_events,
- g_object_ref(stream),
- g_object_unref);
+ stream,
+ NULL);
priv->eventRegistered = TRUE;
}
} else {
@@ -600,8 +604,9 @@ static void gvir_stream_source_finalize(GSource *source)
GVirStreamPrivate *priv = gsource->stream->priv;
priv->sources = g_list_remove(priv->sources, source);
-
gvir_stream_update_events(gsource->stream);
+
+ g_clear_object(&gsource->stream);
}
GSourceFuncs gvir_stream_source_funcs = {
@@ -657,12 +662,14 @@ guint gvir_stream_add_watch_full(GVirStream *stream,
gpointer opaque,
GDestroyNotify notify)
{
+ g_return_val_if_fail(GVIR_IS_STREAM(stream), 0);
+
GVirStreamPrivate *priv = stream->priv;
GVirStreamSource *source = (GVirStreamSource*)g_source_new(&gvir_stream_source_funcs,
sizeof(GVirStreamSource));
guint ret;
- source->stream = stream;
+ source->stream = g_object_ref(stream);
source->cond = cond;
if (priority != G_PRIORITY_DEFAULT)
--
1.7.7.6
--
libvir-list mailing list
libvir-list@xxxxxxxxxx
https://www.redhat.com/mailman/listinfo/libvir-list