patch 3/3 debuginfod client interruptability

["Frank Ch. Eigler" <fche@redhat.com>]

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`