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 <eskultet@redhat.com>

diff --git a/src/admin/admin_remote.c b/src/admin/admin_remote.c
index 10a3b18b00b559e16aa2bc7e2fcf95d744f4c194..f37ff5e8b868b163bf37a89c20b82ce47f0b6b6e 100644 (file)
--- 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 24e4f776a04288592ee1dc5aafcb8f81cb06927b..c8a46b0f31da6a20b8fcba8e197b1a142c91f379 100644 (file)
--- 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 1b5fd443d68795fde48debbf4cba7415331d759a..8877e499ece2b15d1d2b2752ca218a349ce2dfb2 100644 (file)
--- 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;
 }