[libvirt] Is it a problem that after virEventRegisterDefaultImpl we have handlers leaked
Wangyufei (A)
james.wangyufei at huawei.com
Tue Sep 3 12:16:31 UTC 2013
> -----Original Message-----
> From: Daniel P. Berrange [mailto:berrange at redhat.com]
> Sent: Tuesday, September 03, 2013 6:30 PM
> To: Wangyufei (A)
> Cc: libvir-list at redhat.com; Wangrui (K)
> Subject: Re: [libvirt] 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 at redhat.com]
> > > Sent: Tuesday, September 03, 2013 5:14 PM
> > > To: Wangyufei (A)
> > > Cc: libvir-list at 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.
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 :|
More information about the libvir-list
mailing list