fix: block_until_ready can return early when there is still pending work

rust-lang · Oct 15, 2024 · 8db2192 · 8db2192
1 parent b0b6b16
commit 8db2192
Show file tree

Hide file tree

Showing 2 changed files with 12 additions and 7 deletions.
diff --git a/src/cargo/sources/registry/http_remote.rs b/src/cargo/sources/registry/http_remote.rs
@@ -796,19 +796,22 @@ impl<'gctx> RegistryData for HttpRegistry<'gctx> {
         self.downloads.blocking_calls += 1;
 
         loop {
-            self.handle_completed_downloads()?;
-            self.add_sleepers()?;
-
             let remaining_in_multi = tls::set(&self.downloads, || {
                 self.multi
                     .perform()
                     .context("failed to perform http requests")
             })?;
             trace!(target: "network", "{} transfers remaining", remaining_in_multi);
-
+            // Handles transfers performed by `self.multi` above and adds to
+            // `self.downloads.results`. Failed transfers get added to
+            // `self.downloads.sleeping` for retry.
+            self.handle_completed_downloads()?;
             if remaining_in_multi + self.downloads.sleeping.len() as u32 == 0 {
                 return Ok(());
             }
+            // Handles failed transfers in `self.downloads.sleeping` and
+            // re-adds them to `self.multi`.
+            self.add_sleepers()?;
 
             if self.downloads.pending.is_empty() {
                 let delay = self.downloads.sleeping.time_to_next().unwrap();

diff --git a/tests/testsuite/registry.rs b/tests/testsuite/registry.rs
@@ -3349,19 +3349,21 @@ fn sparse_blocking_count() {
 
     Package::new("bar", "0.0.1").publish();
 
+    // Ensure we have the expected number of `block_until_ready` calls.
+    // The 1st (0 transfers pending), is the deliberate extra call in `ensure_loaded` for a source.
+    // The 2nd (1 transfers pending), is the registry `config.json`.
+    // the 3rd (1 transfers pending), is the package metadata for `bar`.
+
     p.cargo("check")
         .env("CARGO_LOG", "network::HttpRegistry::block_until_ready=trace")
         .with_stderr_data(str![[r#"
    [..] TRACE network::HttpRegistry::block_until_ready: 0 transfers pending
 [UPDATING] `dummy-registry` index
    [..] TRACE network::HttpRegistry::block_until_ready: 1 transfers pending
    [..] TRACE network::HttpRegistry::block_until_ready: 1 transfers pending
-   [..] TRACE network::HttpRegistry::block_until_ready: 1 transfers pending
-   [..] TRACE network::HttpRegistry::block_until_ready: 1 transfers pending
 [WARNING] spurious network error (3 tries remaining): failed to get successful HTTP response from `[..]/index/3/b/bar` ([..]), got 500
 body:
 internal server error
-   [..] TRACE network::HttpRegistry::block_until_ready: 1 transfers pending
 [LOCKING] 1 package to latest compatible version
 [DOWNLOADING] crates ...
 [DOWNLOADED] bar v0.0.1 (registry `dummy-registry`)