Re: patch 0/2 debuginfod submission

[Mark Wielaard <mark at klomp dot org>]

Hi Frank,

On Wed, 2019-10-30 at 09:40 -0400, Frank Ch. Eigler wrote:
> OK, sure, IMO don't even bother with a guard.  It's just a dlopen/dlsym,
> which is portable.  Will update the first patch on the branch with that.

Thanks.

> > Also I think you should cache a negative result for
> > fp_debuginfod_find_debuginfo (say assign it (void *) -1) so you
> > don't
> > keep trying to find the library/symbol each and every time.
> 
> Will look into that later on.  With the debuginfod server, a negative
> hit comes back instantly (no file searching, just an indexed database
> lookup), so this may not be a big deal.

Ah, I was not thinking that far yet. I was just worried about the
dlopen/dlsym dance being done on every call. Which does its own file
search to find the library. So simply setting debuginfod_so = (void *)
-1; on first failure to make sure dlopen/dlsym it is never called
again.

> > Having parallel code on my mind I am worried now how this works when
> > called concurrently from two threads. There is a lot of code in libdwfl
> > that isn't concurrent-safe at the moment. But if possible lets not
> > introduce more. Not a big concern, but nice if you could give it a
> > thought.
> 
> The new client-side code doesn't call into elfutils though, so does
> not amplify hypothetical problems there.  In fact I'm not sure it's
> multithreaded at all, or just nonblocking I/O.
> 
> The server is multithreaded, but that's relatively unavoidable, and
> turns out to work without any valgrind complaints (last time I tried).

Again, I wasn't even thinking that far. I was just concerned about the
library/function call setup being not concurrent-safe code. So maybe
simply move it to dwfl_begin?

> > Similarly, our error reporting is already pretty poor, so you aren't
> > making things worse. But have you thought about a way for the libdwfl
> > user to provide some way to indicate why something couldn't be
> > resolved/found? Again, not really a big concern since the current code
> > already has very limited/poor error reporting, but maybe you have
> > thoughts about it?
> 
> We document returning standard errnos generally, and a few particular
> ones for network unreachability.

But those aren't propagated to users for libdwfl find_elf or
find_debuginfo. I was really just wondering if you thought about that
in the scope of the libdwfl calls. Currently there is no real good way
to do any error reporting there. So it isn't a big concern. But if you
have any ideas for improvement that would be nice.

> > Have you thought about just calling debuginfod-find from the libdwfl
> > code? Or is execing from a library really just a no-no?
> 
> It's not great as a practice.  (We do that from debuginfod to popen
> rpm2cpio but reluctantly, and carefully with signals.)  Another reason
> for calling directly is avoidance of a race condition, wherein two
> debuginfod-client processes run at the same time, and one cleans the
> cache right under the foot of the other.  From the C API, you're
> guaranteed to get a usable fd, even if the underlying file was
> unlinked while you were looking.  An fd is all elfutils needs (which I
> love).

All good points. Ignore that I even mentioned calling exec directly
please :)

Thanks,

Mark