Re: patch 3/3 debuginfod client interruptability

[Mark Wielaard <mark@klomp.org>]

[-- Attachment #1: Type: text/plain, Size: 3880 bytes --]

Hi,

On Fri, 2019-11-15 at 18:14 +0000, Pedro Alves wrote:
> On 11/15/19 5:35 PM, Mark Wielaard wrote:
> 
> > IMHO it would be best to avoid any global state from the start. Since
> > we haven't released this api yet we can make it so that it is easy to
> > have state per request object. 
> 
> +1
> 
> > In the gdb thread 
> > https://sourceware.org/ml/gdb-patches/2019-11/msg00118.html Simon
> > Marchi suggested something like:
> > 
> >     debuginfod_client *client = debuginfod_create_client ();
> >     debuginfod_set_progressfn (client, my_progress_cb);
> > 
> > With debuginfod_client * being an opaque pointer. I think that is a
> > good idea. It also matches what libelf (with Elf *) and libdw (with
> > Dwarf *) do. It also makes it easy to add other kinds of state to
> > client requests later.
> > 
> 
> We were discussing this on the #gdb channel on IRC, and I think
> Simon's idea is the best one so far. 

Attached is a variant that adds debuginfod_begin and debuginfo_end
(names matching elf/dwarf_begin/end) and adds a debuginfod_client
handle to each other function. The only state currently held is the
progressfn callback. But it could be extended to hold other state that
is now somewhat global (like the cache_path, server_urls, timeout which
come from the environment variables when set), or that are setup each
time a find call is made (like the handle_data array).

The full interface now looks as follows:

/* Names of environment variables that control the client logic. */
#define DEBUGINFOD_URLS_ENV_VAR "DEBUGINFOD_URLS"
#define DEBUGINFOD_CACHE_PATH_ENV_VAR "DEBUGINFOD_CACHE_PATH"
#define DEBUGINFOD_TIMEOUT_ENV_VAR "DEBUGINFOD_TIMEOUT"


/* Handle for debuginfod-client connection.  */
typedef struct debuginfod_client debuginfod_client;

/* Create a handle for a new debuginfod-client session.  */
debuginfod_client *debuginfod_begin (void);

/* Query the urls contained in $DEBUGINFOD_URLS for a file with
   the specified type and build id.  If build_id_len == 0, the
   build_id is supplied as a lowercase hexadecimal string; otherwise
   it is a binary blob of given legnth.

   If successful, return a file descriptor to the target, otherwise
   return a posix error code.  If successful, set *path to a
   strdup'd copy of the name of the same file in the cache.
   Caller must free() it later. */

int debuginfod_find_debuginfo (debuginfod_client *client,
                               const unsigned char *build_id,
                               int build_id_len,
                               char **path);

int debuginfod_find_executable (debuginfod_client *client,
                                const unsigned char *build_id,
                                int build_id_len,
                                char **path);

int debuginfod_find_source (debuginfod_client *client,
                            const unsigned char *build_id,
                            int build_id_len,
                            const char *filename,
                            char **path);

typedef int (*debuginfod_progressfn_t)(debuginfod_client *c, long a, long b);
void debuginfod_set_progressfn(debuginfod_client *c,
                               debuginfod_progressfn_t fn);

/* Release debuginfod client connection context handle.  */
void debuginfod_end (debuginfod_client *client);

I think the debuginfod_set_progressfn could use a bit more
documentation. It isn't immediately clear what a and b refer to. And
since we seem to use it in different phases maybe it needs an
"explanation message"?

For libdwfl I put all logic in a new file libdwfl/debuginfod-client.c
which sets up a debuginfod_client for each Dwfl that might need it.

The attached patch is against the debuginfod-submit branch on
sourceware elfutils.git.

Cheers,

Mark

[-- Attachment #2: 0001-debuginfod-client-context.patch --]
[-- Type: text/x-patch, Size: 23594 bytes --]

From ed7a78599243a95d8723aef506ee865ce402f9f5 Mon Sep 17 00:00:00 2001
From: Mark Wielaard <mark@klomp.org>
Date: Sun, 17 Nov 2019 22:17:26 +0100
Subject: [PATCH] debuginfod client context

---
 debuginfod/debuginfod-client.c   |  72 ++++++++++++------
 debuginfod/debuginfod-find.c     |  29 ++++++--
 debuginfod/debuginfod.cxx        |  35 ++++++---
 debuginfod/debuginfod.h          |  23 ++++--
 debuginfod/libdebuginfod.map     |   2 +
 libdwfl/Makefile.am              |   2 +-
 libdwfl/debuginfod-client.c      | 122 +++++++++++++++++++++++++++++++
 libdwfl/dwfl_build_id_find_elf.c |  37 +++-------
 libdwfl/dwfl_end.c               |   2 +
 libdwfl/find-debuginfo.c         |  28 +------
 libdwfl/libdwflP.h               |  15 ++++
 11 files changed, 267 insertions(+), 100 deletions(-)
 create mode 100644 libdwfl/debuginfod-client.c

diff --git a/debuginfod/debuginfod-client.c b/debuginfod/debuginfod-client.c
index 9bcf1452..51f612ee 100644
--- a/debuginfod/debuginfod-client.c
+++ b/debuginfod/debuginfod-client.c
@@ -71,6 +71,16 @@
   #include <fts.h>
 #endif
 
+struct debuginfod_client
+{
+  /* Progress/interrupt callback function. */
+  debuginfod_progressfn_t progressfn;
+
+  /* Can contain all other context, like cache_path, server_urls,
+     timeout or other info gotten from environment variables, the
+     handle data, etc. So those don't have to be reparsed and
+     recreated on each request.  */
+};
 
 /* The cache_clean_interval_s file within the debuginfod cache specifies
    how frequently the cache should be cleaned. The file's st_mtime represents
@@ -99,9 +109,6 @@ static const char url_delim_char = ' ';
 static const char *server_timeout_envvar = DEBUGINFOD_TIMEOUT_ENV_VAR;
 static int server_timeout = 5;
 
-/* Progress/interrupt callback function. */
-static debuginfod_progressfn_t progressfn;
-
 /* Data associated with a particular CURL easy handle. Passed to
    the write callback.  */
 struct handle_data
@@ -181,7 +188,9 @@ debuginfod_init_cache (char *cache_path, char *interval_path, char *maxage_path)
 /* Delete any files that have been unmodied for a period
    longer than $DEBUGINFOD_CACHE_CLEAN_INTERVAL_S.  */
 static int
-debuginfod_clean_cache(char *cache_path, char *interval_path, char *max_unused_path)
+debuginfod_clean_cache(debuginfod_client *c,
+		       char *cache_path, char *interval_path,
+		       char *max_unused_path)
 {
   struct stat st;
   FILE *interval_file;
@@ -236,8 +245,8 @@ debuginfod_clean_cache(char *cache_path, char *interval_path, char *max_unused_p
   while ((f = fts_read(fts)) != NULL)
     {
       files++;
-      if (progressfn) /* inform/check progress callback */
-        if ((*progressfn) (files, 0))
+      if (c->progressfn) /* inform/check progress callback */
+        if ((c->progressfn) (c, files, 0))
           break;
 
       switch (f->fts_info)
@@ -275,7 +284,8 @@ debuginfod_clean_cache(char *cache_path, char *interval_path, char *max_unused_p
    descriptor for the target, otherwise return an error code.
 */
 static int
-debuginfod_query_server (const unsigned char *build_id,
+debuginfod_query_server (debuginfod_client *c,
+			 const unsigned char *build_id,
                          int build_id_len,
                          const char *type,
                          const char *filename,
@@ -366,7 +376,7 @@ debuginfod_query_server (const unsigned char *build_id,
   int rc = debuginfod_init_cache(cache_path, interval_path, maxage_path);
   if (rc != 0)
     goto out;
-  rc = debuginfod_clean_cache(cache_path, interval_path, maxage_path);
+  rc = debuginfod_clean_cache(c, cache_path, interval_path, maxage_path);
   if (rc != 0)
     goto out;
 
@@ -501,7 +511,7 @@ debuginfod_query_server (const unsigned char *build_id,
     {
       CURLMcode curl_res;
 
-      if (progressfn) /* inform/check progress callback */
+      if (c->progressfn) /* inform/check progress callback */
         {
           loops ++;
           long pa = loops; /* default params for progress callback */
@@ -541,7 +551,7 @@ debuginfod_query_server (const unsigned char *build_id,
 #endif
             }
 
-          if ((*progressfn) (pa, pb))
+          if ((*c->progressfn) (c, pa, pb))
             break;
         }
 
@@ -674,41 +684,59 @@ debuginfod_query_server (const unsigned char *build_id,
   return rc;
 }
 
-
 /* See debuginfod.h  */
+debuginfod_client  *
+debuginfod_begin (void)
+{
+  debuginfod_client *client;
+  size_t size = sizeof (struct debuginfod_client);
+  client = (debuginfod_client *) malloc (size);
+  if (client != NULL)
+    client->progressfn = NULL;
+  return client;
+}
+
+void
+debuginfod_end (debuginfod_client *client)
+{
+  free (client);
+}
+
 int
-debuginfod_find_debuginfo (const unsigned char *build_id, int build_id_len,
+debuginfod_find_debuginfo (debuginfod_client *client,
+			   const unsigned char *build_id, int build_id_len,
                            char **path)
 {
-  return debuginfod_query_server(build_id, build_id_len,
+  return debuginfod_query_server(client, build_id, build_id_len,
                                  "debuginfo", NULL, path);
 }
 
 
 /* See debuginfod.h  */
 int
-debuginfod_find_executable(const unsigned char *build_id, int build_id_len,
+debuginfod_find_executable(debuginfod_client *client,
+			   const unsigned char *build_id, int build_id_len,
                            char **path)
 {
-  return debuginfod_query_server(build_id, build_id_len,
+  return debuginfod_query_server(client, build_id, build_id_len,
                                  "executable", NULL, path);
 }
 
 /* See debuginfod.h  */
-int debuginfod_find_source(const unsigned char *build_id, int build_id_len,
+int debuginfod_find_source(debuginfod_client *client,
+			   const unsigned char *build_id, int build_id_len,
                            const char *filename, char **path)
 {
-  return debuginfod_query_server(build_id, build_id_len,
+  return debuginfod_query_server(client, build_id, build_id_len,
                                  "source", filename, path);
 }
 
 
-debuginfod_progressfn_t
-debuginfod_set_progressfn(debuginfod_progressfn_t fn)
+void
+debuginfod_set_progressfn(debuginfod_client *client,
+			  debuginfod_progressfn_t fn)
 {
-  debuginfod_progressfn_t it = progressfn;
-  progressfn = fn;
-  return it;
+  client->progressfn = fn;
 }
 
 
diff --git a/debuginfod/debuginfod-find.c b/debuginfod/debuginfod-find.c
index 4c1a94c6..8bd3a3db 100644
--- a/debuginfod/debuginfod-find.c
+++ b/debuginfod/debuginfod-find.c
@@ -49,9 +49,11 @@ static const struct argp_option options[] =
    { NULL, 0, NULL, 0, NULL, 0 }
   };
 
+/* debuginfod connection handle.  */
+static debuginfod_client *client;
 
-
-int progressfn(long a, long b)
+int progressfn(debuginfod_client *c __attribute__((__unused__)),
+	       long a, long b)
 {
   fprintf (stderr, "Progress %ld / %ld\n", a, b);
   return 0;
@@ -64,7 +66,7 @@ static error_t parse_opt (int key, char *arg, struct argp_state *state)
   (void) state;
   switch (key)
     {
-    case 'v': debuginfod_set_progressfn (& progressfn); break;
+    case 'v': debuginfod_set_progressfn (client, & progressfn); break;
     default: return ARGP_ERR_UNKNOWN;
     }
   return 0;
@@ -82,6 +84,13 @@ static struct argp argp =
 int
 main(int argc, char** argv)
 {
+  client = debuginfod_begin ();
+  if (client == NULL)
+    {
+      fprintf(stderr, "Couldn't create debuginfod client context\n");
+      return 1;
+    }
+
   int remaining;
   (void) argp_parse (&argp, argc, argv, ARGP_IN_ORDER|ARGP_NO_ARGS, &remaining, NULL);
 
@@ -98,9 +107,13 @@ main(int argc, char** argv)
      debuginfod_find_* function. If FILETYPE is "source"
      then ensure a FILENAME was also supplied as an argument.  */
   if (strcmp(argv[remaining], "debuginfo") == 0)
-    rc = debuginfod_find_debuginfo((unsigned char *)argv[remaining+1], 0, &cache_name);
+    rc = debuginfod_find_debuginfo(client,
+				   (unsigned char *)argv[remaining+1], 0,
+				   &cache_name);
   else if (strcmp(argv[remaining], "executable") == 0)
-    rc = debuginfod_find_executable((unsigned char *)argv[remaining+1], 0, &cache_name);
+    rc = debuginfod_find_executable(client,
+				    (unsigned char *)argv[remaining+1], 0,
+				    &cache_name);
   else if (strcmp(argv[remaining], "source") == 0)
     {
       if (remaining+2 == argc || argv[3][0] != '/')
@@ -108,8 +121,8 @@ main(int argc, char** argv)
           fprintf(stderr, "If FILETYPE is \"source\" then absolute /FILENAME must be given\n");
           return 1;
         }
-      rc = debuginfod_find_source((unsigned char *)argv[remaining+1], 0,
-                                 argv[remaining+2], &cache_name);
+      rc = debuginfod_find_source(client, (unsigned char *)argv[remaining+1],
+				  0, argv[remaining+2], &cache_name);
     }
   else
     {
@@ -126,5 +139,7 @@ main(int argc, char** argv)
   printf("%s\n", cache_name);
 
   free (cache_name);
+  debuginfod_end (client);
+
   return 0;
 }
diff --git a/debuginfod/debuginfod.cxx b/debuginfod/debuginfod.cxx
index a4d44f11..37b25847 100644
--- a/debuginfod/debuginfod.cxx
+++ b/debuginfod/debuginfod.cxx
@@ -365,6 +365,7 @@ static struct argp argp =
   };
 
 
+static debuginfod_client *client;
 static string db_path;
 static sqlite3 *db;
 static unsigned verbose;
@@ -949,7 +950,7 @@ handle_buildid_match (int64_t b_mtime,
 
 
 static int
-debuginfod_find_progress (long a, long b)
+debuginfod_find_progress (debuginfod_client *, long a, long b)
 {
   if (verbose > 4)
     obatched(clog) << "federated debuginfod progress=" << a << "/" << b << endl;
@@ -1041,15 +1042,24 @@ static struct MHD_Response* handle_buildid (const string& buildid /* unsafe */,
   // is to defer to other debuginfo servers.
 
   int fd = -1;
-  if (artifacttype == "debuginfo")
-    fd = debuginfod_find_debuginfo ((const unsigned char*) buildid.c_str(), 0,
-                                   NULL);
-  else if (artifacttype == "executable")
-    fd = debuginfod_find_executable ((const unsigned char*) buildid.c_str(), 0,
-                                    NULL);
-  else if (artifacttype == "source")
-    fd = debuginfod_find_source ((const unsigned char*) buildid.c_str(), 0,
-                                suffix.c_str(), NULL);
+  if (client != NULL)
+    {
+      if (artifacttype == "debuginfo")
+	fd = debuginfod_find_debuginfo (client,
+					(const unsigned char*) buildid.c_str(),
+					0, NULL);
+      else if (artifacttype == "executable")
+	fd = debuginfod_find_executable (client,
+					 (const unsigned char*) buildid.c_str(),
+					 0, NULL);
+      else if (artifacttype == "source")
+	fd = debuginfod_find_source (client,
+				     (const unsigned char*) buildid.c_str(),
+				     0, suffix.c_str(), NULL);
+    }
+  else
+    fd = -ENOSYS;
+
   if (fd >= 0)
     {
       inc_metric ("http_responses_total","result","upstream");
@@ -2508,7 +2518,9 @@ main (int argc, char *argv[])
              "cannot run database schema ddl: %s", sqlite3_errmsg(db));
     }
 
-  (void) debuginfod_set_progressfn (& debuginfod_find_progress);
+  client = debuginfod_begin ();
+  if (client != NULL)
+    debuginfod_set_progressfn (client, & debuginfod_find_progress);
 
   // Start httpd server threads.  Separate pool for IPv4 and IPv6, in
   // case the host only has one protocol stack.
@@ -2637,6 +2649,7 @@ main (int argc, char *argv[])
   if (d6) MHD_stop_daemon (d6);
 
   /* With all threads known dead, we can clean up the global resources. */
+  debuginfod_end (client);
   delete scan_concurrency_sem;
   rc = sqlite3_exec (db, DEBUGINFOD_SQLITE_CLEANUP_DDL, NULL, NULL, NULL);
   if (rc != SQLITE_OK)
diff --git a/debuginfod/debuginfod.h b/debuginfod/debuginfod.h
index 0620f02a..6b1b1cc3 100644
--- a/debuginfod/debuginfod.h
+++ b/debuginfod/debuginfod.h
@@ -34,10 +34,16 @@
 #define DEBUGINFOD_CACHE_PATH_ENV_VAR "DEBUGINFOD_CACHE_PATH"
 #define DEBUGINFOD_TIMEOUT_ENV_VAR "DEBUGINFOD_TIMEOUT"
 
+/* Handle for debuginfod-client connection.  */
+typedef struct debuginfod_client debuginfod_client;
+
 #ifdef __cplusplus
 extern "C" {
 #endif
 
+/* Create a handle for a new debuginfod-client session.  */
+debuginfod_client *debuginfod_begin (void);
+
 /* Query the urls contained in $DEBUGINFOD_URLS for a file with
    the specified type and build id.  If build_id_len == 0, the
    build_id is supplied as a lowercase hexadecimal string; otherwise
@@ -48,21 +54,28 @@ extern "C" {
    strdup'd copy of the name of the same file in the cache.
    Caller must free() it later. */
   
-int debuginfod_find_debuginfo (const unsigned char *build_id,
+int debuginfod_find_debuginfo (debuginfod_client *client,
+			       const unsigned char *build_id,
                                int build_id_len,
                                char **path);
 
-int debuginfod_find_executable (const unsigned char *build_id,
+int debuginfod_find_executable (debuginfod_client *client,
+				const unsigned char *build_id,
                                 int build_id_len,
                                 char **path);
 
-int debuginfod_find_source (const unsigned char *build_id,
+int debuginfod_find_source (debuginfod_client *client,
+			    const unsigned char *build_id,
                             int build_id_len,
                             const char *filename,
                             char **path);
 
-typedef int (*debuginfod_progressfn_t)(long a, long b);
-debuginfod_progressfn_t debuginfod_set_progressfn(debuginfod_progressfn_t fn);
+typedef int (*debuginfod_progressfn_t)(debuginfod_client *c, long a, long b);
+void debuginfod_set_progressfn(debuginfod_client *c,
+			       debuginfod_progressfn_t fn);
+
+/* Release debuginfod client connection context handle.  */
+void debuginfod_end (debuginfod_client *client);
 
 #ifdef __cplusplus
 }
diff --git a/debuginfod/libdebuginfod.map b/debuginfod/libdebuginfod.map
index b322cba6..0d26f93e 100644
--- a/debuginfod/libdebuginfod.map
+++ b/debuginfod/libdebuginfod.map
@@ -1,6 +1,8 @@
 ELFUTILS_0 { };
 ELFUTILS_0.178 {
   global:
+  debuginfod_begin;
+  debuginfod_end;
   debuginfod_find_debuginfo;
   debuginfod_find_executable;
   debuginfod_find_source;
diff --git a/libdwfl/Makefile.am b/libdwfl/Makefile.am
index 29046e9e..47bd62a5 100644
--- a/libdwfl/Makefile.am
+++ b/libdwfl/Makefile.am
@@ -70,7 +70,7 @@ libdwfl_a_SOURCES = dwfl_begin.c dwfl_end.c dwfl_error.c dwfl_version.c \
 		    link_map.c core-file.c open.c image-header.c \
 		    dwfl_frame.c frame_unwind.c dwfl_frame_pc.c \
 		    linux-pid-attach.c linux-core-attach.c dwfl_frame_regs.c \
-		    gzip.c
+		    gzip.c debuginfod-client.c
 
 if BZLIB
 libdwfl_a_SOURCES += bzip2.c
diff --git a/libdwfl/debuginfod-client.c b/libdwfl/debuginfod-client.c
new file mode 100644
index 00000000..37a4c71f
--- /dev/null
+++ b/libdwfl/debuginfod-client.c
@@ -0,0 +1,122 @@
+/* Try to get an ELF or debug file through the debuginfod.
+   Copyright (C) 2019 Red Hat, Inc.
+   This file is part of elfutils.
+
+   This file is free software; you can redistribute it and/or modify
+   it under the terms of either
+
+     * the GNU Lesser General Public License as published by the Free
+       Software Foundation; either version 3 of the License, or (at
+       your option) any later version
+
+   or
+
+     * the GNU General Public License as published by the Free
+       Software Foundation; either version 2 of the License, or (at
+       your option) any later version
+
+   or both in parallel, as here.
+
+   elfutils is distributed in the hope that it will be useful, but
+   WITHOUT ANY WARRANTY; without even the implied warranty of
+   MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
+   General Public License for more details.
+
+   You should have received copies of the GNU General Public License and
+   the GNU Lesser General Public License along with this program.  If
+   not, see <http://www.gnu.org/licenses/>.  */
+
+#ifdef HAVE_CONFIG_H
+# include <config.h>
+#endif
+
+#include "libdwflP.h"
+#include <dlfcn.h>
+
+static __typeof__ (debuginfod_begin) *fp_debuginfod_begin;
+static __typeof__ (debuginfod_find_executable) *fp_debuginfod_find_executable;
+static __typeof__ (debuginfod_find_debuginfo) *fp_debuginfod_find_debuginfo;
+static __typeof__ (debuginfod_end) *fp_debuginfod_end;
+
+/* NB: this is slightly thread-unsafe */
+
+static debuginfod_client *
+get_client (Dwfl *dwfl)
+{
+  if (dwfl->debuginfod != NULL)
+    return dwfl->debuginfod;
+
+  if (fp_debuginfod_begin == NULL)
+    {
+      void *debuginfod_so = dlopen("libdebuginfod-" VERSION ".so", RTLD_LAZY);
+
+      if (debuginfod_so == NULL)
+	debuginfod_so = dlopen("libdebuginfod.so", RTLD_LAZY);
+
+      if (debuginfod_so != NULL)
+	{
+	  fp_debuginfod_begin = dlsym (debuginfod_so, "debuginfod_begin");
+	  fp_debuginfod_find_executable = dlsym (debuginfod_so,
+						 "debuginfod_find_executable");
+	  fp_debuginfod_find_debuginfo = dlsym (debuginfod_so,
+						"debuginfod_find_debuginfo");
+	  fp_debuginfod_end = dlsym (debuginfod_so, "debuginfod_end");
+	}
+
+      if (fp_debuginfod_begin == NULL
+	  || fp_debuginfod_find_executable == NULL
+	  || fp_debuginfod_find_debuginfo == NULL
+	  || fp_debuginfod_end == NULL)
+	fp_debuginfod_begin = (void *) -1; /* never try again */
+    }
+
+  if (fp_debuginfod_begin != NULL
+      && fp_debuginfod_begin != (void *) -1)
+    {
+      dwfl->debuginfod = (*fp_debuginfod_begin) ();
+      return dwfl->debuginfod;
+    }
+
+  return NULL;
+}
+
+int
+__libdwfl_debuginfod_find_executable (Dwfl *dwfl,
+				      const unsigned char *build_id_bits,
+				      size_t build_id_len)
+{
+  int fd = -1;
+  if (build_id_len > 0)
+    {
+      debuginfod_client *c = get_client (dwfl);
+      if (c != NULL)
+	fd = (*fp_debuginfod_find_executable) (c, build_id_bits,
+					       build_id_len, NULL);
+    }
+
+  return fd;
+}
+
+int
+__libdwfl_debuginfod_find_debuginfo (Dwfl *dwfl,
+				     const unsigned char *build_id_bits,
+				     size_t build_id_len)
+{
+  int fd = -1;
+  if (build_id_len > 0)
+    {
+      debuginfod_client *c = get_client (dwfl);
+      if (c != NULL)
+	fd = (*fp_debuginfod_find_debuginfo) (c, build_id_bits,
+					      build_id_len, NULL);
+    }
+
+  return fd;
+}
+
+void
+__libdwfl_debuginfod_end (debuginfod_client *c)
+{
+  if (c != NULL)
+    (*fp_debuginfod_end) (c);
+}
diff --git a/libdwfl/dwfl_build_id_find_elf.c b/libdwfl/dwfl_build_id_find_elf.c
index c8116468..4e56143f 100644
--- a/libdwfl/dwfl_build_id_find_elf.c
+++ b/libdwfl/dwfl_build_id_find_elf.c
@@ -34,9 +34,7 @@
 #include <inttypes.h>
 #include <fcntl.h>
 #include <unistd.h>
-#include <dlfcn.h>
 #include "system.h"
-#include "debuginfod.h"
 
 
 int
@@ -189,32 +187,15 @@ dwfl_build_id_find_elf (Dwfl_Module *mod,
       free (*file_name);
       *file_name = NULL;
     }
-  else {
-    /* NB: this is slightly thread-unsafe */
-    /* NB: see also similar code in find-debuginfo.c */
-    static __typeof__ (debuginfod_find_executable) *fp_debuginfod_find_executable;
-
-    if (fp_debuginfod_find_executable == NULL)
-      {
-        void *debuginfod_so = dlopen("libdebuginfod-" VERSION ".so", RTLD_LAZY);
-        if (debuginfod_so == NULL)
-          debuginfod_so = dlopen("libdebuginfod.so", RTLD_LAZY);
-        if (debuginfod_so != NULL)
-          fp_debuginfod_find_executable = dlsym (debuginfod_so, "debuginfod_find_executable");
-        if (fp_debuginfod_find_executable == NULL)
-          fp_debuginfod_find_executable = (void *) -1; /* never try again */
-      }
-
-    if (fp_debuginfod_find_executable != NULL && fp_debuginfod_find_executable != (void *) -1)
-      {
-        /* If all else fails and a build-id is available, query the
-           debuginfo-server if enabled.  */
-        if (fd < 0 && mod->build_id_len > 0)
-          fd = (*fp_debuginfod_find_executable) (mod->build_id_bits,
-                                                mod->build_id_len,
-                                                NULL);
-      }
-  }
+  else
+    {
+      /* If all else fails and a build-id is available, query the
+	 debuginfo-server if enabled.  */
+      if (fd < 0 && mod->build_id_len > 0)
+	fd = __libdwfl_debuginfod_find_executable (mod->dwfl,
+						   mod->build_id_bits,
+						   mod->build_id_len);
+    }
 
   if (fd < 0 && errno == 0 && mod->build_id_len > 0)
     /* Setting this with no file yet loaded is a marker that
diff --git a/libdwfl/dwfl_end.c b/libdwfl/dwfl_end.c
index 74ee9e07..4f6c722a 100644
--- a/libdwfl/dwfl_end.c
+++ b/libdwfl/dwfl_end.c
@@ -39,6 +39,8 @@ dwfl_end (Dwfl *dwfl)
   if (dwfl == NULL)
     return;
 
+  __libdwfl_debuginfod_end (dwfl->debuginfod);
+
   if (dwfl->process)
     __libdwfl_process_free (dwfl->process);
 
diff --git a/libdwfl/find-debuginfo.c b/libdwfl/find-debuginfo.c
index 5aa0c5bb..40857645 100644
--- a/libdwfl/find-debuginfo.c
+++ b/libdwfl/find-debuginfo.c
@@ -31,11 +31,9 @@
 #endif
 
 #include "libdwflP.h"
-#include "debuginfod.h"
 #include <stdio.h>
 #include <fcntl.h>
 #include <unistd.h>
-#include <dlfcn.h>
 #include <sys/stat.h>
 #include "system.h"
 
@@ -400,30 +398,8 @@ dwfl_standard_find_debuginfo (Dwfl_Module *mod,
       free (canon);
     }
 
-  {
-    /* NB: this is slightly thread-unsafe */
-    /* NB: see also similar code in find-debuginfo.c */
-    static __typeof__ (debuginfod_find_debuginfo) *fp_debuginfod_find_debuginfo;
-
-    if (fp_debuginfod_find_debuginfo == NULL)
-      {
-        void *debuginfod_so = dlopen("libdebuginfod-" VERSION ".so", RTLD_LAZY);
-        if (debuginfod_so == NULL)
-          debuginfod_so = dlopen("libdebuginfod.so", RTLD_LAZY);
-        if (debuginfod_so != NULL)
-          fp_debuginfod_find_debuginfo = dlsym (debuginfod_so, "debuginfod_find_debuginfo");
-        if (fp_debuginfod_find_debuginfo == NULL)
-          fp_debuginfod_find_debuginfo = (void *) -1; /* never try again */
-      }
-
-    if (fp_debuginfod_find_debuginfo != NULL && fp_debuginfod_find_debuginfo != (void *) -1)
-      {
-        /* If all else fails and a build-id is available, query the
-           debuginfo-server if enabled.  */
-        if (fd < 0 && bits_len > 0)
-          fd = (*fp_debuginfod_find_debuginfo) (bits, bits_len, NULL);
-      }
-  }
+  if (fd < 0 && bits_len > 0)
+    fd = __libdwfl_debuginfod_find_debuginfo (mod->dwfl, bits, bits_len);
 
   return fd;
 }
diff --git a/libdwfl/libdwflP.h b/libdwfl/libdwflP.h
index 941a8b66..3a93fc01 100644
--- a/libdwfl/libdwflP.h
+++ b/libdwfl/libdwflP.h
@@ -40,6 +40,7 @@
 
 #include "../libdw/libdwP.h"	/* We need its INTDECLs.  */
 #include "../libdwelf/libdwelfP.h"
+#include "../debuginfod/debuginfod.h"
 
 typedef struct Dwfl_Process Dwfl_Process;
 
@@ -114,6 +115,7 @@ struct Dwfl_User_Core
 struct Dwfl
 {
   const Dwfl_Callbacks *callbacks;
+  debuginfod_client *debuginfod;
 
   Dwfl_Module *modulelist;    /* List in order used by full traversals.  */
 
@@ -635,6 +637,19 @@ extern Dwfl_Error __libdw_open_elf (int fd, Elf **elfp) internal_function;
 extern bool __libdwfl_dynamic_vaddr_get (Elf *elf, GElf_Addr *vaddrp)
   internal_function;
 
+/* Internal interface to libdebuginfod (if installed).  */
+int
+__libdwfl_debuginfod_find_executable (Dwfl *dwfl,
+				      const unsigned char *build_id_bits,
+				      size_t build_id_len);
+int
+__libdwfl_debuginfod_find_debuginfo (Dwfl *dwfl,
+				     const unsigned char *build_id_bits,
+				     size_t build_id_len);
+void
+__libdwfl_debuginfod_end (debuginfod_client *c);
+
+
 /* These are working nicely for --core, but are not ready to be
    exported interfaces quite yet.  */
 
-- 
2.18.1