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@citrix.com> --- 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); }