> -----Original Message----- > From: Daniel P. Berrange [mailto:berrange@xxxxxxxxxx] > Sent: Tuesday, September 03, 2013 6:30 PM > To: Wangyufei (A) > Cc: libvir-list@xxxxxxxxxx; Wangrui (K) > Subject: Re: Is it a problem that after virEventRegisterDefaultImpl we > have handlers leaked > > On Tue, Sep 03, 2013 at 10:25:45AM +0000, Wangyufei (A) wrote: > > > > > > > -----Original Message----- > > > From: Daniel P. Berrange [mailto:berrange@xxxxxxxxxx] > > > Sent: Tuesday, September 03, 2013 5:14 PM > > > To: Wangyufei (A) > > > Cc: libvir-list@xxxxxxxxxx; Wangrui (K) > > > Subject: Re: 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. > > > > Well, Is there any tiny chance that event loop stopped? > > If that happened now, the handlers leak will affect the whole host OS, > maybe no handlers to use at last. > > Is that what we expect? > > Can we do something to reduce the impact even if something impossible > happened? > > If the event loop has bugs which cause it to stop then those > should be fixed. This isn't something we need / want to work > around elsewhere. Fine, thanks a lot. > > 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 :| -- libvir-list mailing list libvir-list@xxxxxxxxxx https://www.redhat.com/mailman/listinfo/libvir-list