Ensure client streams are closed when marking a client for close

Every active stream results in a reference being held on the
virNetServerClientPtr object. This meant that if a client quit
with any streams active, although all I/O was stopped the
virNetServerClientPtr object would leak. This causes libvirtd
to leak any file handles associated with open streams when a
client quit

To fix this, when we call virNetServerClientClose there is a
callback invoked which lets the daemon release the streams
and thus the extra references

* daemon/remote.c: Add a hook to close all streams
* daemon/stream.c, daemon/stream.h: Add API for releasing
  all streams
* src/rpc/virnetserverclient.c, src/rpc/virnetserverclient.h:
  Allow registration of a hook to trigger when closing client

diff --git a/daemon/remote.c b/daemon/remote.c
index 939044c781f9113a99ce9f199f44692f14c79756..0f088c6172a356187815273ecd68ddb99eaaf794 100644 (file)
--- a/daemon/remote.c
+++ b/daemon/remote.c
@@ -439,6 +439,13 @@ static void remoteClientFreeFunc(void *data)
 }
 
 
+static void remoteClientCloseFunc(virNetServerClientPtr client)
+{
+    struct daemonClientPrivate *priv = virNetServerClientGetPrivateData(client);
+
+    daemonRemoveAllClientStreams(priv->streams);
+}
+
 
 int remoteClientInitHook(virNetServerPtr srv ATTRIBUTE_UNUSED,
                          virNetServerClientPtr client)
@@ -460,7 +467,9 @@ int remoteClientInitHook(virNetServerPtr srv ATTRIBUTE_UNUSED,
     for (i = 0 ; i < VIR_DOMAIN_EVENT_ID_LAST ; i++)
         priv->domainEventCallbackID[i] = -1;
 
-    virNetServerClientSetPrivateData(client, priv, remoteClientFreeFunc);
+    virNetServerClientSetPrivateData(client, priv,
+                                     remoteClientFreeFunc);
+    virNetServerClientSetCloseHook(client, remoteClientCloseFunc);
     return 0;
 }
 

diff --git a/daemon/stream.c b/daemon/stream.c
index 4a8f1eed91276c74fcee8f2669f6532f3212ab26..7dd9ae770e8cebf2261f64335f3e420c65e43639 100644 (file)
--- a/daemon/stream.c
+++ b/daemon/stream.c
@@ -334,13 +334,17 @@ int daemonFreeClientStream(virNetServerClientPtr client,
     msg = stream->rx;
     while (msg) {
         virNetMessagePtr tmp = msg->next;
-        /* Send a dummy reply to free up 'msg' & unblock client rx */
-        memset(msg, 0, sizeof(*msg));
-        msg->header.type = VIR_NET_REPLY;
-        if (virNetServerClientSendMessage(client, msg) < 0) {
-            virNetServerClientImmediateClose(client);
+        if (client) {
+            /* Send a dummy reply to free up 'msg' & unblock client rx */
+            memset(msg, 0, sizeof(*msg));
+            msg->header.type = VIR_NET_REPLY;
+            if (virNetServerClientSendMessage(client, msg) < 0) {
+                virNetServerClientImmediateClose(client);
+                virNetMessageFree(msg);
+                ret = -1;
+            }
+        } else {
             virNetMessageFree(msg);
-            ret = -1;
         }
         msg = tmp;
     }
@@ -441,6 +445,28 @@ daemonRemoveClientStream(virNetServerClientPtr client,
 }
 
 
+void
+daemonRemoveAllClientStreams(daemonClientStream *stream)
+{
+    daemonClientStream *tmp;
+
+    VIR_DEBUG("stream=%p", stream);
+
+    while (stream) {
+        tmp = stream->next;
+
+        if (!stream->closed) {
+            virStreamEventRemoveCallback(stream->st);
+            virStreamAbort(stream->st);
+        }
+
+        daemonFreeClientStream(NULL, stream);
+
+        VIR_DEBUG("next stream=%p", tmp);
+        stream = tmp;
+    }
+}
+
 /*
  * Returns:
  *   -1  if fatal error occurred

diff --git a/daemon/stream.h b/daemon/stream.h
index 626ae60914d322295ded876166cacfa14c9b1b19..7c2d8dc70d8c2095654dd66744b4705895cacd26 100644 (file)
--- a/daemon/stream.h
+++ b/daemon/stream.h
@@ -45,4 +45,7 @@ int
 daemonRemoveClientStream(virNetServerClientPtr client,
                          daemonClientStream *stream);
 
+void
+daemonRemoveAllClientStreams(daemonClientStream *stream);
+
 #endif /* __LIBVIRTD_STREAM_H__ */

diff --git a/src/rpc/virnetserverclient.c b/src/rpc/virnetserverclient.c
index e246fa54c2ee98ba32ac14787c815cdea1dfa2dc..a73b06d692438349db1c539793b9d6e708101a9c 100644 (file)
--- a/src/rpc/virnetserverclient.c
+++ b/src/rpc/virnetserverclient.c
@@ -97,6 +97,7 @@ struct _virNetServerClient
 
     void *privateData;
     virNetServerClientFreeFunc privateDataFreeFunc;
+    virNetServerClientCloseFunc privateDataCloseFunc;
 };
 
 
@@ -492,6 +493,15 @@ void *virNetServerClientGetPrivateData(virNetServerClientPtr client)
 }
 
 
+void virNetServerClientSetCloseHook(virNetServerClientPtr client,
+                                    virNetServerClientCloseFunc cf)
+{
+    virNetServerClientLock(client);
+    client->privateDataCloseFunc = cf;
+    virNetServerClientUnlock(client);
+}
+
+
 void virNetServerClientSetDispatcher(virNetServerClientPtr client,
                                      virNetServerClientDispatchFunc func,
                                      void *opaque)
@@ -560,6 +570,8 @@ void virNetServerClientFree(virNetServerClientPtr client)
  */
 void virNetServerClientClose(virNetServerClientPtr client)
 {
+    virNetServerClientCloseFunc cf;
+
     virNetServerClientLock(client);
     VIR_DEBUG("client=%p refs=%d", client, client->refs);
     if (!client->sock) {
@@ -567,6 +579,15 @@ void virNetServerClientClose(virNetServerClientPtr client)
         return;
     }
 
+    if (client->privateDataCloseFunc) {
+        cf = client->privateDataCloseFunc;
+        client->refs++;
+        virNetServerClientUnlock(client);
+        (cf)(client);
+        virNetServerClientLock(client);
+        client->refs--;
+    }
+
     /* Do now, even though we don't close the socket
      * until end, to ensure we don't get invoked
      * again due to tls shutdown */

diff --git a/src/rpc/virnetserverclient.h b/src/rpc/virnetserverclient.h
index 3d2e1fba4520b0626dd138791172d8da3321c69e..bedb179e747407686b2ae6916bbef2d2aaf2b8e9 100644 (file)
--- a/src/rpc/virnetserverclient.h
+++ b/src/rpc/virnetserverclient.h
@@ -82,6 +82,11 @@ void virNetServerClientSetPrivateData(virNetServerClientPtr client,
                                       virNetServerClientFreeFunc ff);
 void *virNetServerClientGetPrivateData(virNetServerClientPtr client);
 
+typedef void (*virNetServerClientCloseFunc)(virNetServerClientPtr client);
+
+void virNetServerClientSetCloseHook(virNetServerClientPtr client,
+                                    virNetServerClientCloseFunc cf);
+
 void virNetServerClientSetDispatcher(virNetServerClientPtr client,
                                      virNetServerClientDispatchFunc func,
                                      void *opaque);