remote: limit concurrent layer pulls by gnix0 · Pull Request #2271 · google/go-containerregistry

gnix0 · 2026-04-21T19:53:23Z

remote.WithJobs controls remote operation parallelism, but lazy images returned by remote.Get(...).Image() did not carry that limit into later layer reads. Consumers such as layout.ReplaceImage could start one remote blob GET per layer at once.

Carry the configured job limit into remote-backed blob reads and hold a limiter token until each response body is closed.

Fixes #2140

Reproduced:

Added a local repro that pulled an image with 6 layers using remote.WithJobs(2), then concurrently opened every layer's Compressed() stream. Before the fix, the test server observed 6 concurrent layer GETs. After the fix, the maximum observed concurrency was 2.

Tested:

go test ./pkg/v1/remote

go test ./pkg/v1/layout ./pkg/v1/remote

codecov-commenter · 2026-04-21T22:09:36Z

Codecov Report

❌ Patch coverage is 69.76744% with 13 lines in your changes missing coverage. Please review.
✅ Project coverage is 52.07%. Comparing base (85c9514) to head (2f9a9ca).

Files with missing lines	Patch %	Lines
pkg/v1/remote/fetcher.go	54.16%	9 Missing and 2 partials ⚠️
pkg/v1/remote/limiter.go	87.50%	2 Missing ⚠️

Additional details and impacted files

@@            Coverage Diff             @@
##             main    #2271      +/-   ##
==========================================
+ Coverage   52.03%   52.07%   +0.04%     
==========================================
  Files         165      166       +1     
  Lines       14203    14234      +31     
==========================================
+ Hits         7390     7412      +22     
- Misses       6092     6101       +9     
  Partials      721      721

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:

❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

remote.WithJobs controls remote operation parallelism, but lazy images returned by remote.Get(...).Image() did not carry that limit into later layer reads. Consumers such as layout.ReplaceImage could start one remote blob GET per layer at once. Carry the configured job limit into remote-backed blob reads and hold a limiter token until each response body is closed. Fixes google#2140 Reproduced: Added a local repro that pulled an image with 6 layers using remote.WithJobs(2), then concurrently opened every layer's Compressed() stream. Before the fix, the test server observed 6 concurrent layer GETs. After the fix, the maximum observed concurrency was 2. Tested: go test ./pkg/v1/remote go test ./pkg/v1/layout ./pkg/v1/remote

crane export uses mutate.Extract for normal layered images. mutate.Extract opened each layer reader in sequence, but deferred each Close until the whole extraction returned. After #2271, remote layer readers hold a pull job slot until their response body is closed, so extraction could fill all available slots and then block opening the next layer. The single-layer crane export path also copied from Uncompressed without closing the reader. Close each extraction layer reader as soon as that layer is processed, and close the single-layer export reader after copying. Fixes #2276 Reproduced: Added a test image whose layer readers model the remote pull limiter: opening a reader consumes one of four slots, and Close releases it. Before the fix, extracting five layers failed when the fifth reader opened before the previous readers were closed. After the fix, extraction completes and all slots are released. Tested: go test ./pkg/v1/mutate go test ./pkg/crane go test ./pkg/v1/remote ./pkg/v1/mutate ./pkg/crane ./cmd/crane/rebase_test.sh

remote.WithJobs controls remote operation parallelism, but lazy images returned by remote.Get(...).Image() did not carry that limit into later layer reads. Consumers such as layout.ReplaceImage could start one remote blob GET per layer at once. Carry the configured job limit into remote-backed blob reads and hold a limiter token until each response body is closed. Fixes google#2140 Reproduced: Added a local repro that pulled an image with 6 layers using remote.WithJobs(2), then concurrently opened every layer's Compressed() stream. Before the fix, the test server observed 6 concurrent layer GETs. After the fix, the maximum observed concurrency was 2. Tested: go test ./pkg/v1/remote go test ./pkg/v1/layout ./pkg/v1/remote

crane export uses mutate.Extract for normal layered images. mutate.Extract opened each layer reader in sequence, but deferred each Close until the whole extraction returned. After google#2271, remote layer readers hold a pull job slot until their response body is closed, so extraction could fill all available slots and then block opening the next layer. The single-layer crane export path also copied from Uncompressed without closing the reader. Close each extraction layer reader as soon as that layer is processed, and close the single-layer export reader after copying. Fixes google#2276 Reproduced: Added a test image whose layer readers model the remote pull limiter: opening a reader consumes one of four slots, and Close releases it. Before the fix, extracting five layers failed when the fifth reader opened before the previous readers were closed. After the fix, extraction completes and all slots are released. Tested: go test ./pkg/v1/mutate go test ./pkg/crane go test ./pkg/v1/remote ./pkg/v1/mutate ./pkg/crane ./cmd/crane/rebase_test.sh

tarball.MultiRefWrite (and crane pull, which uses it) opens each layer's Compressed() reader and copies it into the tar without closing it. After google#2271, remote layer readers hold a pull-job slot until their response body is closed, so writing more than `defaultJobs` layers fills all available slots and deadlocks the next layer open. This is the symmetric leak to the one google#2277 fixed in mutate.Extract / crane export. Close each layer reader as soon as that layer is written. Reproduced: crane pull postgres:16 /tmp/pg.tar # v0.21.6 hangs; v0.21.5 works Added TestWriteClosesLayerBeforeOpeningNext, which wraps a random.Image with a token-limited Compressed() that fails if a reader is opened before previous readers were closed. Before this change the test fails with "layer reader opened before previous readers were closed". After this change the test passes and the limiter is fully drained. Tested: go test ./pkg/v1/tarball go test ./pkg/v1/tarball ./pkg/v1/mutate ./pkg/crane Signed-off-by: Nandan Bhat <nandanbhatd@gmail.com>

