On 11/11/2017 03:30 AM, xinhua.Cao wrote:
> base on commit fe8f1c8b8650c8ab5e7d27338f4538582651bd14, we solve
> libvirt coredump problem, but it introduce a memory leak sense:
>
> 1. one client register a domain event such as reboot event
> 2. and client was terminated unexpectly, such as kill -9,
> then this client object 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 callbacks when client was close.
> ---
> daemon/remote.c | 62 +++++++++++++++++++++++++++++++++------------------------
> 1 file changed, 36 insertions(+), 26 deletions(-)
>
See my comments from the v2 series....
John
> diff --git a/daemon/remote.c b/daemon/remote.c
> index cbcb6e8..2073534 100644
> --- a/daemon/remote.c
> +++ b/daemon/remote.c
> @@ -1689,6 +1689,37 @@ void remoteRelayConnectionClosedEvent(virConnectPtr conn ATTRIBUTE_UNUSED, int r
> neventCallbacks = 0; \
> } while (0);
>
> +static void
> +remoteClientFreePrivateCallbacks(struct daemonClientPrivate *priv)
> +{
> + DEREG_CB(priv->conn, priv->domainEventCallbacks,
> + priv->ndomainEventCallbacks,
> + virConnectDomainEventDeregisterAny, "domain");
> + DEREG_CB(priv->conn, priv->networkEventCallbacks,
> + priv->nnetworkEventCallbacks,
> + virConnectNetworkEventDeregisterAny, "network");
> + DEREG_CB(priv->conn, priv->storageEventCallbacks,
> + priv->nstorageEventCallbacks,
> + virConnectStoragePoolEventDeregisterAny, "storage");
> + DEREG_CB(priv->conn, priv->nodeDeviceEventCallbacks,
> + priv->nnodeDeviceEventCallbacks,
> + virConnectNodeDeviceEventDeregisterAny, "node device");
> + DEREG_CB(priv->conn, priv->secretEventCallbacks,
> + priv->nsecretEventCallbacks,
> + virConnectSecretEventDeregisterAny, "secret");
> + DEREG_CB(priv->conn, priv->qemuEventCallbacks,
> + priv->nqemuEventCallbacks,
> + virConnectDomainQemuMonitorEventDeregister, "qemu monitor");
> +
> + if (priv->closeRegistered) {
> + if (virConnectUnregisterCloseCallback(priv->conn,
> + remoteRelayConnectionClosedEvent) < 0)
> + VIR_WARN("unexpected close callback event deregister failure");
> + }
> +}
> +#undef DEREG_CB
> +
> +
> /*
> * You must hold lock for at least the client
> * We don't free stuff here, merely disconnect the client's
> @@ -1706,40 +1737,17 @@ void remoteClientFreeFunc(void *data)
>
> virIdentitySetCurrent(sysident);
>
> - DEREG_CB(priv->conn, priv->domainEventCallbacks,
> - priv->ndomainEventCallbacks,
> - virConnectDomainEventDeregisterAny, "domain");
> - DEREG_CB(priv->conn, priv->networkEventCallbacks,
> - priv->nnetworkEventCallbacks,
> - virConnectNetworkEventDeregisterAny, "network");
> - DEREG_CB(priv->conn, priv->storageEventCallbacks,
> - priv->nstorageEventCallbacks,
> - virConnectStoragePoolEventDeregisterAny, "storage");
> - DEREG_CB(priv->conn, priv->nodeDeviceEventCallbacks,
> - priv->nnodeDeviceEventCallbacks,
> - virConnectNodeDeviceEventDeregisterAny, "node device");
> - DEREG_CB(priv->conn, priv->secretEventCallbacks,
> - priv->nsecretEventCallbacks,
> - virConnectSecretEventDeregisterAny, "secret");
> - DEREG_CB(priv->conn, priv->qemuEventCallbacks,
> - priv->nqemuEventCallbacks,
> - virConnectDomainQemuMonitorEventDeregister, "qemu monitor");
> -
> - if (priv->closeRegistered) {
> - if (virConnectUnregisterCloseCallback(priv->conn,
> - remoteRelayConnectionClosedEvent) < 0)
> - VIR_WARN("unexpected close callback event deregister failure");
> - }
> + remoteClientFreePrivateCallbacks(priv);
>
> virConnectClose(priv->conn);
>
> virIdentitySetCurrent(NULL);
> virObjectUnref(sysident);
> - }
>
> + }
> VIR_FREE(priv);
> +
> }
> -#undef DEREG_CB
>
>
> static void remoteClientCloseFunc(virNetServerClientPtr client)
> @@ -1747,6 +1755,8 @@ static void remoteClientCloseFunc(virNetServerClientPtr client)
> struct daemonClientPrivate *priv = virNetServerClientGetPrivateData(client);
>
> daemonRemoveAllClientStreams(priv->streams);
> + if (priv->conn)
> + remoteClientFreePrivateCallbacks(priv);
> }
>
>
>
--
libvir-list mailing list
libvir-list@xxxxxxxxxx
https://www.redhat.com/mailman/listinfo/libvir-list
