In data giovedì 25 giugno 2015 15:54:56, Richard W.M. Jones ha scritto:
On Thu, Jun 25, 2015 at 04:16:59PM +0200, Pino Toscano wrote:
> In data giovedì 25 giugno 2015 14:50:03, Richard W.M. Jones ha scritto:
> > This doesn't handle the guestfsd shutdown case (but nor does the
> > current code, which is both racy on shutdown and introduces separate
> > shutdown paths for --enable-valgrind-daemon and ordinary
> > configurations). But we can punt on that until later. The above
> > would detect all memory errors except for memory leaks.
>
> ... although losing the leaks detection would be a no-go for me, since
> that's something I've been using from time to time, even if not often.
>
> Can you expand a bit more on the parts you consider racy?
There are two objections:
(1) That we currently have separate shutdown paths in the
valgrind/non-valgrind case:
https://github.com/libguestfs/libguestfs/blob/master/src/handle.c#L429-L443
https://github.com/libguestfs/libguestfs/blob/master/src/conn-socket.c#L3...
We shouldn't have two different paths that developers and non-
developers use.
(2) This actually causes bugs. The second one is the race.
https://bugzilla.redhat.com/show_bug.cgi?id=1023630
https://bugzilla.redhat.com/show_bug.cgi?id=1020216
There's also another race which I don't think we have a bug about
where the output from valgrind is truncated (because valgrind is quite
slow at shutdown) so that you don't see valgrind errors reliably.
Attempting (not always successfully) to work around that race is the
purpose of the sleep here:
https://github.com/libguestfs/libguestfs/blob/master/appliance/init#L154
In summary, I think it's a hack. I cannot use it because of the bugs
above, which makes it useless for me. Finding memory leaks is
certainly important, but crashes in the daemon are way more important.
I'm not saying we couldn't fix the memory leak problem later.
Thanks for the explanations.
I'm trying to get a better valgrind mode, using the actual
implementation as starting point, and surely knowing about the current
shortcomings will help in that.
--
Pino Toscano