Re: [libvirt] [PATCH v2] util: avoid fds leak when virEventPollAddHandle fail

["Daniel P. Berrange" <berrange redhat com>]

On Tue, Jul 19, 2011 at 05:34:28PM +0800, ajia redhat com wrote:
> * src/util/event_poll.c: only fix file descriptors leak on when 
>   virEventPollAddHandle fail on virEventPollInit function.
> 
> Detected in valgrind run:
> ==1254==
> ==1254== FILE DESCRIPTORS: 6 open at exit.
> ==1254== Open file descriptor 5:
> ==1254==    at 0x30736D9D47: pipe2 (syscall-template.S:82)
> ==1254==    by 0x4DD6267: rpl_pipe2 (pipe2.c:61)
> ==1254==    by 0x4C4C1C5: virEventPollInit (event_poll.c:648)
> ==1254==    by 0x4C4AA94: virEventRegisterDefaultImpl (event.c:208)
> ==1254==    by 0x42150C: main (virsh.c:13790)
> ==1254==
> ==1254== Open file descriptor 4:
> ==1254==    at 0x30736D9D47: pipe2 (syscall-template.S:82)
> ==1254==    by 0x4DD6267: rpl_pipe2 (pipe2.c:61)
> ==1254==    by 0x4C4C1C5: virEventPollInit (event_poll.c:648)
> ==1254==    by 0x4C4AA94: virEventRegisterDefaultImpl (event.c:208)
> ==1254==    by 0x42150C: main (virsh.c:13790)
> ==1254==
> 
> * how to reproduce?
>   % valgrind -v --track-fds=yes virsh list

This trace is unrelated to the bug you're fixing below, so it
should not be in the commit message.

This trace shows that the handles from virEventInit are never
closed. This is pretty much expected, since we have no equiv
to virEventInit at program exit. Nor do we have any equiv to
virInitialize for shutdown either.

> ---
>  src/util/event_poll.c |    3 +++
>  1 files changed, 3 insertions(+), 0 deletions(-)
> 
> diff --git a/src/util/event_poll.c b/src/util/event_poll.c
> index 285ba50..e2ae3a6 100644
> --- a/src/util/event_poll.c
> +++ b/src/util/event_poll.c
> @@ -36,6 +36,7 @@
>  #include "event_poll.h"
>  #include "memory.h"
>  #include "util.h"
> +#include "files.h"
>  #include "ignore-value.h"
>  #include "virterror_internal.h"
>  
> @@ -657,6 +658,8 @@ int virEventPollInit(void)
>          virEventError(VIR_ERR_INTERNAL_ERROR,
>                        _("Unable to add handle %d to event loop"),
>                        eventLoop.wakeupfd[0]);
> +        VIR_FORCE_CLOSE(eventLoop.wakeupfd[0]);
> +        VIR_FORCE_CLOSE(eventLoop.wakeupfd[1]);
>          return -1;
>      }

ACK, if the commit message is updated

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 :|