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

-----Original Message----- From: Daniel P. Berrange [mailto:berrange@redhat.com] Sent: Tuesday, September 03, 2013 5:14 PM To: Wangyufei (A) Cc: libvir-list(a)redhat.com; Wangrui (K) Subject: Re: [libvirt] Is it a problem that after virEventRegisterDefaultImpl we have handlers leaked 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 :|