Re: [libvirt] [PATCH] lxc: monitor now holds a reference to the domain
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