Hi Mark,
On Wed, Dec 3, 2025 at 11:19 AM Mark Wielaard <[email protected]> wrote:
>
> On Mon, 2025-12-01 at 12:17 -0500, Aaron Merey wrote:
> > Individual debuginfod_client objects and their underlying CURLM handles
> > should not be used concurrently during calls to
> > __libdwfl_debuginfod_find_debuginfo and
> > __libdwfl_debuginfod_find_executable.
> >
> > Add a mutex field to struct Dwfl and synchronize calls to
> > __libdwfl_debuginfod_find_*.
>
> So if I understand correctly there are two issues here.
>
> The debuginfo_client is created and assigned to a Dwfl field lazily. So
> you want to prevent concurrent calls to dwfl_get_debuginfod_client.
>
> A debuginfo_client has CURLM handle which cannot be used by different
> threads concurrently.
>
> Since we have one unique debuginfo_client per dwfl you use one and the
> same lock?
>
> I believe this patch should work. But have you considered having a lock
> inside debuginfod-client itself to protect access to the CURLM handles?
> That would (maybe) result is slightly simpler code. And would help
> other users of libdebuginfod that might use debuginfod_client handles
> from multiple threads (gdb?).
I agree that it is better to have dedicated per-client locks instead.
Theres no need for libdwfl functions to wait on locks held for
independent debuginfod purposes.
>
> > @@ -111,7 +115,8 @@ __libdwfl_debuginfod_end (debuginfod_client *c)
> > }
> >
> > /* Try to get the libdebuginfod library functions.
> > - Only needs to be called once from dwfl_get_debuginfod_client. */
> > + Only needs to be called once from dwfl_get_debuginfod_client.
> > + A per-Dwfl lock must be held when calling this function. */
> > static void
> > __libdwfl_debuginfod_init (void)
> > {
>
> Why does it need a per-Dwfl lock? It is already guarded by a
> pthread_once. And it doesn't seem to manipulate any Dwfl struct field
> structures (and cannot because it isn't passed a Dwfl object).
You're right since this function is only ever used by pthread_once. I
had general usage of __libdwfl_debuginfod_init in mind when I wrote
that but it's not relevant in this case.
Aaron
