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 <f...@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 <f...@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 <f...@redhat.com> + + * run-debuginfod-find.sh: Test debuginfod-find -v progress mode. + 2019-10-28 Aaron Merey <ame...@redhat.com> Frank Ch. Eigler <f...@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`