patch 3/3 debuginfod client interruptability
Frank Ch. Eigler
fche@redhat.com
Mon Nov 4 21:48:00 GMT 2019
Hi -
At the wise counsel of gdb folks such as <tromey> and <simark>:
debuginfod 3/3: client interruptability
For interactive clients such as gdb, interruptibility is important for
usability during longer downloads. This patchset adds a
download-progress callback function to the debuginfod client library,
with which a caller app can interrupt a download as well as be
notified of its quantitative progress.
diff --git a/debuginfod/ChangeLog b/debuginfod/ChangeLog
index b5679a2f9d16..34713746350d 100644
--- a/debuginfod/ChangeLog
+++ b/debuginfod/ChangeLog
@@ -1,3 +1,18 @@
+2019-11-04 Frank Ch. Eigler <fche@redhat.com>
+
+ * debuginfo-client.c (debuginfod_set_progressfn): New function
+ for progress/interrupt callback.
+ (debuginfod_clean_cache, debuginfod_query_server): Call it.
+ * debuginfo.h: Declare it.
+ * debuginfod_set_progressfn.3, *_find_debuginfo.3: Document it.
+ * Makefile.am: Install it.
+ * libdebuginfod.map: Export it all under ELFUTILS_0.178 symversion.
+
+ * debuginfod-find.c: Add -v option to activate progress cb.
+ * debuginfod-find.1: Document it.
+ * debuginfod.cxx: Add $DEBUGINFOD_TEST_WEBAPI_SLEEP env var
+ to insert sleep in webapi callbacks, to help manual testing.
+
2019-10-28 Frank Ch. Eigler <fche@redhat.com>
* debuginfod.cxx: New file: debuginfod server.
diff --git a/debuginfod/Makefile.am b/debuginfod/Makefile.am
index 6e5c7290e68d..790e42317972 100644
--- a/debuginfod/Makefile.am
+++ b/debuginfod/Makefile.am
@@ -60,7 +60,7 @@ debuginfod_SOURCES = debuginfod.cxx
debuginfod_cxx_CXXFLAGS = -Wno-unused-functions
# g++ 4.8 on rhel7 otherwise errors gthr-default.h / __gthrw2(__gthrw_(__pthread_key_create) undefs
dist_man8_MANS = debuginfod.8
-dist_man3_MANS = debuginfod_find_debuginfo.3 debuginfod_find_source.3 debuginfod_find_executable.3
+dist_man3_MANS = debuginfod_find_debuginfo.3 debuginfod_find_source.3 debuginfod_find_executable.3 debuginfod_set_progressfn.3
dist_man1_MANS = debuginfod-find.1
debuginfod_LDADD = $(libdw) $(libelf) $(libeu) $(libdebuginfod) $(libmicrohttpd_LIBS) $(libcurl_LIBS) $(sqlite3_LIBS) $(libarchive_LIBS) -lpthread -ldl
diff --git a/debuginfod/debuginfod-client.c b/debuginfod/debuginfod-client.c
index dd8d99c7af17..6a1ffbf7455f 100644
--- a/debuginfod/debuginfod-client.c
+++ b/debuginfod/debuginfod-client.c
@@ -28,6 +28,7 @@
#include "config.h"
#include "debuginfod.h"
+#include "atomics.h"
#include <assert.h>
#include <dirent.h>
#include <stdio.h>
@@ -77,6 +78,9 @@ 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
@@ -207,8 +211,14 @@ debuginfod_clean_cache(char *cache_path, char *interval_path, char *max_unused_p
return -errno;
FTSENT *f;
+ long files = 0;
while ((f = fts_read(fts)) != NULL)
{
+ files++;
+ if (progressfn) /* inform/check progress callback */
+ if ((*progressfn) (files, 0))
+ break;
+
switch (f->fts_info)
{
case FTS_F:
@@ -237,7 +247,8 @@ debuginfod_clean_cache(char *cache_path, char *interval_path, char *max_unused_p
/* Query each of the server URLs found in $DEBUGINFOD_URLS for the file
with the specified build-id, type (debuginfo, executable or source)
and filename. filename may be NULL. If found, return a file
- descriptor for the target, otherwise return an error code. */
+ descriptor for the target, otherwise return an error code.
+*/
static int
debuginfod_query_server (const unsigned char *build_id,
int build_id_len,
@@ -446,10 +457,35 @@ debuginfod_query_server (const unsigned char *build_id,
/* Query servers in parallel. */
int still_running;
+ unsigned loops = 0;
do
{
CURLMcode curl_res;
+ if (progressfn) /* inform/check progress callback */
+ {
+ loops ++;
+ long pa = loops; /* default params for progress callback */
+ long pb = 0;
+ if (target_handle) /* we've committed to a server; report its download progress */
+ {
+ curl_off_t cl, dl;
+ curl_res = curl_easy_getinfo(target_handle,
+ CURLINFO_SIZE_DOWNLOAD_T,
+ &dl);
+ if (curl_res == 0 && dl >= 0)
+ pa = (dl > LONG_MAX ? LONG_MAX : (long)dl);
+
+ curl_res = curl_easy_getinfo(target_handle,
+ CURLINFO_CONTENT_LENGTH_DOWNLOAD_T,
+ &cl);
+ if (curl_res == 0 && cl >= 0)
+ pb = (cl > LONG_MAX ? LONG_MAX : (long)cl);
+ }
+ if ((*progressfn) (pa, pb))
+ break;
+ }
+
/* Wait 1 second, the minimum DEBUGINFOD_TIMEOUT. */
curl_multi_wait(curlm, NULL, 0, 1000, NULL);
@@ -600,16 +636,21 @@ debuginfod_find_executable(const unsigned char *build_id, int build_id_len,
}
/* See debuginfod.h */
-int debuginfod_find_source(const unsigned char *build_id,
- int build_id_len,
- const char *filename,
- char **path)
+int debuginfod_find_source(const unsigned char *build_id, int build_id_len,
+ const char *filename, char **path)
{
return debuginfod_query_server(build_id, build_id_len,
"source", filename, path);
}
+debuginfod_progressfn_t
+debuginfod_set_progressfn(debuginfod_progressfn_t fn)
+{
+ debuginfod_progressfn_t it = atomic_exchange (& progressfn, fn);
+ return it;
+}
+
/* NB: these are thread-unsafe. */
__attribute__((constructor)) attribute_hidden void libdebuginfod_ctor(void)
diff --git a/debuginfod/debuginfod-find.1 b/debuginfod/debuginfod-find.1
index 6d2251662a57..4c352a4f2873 100644
--- a/debuginfod/debuginfod-find.1
+++ b/debuginfod/debuginfod-find.1
@@ -18,11 +18,11 @@
debuginfod-find \- request debuginfo-related data
.SH SYNOPSIS
-.B debuginfod-find debuginfo \fIBUILDID\fP
+.B debuginfod-find [\fIOPTION\fP]... debuginfo \fIBUILDID\fP
-.B debuginfod-find executable \fIBUILDID\fP
+.B debuginfod-find [\fIOPTION\fP]... executable \fIBUILDID\fP
-.B debuginfod-find source \fIBUILDID\fP \fI/FILENAME\fP
+.B debuginfod-find [\fIOPTION\fP]... source \fIBUILDID\fP \fI/FILENAME\fP
.SH DESCRIPTION
\fBdebuginfod-find\fP queries one or more \fBdebuginfod\fP servers for
@@ -91,6 +91,13 @@ l l.
\../bar/foo.c AT_comp_dir=/zoo source BUILDID /zoo/../bar/foo.c
.TE
+.SH "OPTIONS"
+
+.TP
+.B "\-v"
+Increase verbosity, including printing frequent download-progress messages.
+
+
.SH "SECURITY"
debuginfod-find \fBdoes not\fP include any particular security
diff --git a/debuginfod/debuginfod-find.c b/debuginfod/debuginfod-find.c
index 78322fc6cd29..e26919ae7c53 100644
--- a/debuginfod/debuginfod-find.c
+++ b/debuginfod/debuginfod-find.c
@@ -51,10 +51,39 @@ static const char args_doc[] = N_("debuginfo BUILDID\n"
"executable BUILDID\n"
"source BUILDID /FILENAME");
+/* Definitions of arguments for argp functions. */
+static const struct argp_option options[] =
+ {
+ { "verbose", 'v', NULL, 0, "Increase verbosity.", 0 },
+ { NULL, 0, NULL, 0, NULL, 0 }
+ };
+
+
+
+int progressfn(long a, long b)
+{
+ fprintf (stderr, "Progress %ld / %ld\n", a, b);
+ return 0;
+}
+
+
+static error_t parse_opt (int key, char *arg, struct argp_state *state)
+{
+ (void) arg;
+ (void) state;
+ switch (key)
+ {
+ case 'v': debuginfod_set_progressfn (& progressfn); break;
+ default: return ARGP_ERR_UNKNOWN;
+ }
+ return 0;
+}
+
+
/* Data structure to communicate with argp functions. */
static struct argp argp =
{
- NULL, NULL, args_doc, doc, NULL, NULL, NULL
+ options, parse_opt, args_doc, doc, NULL, NULL, NULL
};
diff --git a/debuginfod/debuginfod.cxx b/debuginfod/debuginfod.cxx
index c9d5b271b328..fe8706cf3e07 100644
--- a/debuginfod/debuginfod.cxx
+++ b/debuginfod/debuginfod.cxx
@@ -370,6 +370,8 @@ static set<string> source_paths;
static vector<string> extra_ddl;
static regex_t file_include_regex;
static regex_t file_exclude_regex;
+static int test_webapi_sleep; /* testing only */
+
/* Handle program arguments. */
static error_t
@@ -922,6 +924,15 @@ handle_buildid_match (int64_t b_mtime,
}
+static int
+debuginfod_find_progress (long a, long b)
+{
+ if (verbose > 4)
+ obatched(clog) << "federated debuginfod progress=" << a << "/" << b << endl;
+
+ return interrupted;
+}
+
static struct MHD_Response* handle_buildid (const string& buildid /* unsafe */,
const string& artifacttype /* unsafe */,
@@ -1004,6 +1015,7 @@ static struct MHD_Response* handle_buildid (const string& buildid /* unsafe */,
// We couldn't find it in the database. Last ditch effort
// is to defer to other debuginfo servers.
+
int fd = -1;
if (artifacttype == "debuginfo")
fd = debuginfod_find_debuginfo ((const unsigned char*) buildid.c_str(), 0,
@@ -1070,6 +1082,9 @@ handler_cb (void * /*cls*/,
if (verbose)
obatched(clog) << conninfo(connection) << " " << method << " " << url << endl;
+ if (test_webapi_sleep)
+ sleep (test_webapi_sleep);
+
try
{
if (string(method) != "GET")
@@ -2303,6 +2318,10 @@ main (int argc, char *argv[])
if (rc != 0)
error (EXIT_FAILURE, 0, "regcomp failure: %d", rc);
+ const char* test_webapi_sleep_str = getenv("DEBUGINFOD_TEST_WEBAPI_SLEEP");
+ if (test_webapi_sleep_str)
+ test_webapi_sleep = atoi (test_webapi_sleep_str);
+
/* Parse and process arguments. */
int remaining;
argp_program_version_hook = print_version; // this works
@@ -2357,6 +2376,8 @@ main (int argc, char *argv[])
"cannot run database schema ddl: %s", sqlite3_errmsg(db));
}
+ (void) debuginfod_set_progressfn (& 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
diff --git a/debuginfod/debuginfod.h b/debuginfod/debuginfod.h
index c268ffebcdb6..2e597013ffbf 100644
--- a/debuginfod/debuginfod.h
+++ b/debuginfod/debuginfod.h
@@ -61,6 +61,9 @@ int debuginfod_find_source (const unsigned char *build_id,
const char *filename,
char **path);
+typedef int (*debuginfod_progressfn_t)(long a, long b);
+debuginfod_progressfn_t debuginfod_set_progressfn(debuginfod_progressfn_t fn);
+
#ifdef __cplusplus
}
#endif
diff --git a/debuginfod/debuginfod_find_debuginfo.3 b/debuginfod/debuginfod_find_debuginfo.3
index 18caec5576ba..7cd30810e252 100644
--- a/debuginfod/debuginfod_find_debuginfo.3
+++ b/debuginfod/debuginfod_find_debuginfo.3
@@ -13,7 +13,7 @@
.RE
..
-.TH DEBUGINFOD_FIND_DEBUGINFO 3
+.TH DEBUGINFOD_FIND_* 3
.SH NAME
debuginfod_find_debuginfo \- request debuginfo from debuginfod
@@ -21,9 +21,13 @@ debuginfod_find_debuginfo \- request debuginfo from debuginfod
.nf
.B #include <elfutils/debuginfod.h>
.PP
-.BI "debuginfod_find_debuginfo(const unsigned char *" build_id ", int " build_id_len ", char ** " path ");"
-.BI "debuginfod_find_executable(const unsigned char *" build_id ", int " build_id_len ", char ** " path ");"
-.BI "debuginfod_find_source(const unsigned char *" build_id ", int " build_id_len ", const char *" filename ", char ** " path ");"
+.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 ");"
+
+Link with \fB-ldebuginfod\fP.
.SH DESCRIPTION
.BR debuginfod_find_debuginfo (),
@@ -56,6 +60,35 @@ The URLs in \fB$DEBUGINFOD_URLS\fP are queried in parallel. As soon as a
debuginfod server begins transfering the target file all of the connections
to the other servers are closed.
+.SH "RETURN VALUE"
+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
+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
+.BR debuginfod_set_progressfn ().
+This function sets a new progress callback function (or NULL) and
+returns the previously set function (or NULL).
+
+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
+numeric parameters that, if thought of as a numerator \fIa\fP and
+denominator \fIb\fP, together represent a completion fraction
+\fIa/b\fP. The denominator may be zero initially, until a quantity
+such as an exact download size becomes known.
+
+The progress callback function is also the supported way to
+\fIinterrupt\fP the download operation. (The library does \fInot\fP
+modify or trigger signals.) The progress callback must return 0 to
+continue the work, or any other value to stop work as soon as
+possible. Consequently, the \fBdebuginfod_find_*\fP() function will
+likely return with an error, but might still succeed.
+
+
.SH "CACHE"
If the query is successful, the \fBdebuginfod_find_*\fP() functions save
the target file to a local cache. The location of the cache is controlled
@@ -99,11 +132,6 @@ This environment variable governs the location of the cache where
downloaded files are kept. It is cleaned periodically as this
program is reexecuted. The default is $HOME/.debuginfod_client_cache.
-.SH "RETURN VALUE"
-If the query is successful, these functions save the target file
-to the client cache and return a file descriptor to that file.
-Otherwise an error code is returned.
-
.SH "ERRORS"
The following list is not comprehensive. Error codes may also
originate from calls to various C Library functions.
diff --git a/debuginfod/debuginfod_set_progressfn.3 b/debuginfod/debuginfod_set_progressfn.3
new file mode 100644
index 000000000000..16279936e2ea
--- /dev/null
+++ b/debuginfod/debuginfod_set_progressfn.3
@@ -0,0 +1 @@
+.so man3/debuginfod_find_debuginfo.3
diff --git a/debuginfod/libdebuginfod.map b/debuginfod/libdebuginfod.map
index 50d6fbaec639..b322cba644f5 100644
--- a/debuginfod/libdebuginfod.map
+++ b/debuginfod/libdebuginfod.map
@@ -1,7 +1,8 @@
ELFUTILS_0 { };
-ELFUTILS_0.177 {
+ELFUTILS_0.178 {
global:
debuginfod_find_debuginfo;
debuginfod_find_executable;
debuginfod_find_source;
+ debuginfod_set_progressfn;
} ELFUTILS_0;
diff --git a/tests/ChangeLog b/tests/ChangeLog
index ef5f2f0f1211..7b091158cff2 100644
--- a/tests/ChangeLog
+++ b/tests/ChangeLog
@@ -1,3 +1,7 @@
+2019-11-04 Frank Ch. Eigler <fche@redhat.com>
+
+ * run-debuginfod-find.sh: Test debuginfod-find -v progress mode.
+
2019-10-28 Aaron Merey <amerey@redhat.com>
Frank Ch. Eigler <fche@redhat.com>
diff --git a/tests/run-debuginfod-find.sh b/tests/run-debuginfod-find.sh
index 2facef2cbaad..fe38eaac9c77 100755
--- a/tests/run-debuginfod-find.sh
+++ b/tests/run-debuginfod-find.sh
@@ -44,7 +44,7 @@ ldpath=`testrun sh -c 'echo $LD_LIBRARY_PATH'`
mkdir F R
tempfiles F R
-env LD_LIBRARY_PATH=$ldpath DEBUGINFOD_URLS= ${abs_builddir}/../debuginfod/debuginfod -vvvv -d $DB \
+env DEBUGINFOD_TEST_WEBAPI_SLEEP=3 LD_LIBRARY_PATH=$ldpath DEBUGINFOD_URLS= ${abs_builddir}/../debuginfod/debuginfod -vvvv -d $DB \
-p $PORT1 -t0 -g0 R F &
PID1=$!
sleep 3
@@ -105,8 +105,11 @@ kill -USR1 $PID1
sleep 3
# Rerun same tests for the prog2 binary
-filename=`testrun ${abs_top_builddir}/debuginfod/debuginfod-find debuginfo $BUILDID2`
+filename=`testrun ${abs_top_builddir}/debuginfod/debuginfod-find -v debuginfo $BUILDID2 2>vlog`
cmp $filename F/prog2
+cat vlog
+grep -q Progress vlog
+tempfiles vlog
filename=`testrun ${abs_top_builddir}/debuginfod/debuginfod-find executable $BUILDID2`
cmp $filename F/prog2
filename=`testrun ${abs_top_builddir}/debuginfod/debuginfod-find source $BUILDID2 ${PWD}/prog2.c`
More information about the Elfutils-devel
mailing list