Skip to content

Memory Leak with invalid status code #316

@Jorgevillada

Description

@Jorgevillada

Hi, thank you for this project.

I been testing this project in kubernetes with 100/200 Req/sec and with invalid status code (> 299) there are some problems with the memory and a lot of OOMKilled in a day(10-15).

I tested this PR updated with the current version of the branch main(#235) but it has other errors. (Probably channels closed)

error copying response: readfrom tcp 127.0.0.1:8080->127.0.0.1:53150: write tcp 127.0.0.1:8080->127.0.0.1:53150: write: broken pipe

This image is from prometheus with the current branch main.
Memory incrase until OOMKilled happens
image

I been trying some code modifications and this behavior changes when added this If (after this if https://github.com/willnorris/imageproxy/blob/main/imageproxy.go#L484-L487)

if should304(req, resp) {
		// bare 304 response, full response will be used from cache
		return &http.Response{StatusCode: http.StatusNotModified}, nil
}
if resp.StatusCode > 299 {
    return nil, fmt.Errorf("invalid http code %s: %s", req.URL.String(), resp.Status)
}

and this. https://github.com/willnorris/imageproxy/blob/main/imageproxy.go#L207
i moved it before this if.

resp, err := p.Client.Do(actualReq)
// close the original resp.Body, even if we wrap it in a NopCloser below
if resp != nil && resp.Body != nil {
  defer resp.Body.Close()
}

if err != nil {
  msg := fmt.Sprintf("error fetching remote image: %v", err)
  p.log(msg)
  http.Error(w, msg, http.StatusInternalServerError)
  metricRemoteErrors.Inc()
  return
}

I'm no sure if this status code validation works in all escenarios, but for me it works.
This is prometheus with this change(1 hour before vs 9 hour now)
image

The memory problem is still present, but the behavior is different.

I added "net/http/pprof" to try to find aditional strange behaviors.
and I found this.
image

It seems like a problem with cached request too.

I found this open PR https://github.com/gregjones/httpcache/pull/102/files

This is from pprof:
Heap: https://drive.google.com/file/d/1lH9oYfO_88EAAMdxEwZV1FrcNS-3CaN-/view?usp=sharing
goroutine: https://drive.google.com/file/d/1_jinUorbPVNLxEMrUbzXs5Z9ITT5_2MD/view?usp=sharing

Relates to: #97 #196 #92

Metadata

Metadata

Assignees

No one assigned

    Labels

    No labels
    No labels

    Projects

    No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions