From 2267166a698e5024bc02198d4e8bb9d77c8319e3 Mon Sep 17 00:00:00 2001 From: Sai Asish Y Date: Fri, 17 Apr 2026 14:40:50 -0700 Subject: [PATCH 1/2] fix(backend): close the ReadCloser returned by PullBlob in push pushIfNotExist pulled each blob's content from the source storage and then handed it to Blobs().Push wrapped in io.NopCloser. The NopCloser is there on purpose (Close on the distribution reader returns an error, see #50), but it also means the original ReadCloser from PullBlob was never closed on any path, leaking a file descriptor or HTTP body per blob. Add a `defer content.Close()` immediately after the nil-error check so the original reader is released on both success and error paths. The existing NopCloser wrapper still prevents Push from calling Close itself, so the workaround for #50 is preserved. Fixes #491 Signed-off-by: SAY-5 --- pkg/backend/push.go | 7 +++++++ 1 file changed, 7 insertions(+) diff --git a/pkg/backend/push.go b/pkg/backend/push.go index 858a93ec..dfccb201 100644 --- a/pkg/backend/push.go +++ b/pkg/backend/push.go @@ -186,6 +186,13 @@ func pushIfNotExist(ctx context.Context, pb *internalpb.ProgressBar, prompt stri if err != nil { return err } + // The ReadCloser returned by PullBlob was previously never closed on + // either path, leaking the underlying file descriptor (or HTTP body) + // for every blob we pushed. Close on the distribution implementation + // returns an error (#50), which is why we still wrap the reader with + // io.NopCloser below, but we DO need to close `content` ourselves. + // See #491. + defer content.Close() reader := pb.Add(prompt, desc.Digest.String(), desc.Size, tracker.WrapReader(content)) // resolve issue: https://github.com/modelpack/modctl/issues/50 From d81ffec5c939a8f2c927e912fe0fc9fc125ec91a Mon Sep 17 00:00:00 2001 From: Sai Asish Y Date: Sun, 24 May 2026 22:32:46 -0700 Subject: [PATCH 2/2] fix(backend): simplify close comment per review Signed-off-by: say --- pkg/backend/push.go | 9 +++------ 1 file changed, 3 insertions(+), 6 deletions(-) diff --git a/pkg/backend/push.go b/pkg/backend/push.go index dfccb201..a37a09d5 100644 --- a/pkg/backend/push.go +++ b/pkg/backend/push.go @@ -186,12 +186,9 @@ func pushIfNotExist(ctx context.Context, pb *internalpb.ProgressBar, prompt stri if err != nil { return err } - // The ReadCloser returned by PullBlob was previously never closed on - // either path, leaking the underlying file descriptor (or HTTP body) - // for every blob we pushed. Close on the distribution implementation - // returns an error (#50), which is why we still wrap the reader with - // io.NopCloser below, but we DO need to close `content` ourselves. - // See #491. + // Ensure the blob content is closed to avoid leaking resources (#491). + // We use defer here and io.NopCloser below to work around the distribution + // library's Close() implementation which returns a known error (#50). defer content.Close() reader := pb.Add(prompt, desc.Digest.String(), desc.Size, tracker.WrapReader(content))