debuginfod: Fix concurrent request leading to a same .rpm file [PR29474]upstream/users/marxin/try-PR29474-fix-debuginfod-concurrency

As explained in detail in the PR, the problem happens when multiple
requests lead to a single RPM and one of the threads does prefetching.
In that case, the requested file is added to fdcache and thus skip
as interned.

1 files changed, 43 insertions, 26 deletions

diff --git a/debuginfod/debuginfod.cxx b/debuginfod/debuginfod.cxx
index a089d0bd..e69a4bbc 100644
--- a/debuginfod/debuginfod.cxx
+++ b/debuginfod/debuginfod.cxx
@@ -1610,39 +1610,23 @@ string canonicalized_archive_entry_pathname(struct archive_entry *e)
     return string("/")+fn;
 }
 
-
-
+// Try getting build_r match from fdcache.
 static struct MHD_Response*
-handle_buildid_r_match (bool internal_req_p,
-                        int64_t b_mtime,
-                        const string& b_source0,
-                        const string& b_source1,
-                        int *result_fd)
+try_buildid_r_match_from_cache (const string& b_source0, const string& b_source1, int *result_fd)
 {
-  struct stat fs;
-  int rc = stat (b_source0.c_str(), &fs);
-  if (rc != 0)
-    throw libc_exception (errno, string("stat ") + b_source0);
-
-  if ((int64_t) fs.st_mtime != b_mtime)
-    {
-      if (verbose)
-        obatched(clog) << "mtime mismatch for " << b_source0 << endl;
-      return 0;
-    }
-
   // check for a match in the fdcache first
+  struct stat fs;
   int fd = fdcache.lookup(b_source0, b_source1);
-  while (fd >= 0) // got one!; NB: this is really an if() with a possible branch out to the end
+  if (fd >= 0) // got one!; NB: this is really an if() with a possible branch out to the end
     {
-      rc = fstat(fd, &fs);
+      int rc = fstat(fd, &fs);
       if (rc < 0) // disappeared?
         {
           if (verbose)
             obatched(clog) << "cannot fstat fdcache " << b_source0 << endl;
           close(fd);
           fdcache.clear(b_source0, b_source1);
-          break; // branch out of if "loop", to try new libarchive fetch attempt
+          return 0;
         }
 
       struct MHD_Response* r = MHD_create_response_from_fd (fs.st_size, fd);
@@ -1651,7 +1635,7 @@ handle_buildid_r_match (bool internal_req_p,
           if (verbose)
             obatched(clog) << "cannot create fd-response for " << b_source0 << endl;
           close(fd);
-          break; // branch out of if "loop", to try new libarchive fetch attempt
+          return 0;
         }
 
       inc_metric ("http_responses_total","result","archive fdcache");
@@ -1668,9 +1652,34 @@ handle_buildid_r_match (bool internal_req_p,
       if (result_fd)
         *result_fd = fd;
       return r;
-      // NB: see, we never go around the 'loop' more than once
     }
 
+  return NULL;
+}
+
+static struct MHD_Response*
+handle_buildid_r_match (bool internal_req_p,
+                        int64_t b_mtime,
+                        const string& b_source0,
+                        const string& b_source1,
+                        int *result_fd)
+{
+  struct stat fs;
+  int rc = stat (b_source0.c_str(), &fs);
+  if (rc != 0)
+    throw libc_exception (errno, string("stat ") + b_source0);
+
+  if ((int64_t) fs.st_mtime != b_mtime)
+    {
+      if (verbose)
+        obatched(clog) << "mtime mismatch for " << b_source0 << endl;
+      return 0;
+    }
+
+  struct MHD_Response *r = try_buildid_r_match_from_cache (b_source0, b_source1, result_fd);
+  if (r != 0)
+    return r;
+
   // no match ... grumble, must process the archive
   string archive_decoder = "/dev/null";
   string archive_extension = "";
@@ -1721,7 +1730,7 @@ handle_buildid_r_match (bool internal_req_p,
   // 2) extract the matching entry name (set r = result)
   // 3) extract some number of prefetched entries (just into fdcache)
   // 4) abort any further processing
-  struct MHD_Response* r = 0;                 // will set in stage 2
+  r = 0;                 // will set in stage 2
   unsigned prefetch_count =
     internal_req_p ? 0 : fdcache_prefetch;    // will decrement in stage 3
 
@@ -1743,7 +1752,15 @@ handle_buildid_r_match (bool internal_req_p,
         continue;
 
       if (fdcache.probe (b_source0, fn)) // skip if already interned
+      {
+        if (r == 0)
+        {
+          r = try_buildid_r_match_from_cache (b_source0, b_source1, result_fd);
+	  if (r != NULL)
+	    return r;
+        }
         continue;
+      }
 
       // extract this file to a temporary file
       char* tmppath = NULL;
@@ -1751,7 +1768,7 @@ handle_buildid_r_match (bool internal_req_p,
       if (rc < 0)
         throw libc_exception (ENOMEM, "cannot allocate tmppath");
       defer_dtor<void*,void> tmmpath_freer (tmppath, free);
-      fd = mkstemp (tmppath);
+      int fd = mkstemp (tmppath);
       if (fd < 0)
         throw libc_exception (errno, "cannot create temporary file");
       // NB: don't unlink (tmppath), as fdcache will take charge of it.