[libvirt] 答复: [PATCH] remote: Fix libvirtd service memory leak when libvirt client was terminated unexpectly

Caoxinhua caoxinhua at huawei.com
Thu Nov 2 01:08:13 UTC 2017


ping

-----邮件原件-----
发件人: Caoxinhua 
发送时间: 2017年10月28日 9:33
收件人: Caoxinhua; libvir-list at redhat.com; jferlan at redhat.com; mkletzan at redhat.com; berrange at redhat.com
抄送: Huangweidong (C); Yanqiangjun; weifuqiang; Wangjing (King, Euler)
主题: 答复: [PATCH] remote: Fix libvirtd service memory leak when libvirt client was terminated unexpectly

ping

-----邮件原件-----
发件人: Caoxinhua
发送时间: 2017年10月24日 13:52
收件人: libvir-list at redhat.com; jferlan at redhat.com; mkletzan at redhat.com; berrange at redhat.com
抄送: Huangweidong (C); Yanqiangjun; weifuqiang; Wangjing (King, Euler); Caoxinhua
主题: [PATCH] remote: Fix libvirtd service memory leak when libvirt client was terminated unexpectly

base on commit 2033e8cc119454bc4273e8a41e66c899c60ba58b and fe8f1c8b8650c8ab5e7d27338f4538582651bd14, we solve libvirt coredump problem, but it introduce a memory leak sense.
the sense follow
1. one client register a domain event such as reboot event 2. and client was terminated unexpectly, then this client will not free at libvirtd service program.

remoteDispatchConnectDomainEventCallbackRegisterAny reference the client, but when client was terminated before it call deRegisterAny, the reference of client will not reduced to zero. so the memory leak take place. this patch will deRegister all event when client was close.
---
 daemon/remote.c | 47 +++++++++++++++++++++++++++++------------------
 1 file changed, 29 insertions(+), 18 deletions(-)

diff --git a/daemon/remote.c b/daemon/remote.c index 3f7d2d3..2b5a18b 100644
--- a/daemon/remote.c
+++ b/daemon/remote.c
@@ -1686,25 +1686,16 @@ void remoteRelayConnectionClosedEvent(virConnectPtr conn ATTRIBUTE_UNUSED, int r
                 VIR_WARN("unexpected %s event deregister failure", name);   \
         }                                                                   \
         VIR_FREE(eventCallbacks);                                           \
+        neventCallbacks = 0;                                                \
     } while (0);
 
-/*
- * You must hold lock for at least the client
- * We don't free stuff here, merely disconnect the client's
- * network socket & resources.
- * We keep the libvirt connection open until any async
- * jobs have finished, then clean it up elsewhere
- */
-void remoteClientFreeFunc(void *data)
+static void
+remoteFreePrivCallbacks(void *data)
 {
     struct daemonClientPrivate *priv = data;
 
     /* Deregister event delivery callback */
-    if (priv->conn) {
-        virIdentityPtr sysident = virIdentityGetSystem();
-
-        virIdentitySetCurrent(sysident);
-
+    if (priv && priv->conn) {
         DEREG_CB(priv->conn, priv->domainEventCallbacks,
                  priv->ndomainEventCallbacks,
                  virConnectDomainEventDeregisterAny, "domain"); @@ -1723,6 +1714,26 @@ void remoteClientFreeFunc(void *data)
         DEREG_CB(priv->conn, priv->qemuEventCallbacks,
                  priv->nqemuEventCallbacks,
                  virConnectDomainQemuMonitorEventDeregister, "qemu monitor");
+    }
+}
+#undef DEREG_CB
+
+/*
+ * You must hold lock for at least the client
+ * We don't free stuff here, merely disconnect the client's
+ * network socket & resources.
+ * We keep the libvirt connection open until any async
+ * jobs have finished, then clean it up elsewhere  */ void 
+remoteClientFreeFunc(void *data) {
+    struct daemonClientPrivate *priv = data;
+
+    if (priv) {
+        virIdentityPtr sysident = virIdentityGetSystem();
+
+        virIdentitySetCurrent(sysident);
+        remoteFreePrivCallbacks(priv);
 
         if (priv->closeRegistered) {
             if (virConnectUnregisterCloseCallback(priv->conn,
@@ -1734,18 +1745,18 @@ void remoteClientFreeFunc(void *data)
 
         virIdentitySetCurrent(NULL);
         virObjectUnref(sysident);
+        VIR_FREE(priv);
     }
-
-    VIR_FREE(priv);
 }
-#undef DEREG_CB
-
 
 static void remoteClientCloseFunc(virNetServerClientPtr client)  {
     struct daemonClientPrivate *priv = virNetServerClientGetPrivateData(client);
 
-    daemonRemoveAllClientStreams(priv->streams);
+    if (priv) {
+        daemonRemoveAllClientStreams(priv->streams);
+        remoteFreePrivCallbacks(priv);
+    }
 }
 
 
--
2.8.3






More information about the libvir-list mailing list