On 06.12.2016 15:39, Cédric Bosdonnat wrote:

If the monitor doesn't hold a reference to the domain object the object may be destroyed before the monitor actually stops. --- src/lxc/lxc_monitor.c | 3 +++ 1 file changed, 3 insertions(+)

diff --git a/src/lxc/lxc_monitor.c b/src/lxc/lxc_monitor.c index d828d52..de63f9e 100644 --- a/src/lxc/lxc_monitor.c +++ b/src/lxc/lxc_monitor.c @@ -179,6 +179,7 @@ virLXCMonitorPtr virLXCMonitorNew(virDomainObjPtr vm, memcpy(&mon->cb, cb, sizeof(mon->cb));

virObjectRef(mon); + virObjectRef(vm);

You can move this a few lines above: mon->vm = virObjectRef(vm); if you want.

virNetClientSetCloseCallback(mon->client, virLXCMonitorEOFNotify, mon, virLXCMonitorCloseFreeCallback);

@@ -188,6 +189,7 @@ virLXCMonitorPtr virLXCMonitorNew(virDomainObjPtr vm,

error: virObjectUnref(mon); + virObjectUnref(vm);

This doesn't feel right. Imagine something bad happened before @vm got ref'd (the first hunk). The control jumps over to error label and unref @vm even though it hasn't been ref'd yet. Or worse - @mon has exactly one reference (we are creating it here in this function), therefore the above unref(mon) causes the MonitorDispose callback to be called, which unrefs the @vm again. Fortunately, this scenario is currently impossible as there's no 'goto error' after @mon->vm is set, but you get my point.

mon = NULL; goto cleanup; } @@ -201,6 +203,7 @@ static void virLXCMonitorDispose(void *opaque) if (mon->cb.destroy) (mon->cb.destroy)(mon, mon->vm); virObjectUnref(mon->program); + virObjectUnref(mon->vm); }

ACK if you drop the 2nd hunk. Michal