[libvirt] [PATCH v3 2/2] remote: free client all event callbacks at remoteClientCloseFunc

[John Ferlan]

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);
>  }
>  
>  
>