Frediano Ziglio wrote:
> When creating a timer/event handler reference counting is used. So it could
> be possible (in theory) that libxlDomainObjPrivateFree is called with
> reference counting >1. The problem is that libxlDomainObjPrivateFree leave
> the object in an invalid state with ctx freed (but still having dandling
> pointer). This can lead timer/event handler to core.
>
Thanks for the analysis and the patch!
> This patch destroy the object before disposing it so at timer/event object
> is still valid.
>
I'm not sure that is a good description of the fix :). IMO, it would be
clearer to say
This patch implements a dispose method for libxlDomainObjPrivate, and
moves freeing the libxl ctx to the dispose method, ensuring the ctx is
valid while the object's reference count is > 0.
> Signed-off-by: Frediano Ziglio <frediano.ziglio@xxxxxxxxxx>
> ---
> src/libxl/libxl_driver.c | 20 +++++++++++++++++---
> 1 file changed, 17 insertions(+), 3 deletions(-)
>
> diff --git a/src/libxl/libxl_driver.c b/src/libxl/libxl_driver.c
> index 935919b..1c8cfd7 100644
> --- a/src/libxl/libxl_driver.c
> +++ b/src/libxl/libxl_driver.c
> @@ -110,6 +110,8 @@ static int
> libxlVmStart(libxlDriverPrivatePtr driver, virDomainObjPtr vm,
> bool start_paused, int restore_fd);
>
> +static void libxlDomainObjPrivateDispose(void *obj);
> +
>
libvirt uses the pattern
static void
libxlDomainObjPrivateDispose(void *obj);
I've fixed these small nits and pushed the patch.
Regards,
Jim
> /* Function definitions */
> static int
> libxlDomainObjPrivateOnceInit(void)
> @@ -117,7 +119,7 @@ libxlDomainObjPrivateOnceInit(void)
> if (!(libxlDomainObjPrivateClass = virClassNew(virClassForObjectLockable(),
> "libxlDomainObjPrivate",
> sizeof(libxlDomainObjPrivate),
> - NULL)))
> + libxlDomainObjPrivateDispose)))
> return -1;
>
> return 0;
> @@ -418,14 +420,26 @@ libxlDomainObjPrivateAlloc(void)
> }
>
> static void
> -libxlDomainObjPrivateFree(void *data)
> +libxlDomainObjPrivateDispose(void *obj)
> {
> - libxlDomainObjPrivatePtr priv = data;
> + libxlDomainObjPrivatePtr priv = obj;
>
> if (priv->deathW)
> libxl_evdisable_domain_death(priv->ctx, priv->deathW);
>
> libxl_ctx_free(priv->ctx);
> +}
> +
> +static void
> +libxlDomainObjPrivateFree(void *data)
> +{
> + libxlDomainObjPrivatePtr priv = data;
> +
> + if (priv->deathW) {
> + libxl_evdisable_domain_death(priv->ctx, priv->deathW);
> + priv->deathW = NULL;
> + }
> +
> virObjectUnref(priv);
> }
>
>
--
libvir-list mailing list
libvir-list@xxxxxxxxxx
https://www.redhat.com/mailman/listinfo/libvir-list