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 to serialize calls to
__libdwfl_debuginfod_find_*.

Signed-off-by: Aaron Merey <[email protected]>
---

v2 changes: Reverted changes to a comment for __libdwfl_debuginfod_init.
Rename dwfl->lock to dwfl->debuginfod_lock.

On Thu, Dec 4, 2025 at 7:03 PM Aaron Merey <[email protected]> wrote:
> On Wed, Dec 3, 2025 at 11:19 AM Mark Wielaard <[email protected]> wrote:
> > 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.

I experimented with adding thread safety at the libdebuginfod level
instead of libdwfl.  Reasoning about its correctness ended up being
more complex than expected, in part due to the client progressfn
callback that can be called during a query.

After some discussion with Frank I think that the effort to make
libdebuginfod thread safe isn't trivial and may not be worth the time,
as we don't currently have a clear use case that would see a worthwhile
performance benefit.

 libdwfl/debuginfod-client.c | 6 +++++-
 libdwfl/dwfl_begin.c        | 2 ++
 libdwfl/dwfl_end.c          | 1 +
 libdwfl/libdwflP.h          | 3 +++
 4 files changed, 11 insertions(+), 1 deletion(-)

diff --git a/libdwfl/debuginfod-client.c b/libdwfl/debuginfod-client.c
index 882a5eff..ecb053fc 100644
--- a/libdwfl/debuginfod-client.c
+++ b/libdwfl/debuginfod-client.c
@@ -49,7 +49,7 @@ static void __libdwfl_debuginfod_init (void);
 
 static pthread_once_t init_control = PTHREAD_ONCE_INIT;
 
-/* NB: this is slightly thread-unsafe */
+/* dwfl->debuginfod_lock must be held when calling this function.  */
 
 debuginfod_client *
 dwfl_get_debuginfod_client (Dwfl *dwfl)
@@ -77,10 +77,12 @@ __libdwfl_debuginfod_find_executable (Dwfl *dwfl,
   int fd = -1;
   if (build_id_len > 0)
     {
+      mutex_lock (dwfl->debuginfod_lock);
       debuginfod_client *c = INTUSE (dwfl_get_debuginfod_client) (dwfl);
       if (c != NULL)
        fd = (*fp_debuginfod_find_executable) (c, build_id_bits,
                                               build_id_len, NULL);
+      mutex_unlock (dwfl->debuginfod_lock);
     }
 
   return fd;
@@ -94,10 +96,12 @@ __libdwfl_debuginfod_find_debuginfo (Dwfl *dwfl,
   int fd = -1;
   if (build_id_len > 0)
     {
+      mutex_lock (dwfl->debuginfod_lock);
       debuginfod_client *c = INTUSE (dwfl_get_debuginfod_client) (dwfl);
       if (c != NULL)
        fd = (*fp_debuginfod_find_debuginfo) (c, build_id_bits,
                                              build_id_len, NULL);
+      mutex_unlock (dwfl->debuginfod_lock);
     }
 
   return fd;
diff --git a/libdwfl/dwfl_begin.c b/libdwfl/dwfl_begin.c
index b03f5cf4..835de8e3 100644
--- a/libdwfl/dwfl_begin.c
+++ b/libdwfl/dwfl_begin.c
@@ -50,6 +50,8 @@ dwfl_begin (const Dwfl_Callbacks *callbacks)
       dwfl->offline_next_address = OFFLINE_REDZONE;
     }
 
+  mutex_init (dwfl->debuginfod_lock);
+
   return dwfl;
 }
 INTDEF (dwfl_begin)
diff --git a/libdwfl/dwfl_end.c b/libdwfl/dwfl_end.c
index d9cf569b..c82d83fb 100644
--- a/libdwfl/dwfl_end.c
+++ b/libdwfl/dwfl_end.c
@@ -53,6 +53,7 @@ dwfl_end (Dwfl *dwfl)
   free (dwfl->lookup_module);
   free (dwfl->lookup_segndx);
   free (dwfl->sysroot);
+  mutex_fini (dwfl->debuginfod_lock);
 
   Dwfl_Module *next = dwfl->modulelist;
   while (next != NULL)
diff --git a/libdwfl/libdwflP.h b/libdwfl/libdwflP.h
index a5d88d60..6e394c26 100644
--- a/libdwfl/libdwflP.h
+++ b/libdwfl/libdwflP.h
@@ -141,6 +141,9 @@ struct Dwfl
   struct Dwfl_User_Core *user_core;
   char *sysroot;               /* sysroot, or NULL to search standard system
                                   paths */
+
+  /* Serialize debuginfod_client usage.  */
+  mutex_define (, debuginfod_lock);
 };
 
 #define OFFLINE_REDZONE                0x10000
-- 
2.52.0

Reply via email to