Hi,
On Sun, 2019-11-17 at 21:50 -0500, Frank Ch. Eigler wrote:
> > 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.
>
> Sure, if you like.
OK, I rebased on the debuginfod-submit branch and added documentation
(see attached).
> Would you be sympathetic to supporting a
> client=NULL entrypoint to the lookup functions, ergo no begin/end, for
> applications that don't want a progressfn callback? That way the
> simple case looks simple.
I think it is better to be consistent and always require a valid client
connection handle for the reasons that Pedro gave.
> > diff --git a/debuginfod/debuginfod.cxx b/debuginfod/debuginfod.cxx
> >
> > +static debuginfod_client *client;
>
> Note that multiple http webapi handling threads may make
> federated debuginfod calls concurrently. Is it your idea
> that they share a single client object?
No, it would be better to give every thread it own handle.
The attached patch does that.
All patches are also on
https://code.wildebeest.org/git/user/mjw/elfutils/log/?h=debuginfod-client-context
If you think those patches are ok I can squash them and add them to the
debuginfod-submit branch.
Cheers,
Mark
From 8006db933298ecca35a04207aad3991b242c21da Mon Sep 17 00:00:00 2001
From: Mark Wielaard <m...@klomp.org>
Date: Tue, 19 Nov 2019 13:09:24 +0100
Subject: [PATCH] update documentation for client connection handle usage
---
doc/debuginfod_begin.3 | 1 +
doc/debuginfod_end.3 | 1 +
doc/debuginfod_find_debuginfo.3 | 51 ++++++++++++++++++++++++++-------
3 files changed, 42 insertions(+), 11 deletions(-)
create mode 100644 doc/debuginfod_begin.3
create mode 100644 doc/debuginfod_end.3
diff --git a/doc/debuginfod_begin.3 b/doc/debuginfod_begin.3
new file mode 100644
index 00000000..16279936
--- /dev/null
+++ b/doc/debuginfod_begin.3
@@ -0,0 +1 @@
+.so man3/debuginfod_find_debuginfo.3
diff --git a/doc/debuginfod_end.3 b/doc/debuginfod_end.3
new file mode 100644
index 00000000..16279936
--- /dev/null
+++ b/doc/debuginfod_end.3
@@ -0,0 +1 @@
+.so man3/debuginfod_find_debuginfo.3
diff --git a/doc/debuginfod_find_debuginfo.3 b/doc/debuginfod_find_debuginfo.3
index b65a6c57..66619157 100644
--- a/doc/debuginfod_find_debuginfo.3
+++ b/doc/debuginfod_find_debuginfo.3
@@ -21,15 +21,39 @@ debuginfod_find_debuginfo \- request debuginfo from debuginfod
.nf
.B #include <elfutils/debuginfod.h>
.PP
-.BI "int debuginfod_find_debuginfo(const unsigned char *" build_id ", int " build_id_len ", char ** " path ");"
-.BI "int debuginfod_find_executable(const unsigned char *" build_id ", int " build_id_len ", char ** " path ");"
-.BI "int debuginfod_find_source(const unsigned char *" build_id ", int " build_id_len ", const char *" filename ", char ** " path ");"
-.BI "typedef int (*debuginfo_progressfn_t)(long a, long b);"
-.BI "debuginfo_progressfn_t debuginfod_set_progressfn(debuginfo_progressfn_t " progressfn ");"
+.BI "debuginfod_client *debuginfod_begin(void);"
+.BI "void debuginfod_end(debuginfod_client *" client ");"
+
+.BI "int debuginfod_find_debuginfo(debuginfod_client *" client ","
+.BI " const unsigned char *" build_id ","
+.BI " int " build_id_len ","
+.BI " char ** " path ");"
+.BI "int debuginfod_find_executable(debuginfod_client *" client ","
+.BI " const unsigned char *" build_id ","
+.BI " int " build_id_len ","
+.BI " char ** " path ");"
+.BI "int debuginfod_find_source(debuginfod_client *" client ","
+.BI " const unsigned char *" build_id ","
+.BI " int " build_id_len ","
+.BI " const char *" filename ","
+.BI " char ** " path ");"
+
+.BI "typedef int (*debuginfo_progressfn_t)(debuginfod_client *" client ","
+.BI " long a, long b);"
+.BI "void debuginfod_set_progressfn(debuginfod_client *" client ","
+.BI " debuginfo_progressfn_t " progressfn ");"
Link with \fB-ldebuginfod\fP.
.SH DESCRIPTION
+
+.BR debuginfod_begin ()
+creates a \fBdebuginfod_client\fP connection handle that should be used
+with all other calls.
+.BR debuginfod_end ()
+should be called on the \fBclient\fP handle to release all state and
+storage when done.
+
.BR debuginfod_find_debuginfo (),
.BR debuginfod_find_executable (),
and
@@ -65,9 +89,14 @@ The URLs in \fB$DEBUGINFOD_URLS\fP may be queried in parallel. As soon
as a debuginfod server begins transferring the target file all of the
connections to the other servers are closed.
-These functions are MT-safe.
+A \fBclient\fP handle should be used from only one thread at a time.
.SH "RETURN VALUE"
+
+\fBdebuginfod_begin\fP returns the \fBdebuginfod_client\fP handle to
+use with all other calls. On error \fBNULL\fP will be returned and
+\fBerrno\fP will be set.
+
If a find family function is successful, the resulting file is saved
to the client cache and a file descriptor to that file is returned.
The caller needs to \fBclose\fP() this descriptor. Otherwise, a
@@ -75,12 +104,12 @@ negative error code is returned.
.SH "PROGRESS CALLBACK"
-As the \fBdebuginfod_find_*\fP() functions may block for seconds or longer, a progress
-callback function is called periodically, if configured with
+As the \fBdebuginfod_find_*\fP() functions may block for seconds or
+longer, a progress callback function is called periodically, if
+configured with
.BR debuginfod_set_progressfn ().
-This function sets a new progress callback function (or NULL) and
-returns the previously set function (or NULL). This function may be
-MT-unsafe.
+This function sets a new progress callback function (or NULL) for the
+client handle.
The given callback function is called from the context of each thread
that is invoking any of the other lookup functions. It is given two
--
2.18.1
From 9c3817f2a52d516a88961a25e4e0d8f177b42463 Mon Sep 17 00:00:00 2001
From: Mark Wielaard <m...@klomp.org>
Date: Tue, 19 Nov 2019 13:50:41 +0100
Subject: [PATCH] debuginfod: create a client connection handle for each
handler thread
---
debuginfod/debuginfod.cxx | 12 +++++-------
1 file changed, 5 insertions(+), 7 deletions(-)
diff --git a/debuginfod/debuginfod.cxx b/debuginfod/debuginfod.cxx
index 7d3d580f..7baf18ec 100644
--- a/debuginfod/debuginfod.cxx
+++ b/debuginfod/debuginfod.cxx
@@ -360,7 +360,6 @@ static struct argp argp =
};
-static debuginfod_client *client;
static string db_path;
static sqlite3 *db;
static unsigned verbose;
@@ -1039,8 +1038,11 @@ static struct MHD_Response* handle_buildid (const string& buildid /* unsafe */,
// is to defer to other debuginfo servers.
int fd = -1;
+ debuginfod_client *client = debuginfod_begin ();
if (client != NULL)
{
+ debuginfod_set_progressfn (client, & debuginfod_find_progress);
+
if (artifacttype == "debuginfo")
fd = debuginfod_find_debuginfo (client,
(const unsigned char*) buildid.c_str(),
@@ -1055,7 +1057,8 @@ static struct MHD_Response* handle_buildid (const string& buildid /* unsafe */,
0, suffix.c_str(), NULL);
}
else
- fd = -ENOSYS;
+ fd = -errno; /* Set by debuginfod_begin. */
+ debuginfod_end (client);
if (fd >= 0)
{
@@ -2518,10 +2521,6 @@ main (int argc, char *argv[])
"cannot run database schema ddl: %s", sqlite3_errmsg(db));
}
- 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.
MHD_Daemon *d4 = MHD_start_daemon (MHD_USE_THREAD_PER_CONNECTION
@@ -2649,7 +2648,6 @@ 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)
--
2.18.1
