[libvirt] Is it a problem that after virEventRegisterDefaultImpl we have handlers leaked

[Daniel P. Berrange]

On Mon, Sep 02, 2013 at 07:44:39AM +0000, Wangyufei (A) wrote:
> Hello,
> When I ran programme event-test compiled from event-test.c, I found a problem that, after virEventRegisterDefaultImpl I do virConnectOpenAuth and virConnectClose, there will be handlers of socket and pipe opened by virConnectOpenAuth leaked after virConnectClose. So I did some analysis, and I found the fact following:
> 
> In the condition that we only have one connection here
> 
> int virNetSocketAddIOCallback(virNetSocketPtr sock,
>                               int events,
>                               virNetSocketIOFunc func,
>                               void *opaque,
>                               virFreeCallback ff)
> {
>     int ret = -1;
> 
>     virObjectRef(sock); //Here we add sock refers once, then we will get refers equal 2 after
>     virObjectLock(sock);
>     if (sock->watch > 0) {
>         VIR_DEBUG("Watch already registered on socket %p", sock);
>         goto cleanup;
>     }
> 
>     if ((sock->watch = virEventAddHandle(sock->fd, //If we have called virEventRegisterDefaultImpl, then here we'll get a sock watch non negative and will not go to cleanup.
>                                          events,
>                                          virNetSocketEventHandle,
>                                          sock,
>                                          virNetSocketEventFree)) < 0) {
>         VIR_DEBUG("Failed to register watch on socket %p", sock);
>         goto cleanup;
>     }
>     sock->func = func;
>     sock->opaque = opaque;
>     sock->ff = ff;
> 
>     ret = 0;
> 
> cleanup:
>     virObjectUnlock(sock);
>     if (ret != 0)
>         virObjectUnref(sock); //If we haven't called virEventRegisterDefaultImpl, we'll be here after virEventAddHandle, and sock refers will decrease to 1
>     return ret;
> }
> 
> Condition with virEventRegisterDefaultImpl, we'll do unrefer action in two places:
> 
> 1.       virEventRunDefaultImpl  ->virEventPollRunOnce  ->virEventRunDefaultImpl  ->virEventPollRunOnce  ->virEventPollCleanupHandles -> virNetSocketEventFree -> virObjectUnref
> 
> 2.       virConnectClose ->virObjectUnref ->virConnectDispose ->remoteConnectClose  ->doRemoteClose  ->virNetClientClose  ->virNetClientCloseInternal -> virNetClientIOEventLoopPassTheBuck -> virNetClientCloseLocked -> virObjectUnref
> 
> When event dealing loop is alive, everything work fine, we'll get sock refers 2
> after virConnectOpenAuth and unrefer twice in two places above after virConnectClose.
> But If some accidents happened like we quit event dealing loop or virEventRunDefaultImpl
> suspended, then sock refers will never be zero, and handlers will never be freed.

Do not stop running the event loop. It is a requirement of the API that once you have
called  virEventRegisterDefaultImpl, you *must* always execute the event loop forever
after until your application exits.


> I consider to add something like virEventDeregisterDefaultImpl to set addHandleImpl and his buddy NULL. Apparently it is far away from fixing it completely.

No, stopping or de-registering event loop impls is a non-goal.

> At last I have two questions here:
> 
> 
> 1.       Is it a problem that after virEventRegisterDefaultImpl we have handlers leaked?

There are no handlers leaked if you run the event loop, which is a requirement
of the API.


Daniel
-- 
|: http://berrange.com      -o-    http://www.flickr.com/photos/dberrange/ :|
|: http://libvirt.org              -o-             http://virt-manager.org :|
|: http://autobuild.org       -o-         http://search.cpan.org/~danberr/ :|
|: http://entangle-photo.org       -o-       http://live.gnome.org/gtk-vnc :|