remoteClientCloseFunc: Don't mangle connection object refcount

Well, in 8ad126e6 we tried to fix a memory corruption problem.
However, the fix was not as good as it could be. I mean, the
commit has one line more than it should. I've noticed this output
just recently:

  # ./run valgrind --leak-check=full --show-reachable=yes ./tools/virsh domblklist gentoo
  ==17019== Memcheck, a memory error detector
  ==17019== Copyright (C) 2002-2013, and GNU GPL'd, by Julian Seward et al.
  ==17019== Using Valgrind-3.10.1 and LibVEX; rerun with -h for copyright info
  ==17019== Command: /home/zippy/work/libvirt/libvirt.git/tools/.libs/virsh domblklist gentoo
  ==17019==
  Target     Source
  ------------------------------------------------
  fda        /var/lib/libvirt/images/fd.img
  vda        /var/lib/libvirt/images/gentoo.qcow2
  hdc        /home/zippy/tmp/install-amd64-minimal-20150402.iso

  ==17019== Thread 2:
  ==17019== Invalid read of size 4
  ==17019==    at 0x4EFF5B4: virObjectUnref (virobject.c:258)
  ==17019==    by 0x5038CFF: remoteClientCloseFunc (remote_driver.c:552)
  ==17019==    by 0x5069D57: virNetClientCloseLocked (virnetclient.c:685)
  ==17019==    by 0x506C848: virNetClientIncomingEvent (virnetclient.c:1852)
  ==17019==    by 0x5082136: virNetSocketEventHandle (virnetsocket.c:1913)
  ==17019==    by 0x4ECD64E: virEventPollDispatchHandles (vireventpoll.c:509)
  ==17019==    by 0x4ECDE02: virEventPollRunOnce (vireventpoll.c:658)
  ==17019==    by 0x4ECBF00: virEventRunDefaultImpl (virevent.c:308)
  ==17019==    by 0x130386: vshEventLoop (vsh.c:1864)
  ==17019==    by 0x4F1EB07: virThreadHelper (virthread.c:206)
  ==17019==    by 0xA8462D3: start_thread (in /lib64/libpthread-2.20.so)
  ==17019==    by 0xAB441FC: clone (in /lib64/libc-2.20.so)
  ==17019==  Address 0x139023f4 is 4 bytes inside a block of size 240 free'd
  ==17019==    at 0x4C2B1F0: free (in /usr/lib64/valgrind/vgpreload_memcheck-amd64-linux.so)
  ==17019==    by 0x4EA8949: virFree (viralloc.c:582)
  ==17019==    by 0x4EFF6D0: virObjectUnref (virobject.c:273)
  ==17019==    by 0x4FE74D6: virConnectClose (libvirt.c:1390)
  ==17019==    by 0x13342A: virshDeinit (virsh.c:406)
  ==17019==    by 0x134A37: main (virsh.c:950)

The problem is, when registering remoteClientCloseFunc(), it's
conn->closeCallback which is ref'd. But in the function itself
it's conn->closeCallback->conn what is unref'd. This is causing
imbalance in reference counting. Moreover, there's no need for
the remote driver to increase/decrease conn refcount since it's
not used anywhere. It's just merely passed to client registered
callback. And for that purpose it's correctly ref'd in
virConnectRegisterCloseCallback() and then unref'd in
virConnectUnregisterCloseCallback().

Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
(cherry picked from commit e68930077034f786e219bdb015f8880dbc5a246f)
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>

diff --git a/src/remote/remote_driver.c b/src/remote/remote_driver.c
index ed7dde6a450564ae05ac4234a36cd2d88cf96b79..056d07ac9d60741ffd26a2a30e2a21efcd4ed68f 100644 (file)
--- a/src/remote/remote_driver.c
+++ b/src/remote/remote_driver.c
@@ -540,10 +540,6 @@ remoteClientCloseFunc(virNetClientPtr client ATTRIBUTE_UNUSED,
         cbdata->freeCallback = NULL;
     }
     virObjectUnlock(cbdata);
-
-    /* free the connection reference that comes along with the callback
-     * registration */
-    virObjectUnref(cbdata->conn);
 }
 
 /* helper macro to ease extraction of arguments from the URI */