[libvirt] [PATCH] lxc: monitor now holds a reference to the domain

[Michal Privoznik]

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