install: fail instead of hanging when a tarball download task cannot be created

Closed
RO
1mo ago
+130-1

Problem

  • bun install with the isolated linker hangs forever when a tarball download task cannot be created, for example a tarball URL that is not http. Two cases reach it: the second store entry of the package (peer variants), and any retry of the install once the manifest is in the manifest cache, because the resolve phase already failed the same way.
  • The cause is in generate_network_task_for_tarball (src/install/PackageManager/runTasks.rs). It records the task id in network_dedupe_map before for_tarball can fail. A later enqueue for the same id then sees a created task and queues its callback behind it. No task exists, so the callback never runs and the store entry's pending-task slot is never released.
  • The vended NetworkTask slot was also leaked on that path.

Fix

  • On the for_tarball error path, mark the task id failed and return the slot to the pool. Later enqueues get AlreadyFailed, which every caller (isolated, hoisted, runtime auto-install) already handles by releasing its own bookkeeping, and the install exits 1 with the existing error.
  • This is the function that creates the dedupe record, so all six callers are covered, including the resolve-phase prefetch that causes the manifest cache case.
  • Verified: two new tests in test/cli/install/isolated-install.test.ts. Both hang on the released bun (the test times out) and pass in under a second with the fix. Also the install suites listed in Notes.

Background

  • The install phase reserves one pending-task slot per store entry and waits until all slots are released. An entry that needs a download releases its slot when the download's callbacks run.
  • task_queue maps a task id to the callbacks waiting for it. network_dedupe_map records which task ids already have a task, so one tarball is downloaded once. failed on that record (#34103) makes later enqueues fail fast with AlreadyFailed.
  • The hoisted linker does not reserve slots, so it already exited 1 in both cases. Only the isolated linker hung.
Notes

Repro of the manifest cache case (the realistic one: any failed install caches the manifest, and the retry hangs). Registry manifest with dist.tarball: "ftp://...", isolated linker. Run 1: error: Expected tarball URL to start with https:// or http:// and exit 1 (the error propagates out of the resolve phase). Run 2 within the manifest max-age: the dependency resolves synchronously from the cache, the prefetch fails and is only logged, the install phase enqueues the tarball, has_created_network_task says it exists, and the process never exits. With the fix run 2 prints the same error and exits 1. The hoisted linker exits 1 in run 2 before and after.

Repro of the peer variant case. Two workspaces depend on peer-deps@1.0.0 with different no-deps versions. Install, replace the tarball URL of peer-deps in bun.lock with ftp://..., remove node_modules, install with an empty cache. Released bun prints InvalidURL: failed to enqueue package for download: peer-deps@1.0.0 once and never exits.

Why here and not in the callers. An earlier version of this change removed the queued callback in enqueue_package_for_download and enqueue_tarball_for_download. That fixes the peer variant case only. The manifest cache case fails in the resolve-phase prefetch (PackageManagerEnqueue.rs, get_or_put_resolved_package), which never touches task_queue, so the install phase still parked the entry. Marking the record where it is created covers both. The callbacks that the two enqueue functions queued before the failure stay in task_queue. Nothing reads them: both functions check network_task_has_failed before they look at the queue.

Slot return. All error returns in for_tarball happen before it initializes unsafe_http_client, so put (which drops the rest of the NetworkTask) is the whole cleanup.

Suites run with the debug build: isolated-install (84), bun-install-tarball-integrity, bun-install-retry, bun-add, bun-install-patch, bun-install-git-deps, bun-install (the same 14 failures as on main in this sandbox: they need the public internet or an IPv6 localhost). cargo fmt is clean.

Split out of #39640, which keeps the unrelated isolated installer drain.

no test proof · iteration 0 · Platform-specific test(s) that do not run on this machine. Deferring to CI, which covers all platforms: test/cli/install/isolated-install.test.ts