* tarball: close layer readers during Write tarball.MultiRefWrite (and crane pull, which uses it) opens each layer's Compressed() reader and copies it into the tar without closing it. After #2271, remote layer readers hold a pull-job slot until their response body is closed, so writing more than `defaultJobs` layers fills all available slots and deadlocks the next layer open. This is the symmetric leak to the one #2277 fixed in mutate.Extract / crane export. Close each layer reader as soon as that layer is written. Reproduced: crane pull postgres:16 /tmp/pg.tar # v0.21.6 hangs; v0.21.5 works Added TestWriteClosesLayerBeforeOpeningNext, which wraps a random.Image with a token-limited Compressed() that fails if a reader is opened before previous readers were closed. Before this change the test fails with "layer reader opened before previous readers were closed". After this change the test passes and the limiter is fully drained. Tested: go test ./pkg/v1/tarball go test ./pkg/v1/tarball ./pkg/v1/mutate ./pkg/crane Signed-off-by: Nandan Bhat <nandanbhatd@gmail.com> * tarball: place writeLayer next to writeTarEntry Address review feedback on #2308: the previous patch inserted writeLayer between writeTarEntry's leading doc-comment and its declaration, orphaning the comment. Move writeLayer immediately after writeTarEntry so the existing function block is untouched, and tighten writeLayer's own doc-comment to the non-obvious WHY (pull-limiter slot release). No behavioral change. Signed-off-by: Nandan Bhat <nandanbhatd@gmail.com>

Move defer r.Close() and defer pw.Close() to the top of the goroutine so they execute regardless of which error path exits the function. Previously, r.Close() was only called inside the io.Copy error path and the happy-path end, leaving it unclosed when gw.Close() or bw.Flush() failed. When r wraps a limitedReadCloser (used by the pullLimiter introduced in google#2271), failing to close r permanently leaks the limiter token slot. Under layout.WriteImage (used by kaniko for build-stage caching), this eventually deadlocks because all slots are occupied. Also add defer gw.Close() after gw creation so the gzip writer is always properly finalized. Fixes google#2342 Signed-off-by: Aaron <aaroniofjm@gmail.com>

Move defer r.Close() and defer pw.Close() to the top of the goroutine so they execute regardless of which error path exits the function. Previously, r.Close() was only called inside the io.Copy error path and the happy-path end, leaving it unclosed when gw.Close() or bw.Flush() failed. When r wraps a limitedReadCloser (used by the pullLimiter introduced in #2271), failing to close r permanently leaks the limiter token slot. Under layout.WriteImage (used by kaniko for build-stage caching), this eventually deadlocks because all slots are occupied. Also add defer gw.Close() after gw creation so the gzip writer is always properly finalized. Fixes #2342 Signed-off-by: Aaron <aaroniofjm@gmail.com>

Subserial force-pushed the fix-remote-layer-pull-jobs branch from ae4f5de to 2f9a9ca Compare April 21, 2026 22:25

Subserial approved these changes Apr 21, 2026

View reviewed changes

Subserial merged commit 3502eac into google:main Apr 21, 2026
17 checks passed

adrian-thurston mentioned this pull request Apr 22, 2026

crane: hang on crane export #2276

Closed

gnix0 mentioned this pull request Apr 23, 2026

mutate: close layer readers during export #2277

Merged

BrewTestBot mentioned this pull request May 18, 2026

crane 0.21.6 Homebrew/homebrew-core#283464

Merged

This was referenced May 19, 2026

tarball: close layer readers during Write #2308

Merged

crane pull hangs on multi-layer images (v0.21.6 regression, symmetric to #2276 — pkg/v1/tarball) #2309

Closed

rdimitrov mentioned this pull request Jun 3, 2026

Update module github.com/stacklok/toolhive-core to v0.0.23 stacklok/toolhive#5317

Merged

1 task

SebastienSyd mentioned this pull request Jun 5, 2026

Regression in v0.113.0: grype hangs indefinitely (no timeout) on registry image pull — works in v0.112.0 anchore/grype#3496

Closed

FrancisQuaisr mentioned this pull request Jun 9, 2026

internal/gzip: ReadCloserLevel goroutine leaks r on error, causing pullLimiter deadlock via layout.WriteImage #2342

Closed

yanuzai1final mentioned this pull request Jun 11, 2026

fix(deps): downgrade go-containerregistry to v0.21.5 ckotzbauer/sbom-operator#930

Merged

amarkdotdev mentioned this pull request Jun 16, 2026

internal/gzip: fix goroutine leak in ReadCloserLevel #2347

Merged

chappjc mentioned this pull request Jul 10, 2026

remote: pullLimiter deadlocks callers that read Compressed() to EOF without calling Close() #2369

Closed

Navigation Menu

Repository navigation

remote: limit concurrent layer pulls - #2271

remote: limit concurrent layer pulls#2271
Subserial merged 1 commit into
google:mainfrom
gnix0:fix-remote-layer-pull-jobs

gnix0 commented Apr 21, 2026

Uh oh!

codecov-commenter commented Apr 21, 2026 •

edited

Loading

Uh oh!

Uh oh!

Reviewers

Uh oh!

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

Conversation

gnix0 commented Apr 21, 2026

Uh oh!

codecov-commenter commented Apr 21, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Codecov Report

Uh oh!

Uh oh!

Reviewers

Uh oh!

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

codecov-commenter commented Apr 21, 2026 •

edited

Loading