[Libguestfs] [PATCH] launch: rework handling of --enable-valgrind-daemon

[Pino Toscano]

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#L346-L364
> 
> 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