On Tue, Sep 03, 2013 at 10:25:45AM +0000, Wangyufei (A) wrote: > > > > -----Original Message----- > > From: Daniel P. Berrange [mailto:berra...@redhat.com] > > Sent: Tuesday, September 03, 2013 5:14 PM > > To: Wangyufei (A) > > Cc: libvir-list@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. > > 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. 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@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
