From e555ed6f7b06c8dce6198b6f822f6b4e5fd478d5 Mon Sep 17 00:00:00 2001 From: Erik Skultety Date: Thu, 10 Nov 2016 13:21:29 +0100 Subject: [PATCH] admin: Use the newly introduced close callback handling helpers Use the newly introduced close callback helpers to make the code look just a bit cleaner and more importantly, to fix the following memleak regarding a dangling virAdmConnect object reference caused by assigning NULL to the close callback data once the catch-disconnect routine used the callback followed by a comparison of NULL to the originally defined close callback (which at that moment had already been NULL'd by remoteAdminClientCloseFunc) in virAdmConnectCloseCallbackUnregister. 717 (88 direct, 629 indirect) bytes in 1 blocks are definitely lost record 110 of 141 at 0x4C2A988: calloc (vg_replace_malloc.c:711) by 0x530696F: virAllocVar (viralloc.c:560) by 0x53689E6: virObjectNew (virobject.c:193) by 0x5368B5E: virObjectLockableNew (virobject.c:219) by 0x4E3E7EE: virAdmConnectNew (datatypes.c:900) by 0x4E398BB: virAdmConnectOpen (libvirt-admin.c:220) by 0x10D3E3: vshAdmConnect (virt-admin.c:161) by 0x10D624: vshAdmReconnect (virt-admin.c:215) by 0x10DB0A: cmdConnect (virt-admin.c:353) by 0x11288F: vshCommandRun (vsh.c:1313) by 0x10FDB6: main (virt-admin.c:1439) Resolves: https://bugzilla.redhat.com/show_bug.cgi?id=1357358 Signed-off-by: Erik Skultety --- src/admin/admin_remote.c | 6 +----- src/datatypes.c | 5 +---- src/libvirt-admin.c | 18 +----------------- 3 files changed, 3 insertions(+), 26 deletions(-) diff --git a/src/admin/admin_remote.c b/src/admin/admin_remote.c index 10a3b18b00..f37ff5e8b8 100644 --- a/src/admin/admin_remote.c +++ b/src/admin/admin_remote.c @@ -144,11 +144,7 @@ remoteAdminClientCloseFunc(virNetClientPtr client ATTRIBUTE_UNUSED, VIR_DEBUG("Triggering connection close callback %p reason=%d, opaque=%p", cbdata->callback, reason, cbdata->opaque); cbdata->callback(cbdata->conn, reason, cbdata->opaque); - - if (cbdata->freeCallback) - cbdata->freeCallback(cbdata->opaque); - cbdata->callback = NULL; - cbdata->freeCallback = NULL; + virAdmConnectCloseCallbackDataReset(cbdata); } virObjectUnlock(cbdata); } diff --git a/src/datatypes.c b/src/datatypes.c index 24e4f776a0..c8a46b0f31 100644 --- a/src/datatypes.c +++ b/src/datatypes.c @@ -928,10 +928,7 @@ virAdmConnectCloseCallbackDataDispose(void *obj) virAdmConnectCloseCallbackDataPtr cb_data = obj; virObjectLock(cb_data); - - if (cb_data->freeCallback) - cb_data->freeCallback(cb_data->opaque); - + virAdmConnectCloseCallbackDataReset(cb_data); virObjectUnlock(cb_data); } diff --git a/src/libvirt-admin.c b/src/libvirt-admin.c index 1b5fd443d6..8877e499ec 100644 --- a/src/libvirt-admin.c +++ b/src/libvirt-admin.c @@ -513,29 +513,13 @@ int virAdmConnectUnregisterCloseCallback(virAdmConnectPtr conn, virResetLastError(); virCheckAdmConnectReturn(conn, -1); - - virObjectLock(conn->closeCallback); - virCheckNonNullArgGoto(cb, error); - if (conn->closeCallback->callback != cb) { - virReportError(VIR_ERR_OPERATION_INVALID, "%s", - _("A different callback was requested")); + if (virAdmConnectCloseCallbackDataUnregister(conn->closeCallback, cb) < 0) goto error; - } - - conn->closeCallback->callback = NULL; - if (conn->closeCallback->freeCallback) - conn->closeCallback->freeCallback(conn->closeCallback->opaque); - conn->closeCallback->freeCallback = NULL; - - virObjectUnlock(conn->closeCallback); - virObjectUnref(conn); return 0; - error: - virObjectUnlock(conn->closeCallback); virDispatchError(NULL); return -1; }