[libvirt] virNetClientPtr leak in remote driver

[Matthias Bolte]

doRemoteClose doesn't free the virNetClientPtr and this creates a
260kb leak per remote connection. This happens because
virNetClientFree doesn't remove the last ref, because virNetClientNew
creates the virNetClientPtr with a refcount of 2.

static virNetClientPtr virNetClientNew(virNetSocketPtr sock,
                                       const char *hostname)
{
[...]
    client->refs = 1;
[...]
    /* Set up a callback to listen on the socket data */
    client->refs++;
    if (virNetSocketAddIOCallback(client->sock,
                                  VIR_EVENT_HANDLE_READABLE,
                                  virNetClientIncomingEvent,
                                  client,
                                  virNetClientEventFree) < 0) {
        client->refs--;
        VIR_DEBUG("Failed to add event watch, disabling events");
    }
[...]
}

virNetClientNew adds a ref before calling virNetSocketAddIOCallback
but only removes it when virNetSocketAddIOCallback fails. This seems
wrong too me and the ref should be removed in the success case too.

The same pattern was added in 0302391ee643ad91fdc6d2ecf7e4cf0fc9724516
(Fix race in ref counting when handling RPC jobs)

--- a/src/rpc/virnetserverclient.c
+++ b/src/rpc/virnetserverclient.c
@@ -763,10 +763,12 @@ readmore:

         /* Send off to for normal dispatch to workers */
         if (msg) {
+            client->refs++;
             if (!client->dispatchFunc ||
                 client->dispatchFunc(client, msg,
client->dispatchOpaque) < 0) {
                 virNetMessageFree(msg);
                 client->wantClose = true;
+                client->refs--;
                 return;
             }
         }

Again, this seems wrong and the ref should be removed in the success
case here too. Before I spent time to figure out how the refcounting
is supposed to work here I just report it and hope that Dan can easily
fix this.

-- 
Matthias Bolte
http://photron.blogspot.com