fix: close cached content to prevent memory leak in Goproxy.serveFetchDownload (#152)

Copilot · aofei · web-flow · commit 73b872489536 · 2025-10-10T09:26:06.000+08:00
* Initial plan

* Fix memory leak by adding defer content.Close() in serveFetchDownload

Co-authored-by: aofei &lt;5037285+aofei@users.noreply.github.com&gt;

* Simplify test code based on code review feedback

Co-authored-by: aofei &lt;5037285+aofei@users.noreply.github.com&gt;

* Add .gitignore to exclude build artifacts

Co-authored-by: aofei &lt;5037285+aofei@users.noreply.github.com&gt;

* Move test to subtest and remove .gitignore per code review

Co-authored-by: aofei &lt;5037285+aofei@users.noreply.github.com&gt;

* Use existing closerFunc instead of testReadCloser

Co-authored-by: aofei &lt;5037285+aofei@users.noreply.github.com&gt;

---------

Co-authored-by: copilot-swe-agent[bot] &lt;198982749+Copilot@users.noreply.github.com&gt;
Co-authored-by: aofei &lt;5037285+aofei@users.noreply.github.com&gt;
diff --git a/goproxy.go b/goproxy.go
@@ -260,6 +260,7 @@ func (g *Goproxy) serveFetchDownload(rw http.ResponseWriter, req *http.Request,
 	}
 
 	if content, err := g.cache(req.Context(), target); err == nil {
+		defer content.Close()
 		responseSuccess(rw, req, content, contentType, cacheControlMaxAge)
 		return
 	} else if !errors.Is(err, fs.ErrNotExist) {
diff --git a/goproxy_test.go b/goproxy_test.go
@@ -872,6 +872,52 @@ func TestGoproxyServeFetchDownload(t *testing.T) {
 			}
 		})
 	}
+
+	t.Run("CachedContentClosed", func(t *testing.T) {
+		// Track whether Close was called on the cached content
+		var closeCalled bool
+
+		// Create a custom cacher that returns closable content
+		cacher := &testCacher{
+			Cacher: DirCacher(t.TempDir()),
+			get: func(ctx context.Context, c Cacher, name string) (io.ReadCloser, error) {
+				return struct {
+					io.Reader
+					io.Closer
+				}{
+					Reader: strings.NewReader(info),
+					Closer: closerFunc(func() error {
+						closeCalled = true
+						return nil
+					}),
+				}, nil
+			},
+		}
+
+		g := &Goproxy{
+			Fetcher: &GoFetcher{
+				Env:     []string{"GOPROXY=off", "GOSUMDB=off"},
+				TempDir: t.TempDir(),
+			},
+			Cacher:  cacher,
+			TempDir: t.TempDir(),
+			Logger:  slog.New(slog.DiscardHandler),
+		}
+		g.initOnce.Do(g.init)
+
+		rec := httptest.NewRecorder()
+		target := "example.com/@v/v1.0.0.info"
+		g.serveFetchDownload(rec, httptest.NewRequest("", "/", nil), target, "example.com", "v1.0.0", false)
+
+		if !closeCalled {
+			t.Error("cached content was not closed, potential memory leak")
+		}
+
+		recr := rec.Result()
+		if got, want := recr.StatusCode, http.StatusOK; got != want {
+			t.Errorf("got status %d, want %d", got, want)
+		}
+	})
 }
 
 func TestGoproxyServeSumDB(t *testing.T) {

Original file line number	Diff line number	Diff line change
`@@ -260,6 +260,7 @@ func (g Goproxy) serveFetchDownload(rw http.ResponseWriter, req http.Request,`
`260`	`260`	`}`
`261`	`261`
`262`	`262`	`if content, err := g.cache(req.Context(), target); err == nil {`
	`263`	`+ defer content.Close()`
`263`	`264`	`responseSuccess(rw, req, content, contentType, cacheControlMaxAge)`
`264`	`265`	`return`
`265`	`266`	`} else if !errors.Is(err, fs.ErrNotExist) {`