[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]

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



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


[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]