Memory leak in v2.6.2

[ChrisLahaye]

Hello,

We are observing a memory leak after upgrading from v2.6.1 to v2.6.2.

The moment the memory issue starts occurring coincides with the release of v2.6.2. The drops in memory are caused by the container getting terminated after reaching the maximum allowed memory.

Dockerfile

FROM caddy:builder AS builder

RUN xcaddy build v2.6.2 \
    --with github.com/greenpau/caddy-security@v1.1.15

FROM caddy:latest

COPY --from=builder /usr/bin/caddy /usr/bin/caddy

Patch

@@ -1,6 +1,6 @@
 FROM caddy:builder AS builder

-RUN xcaddy build v2.6.1 \
+RUN xcaddy build v2.6.2 \
     --with github.com/greenpau/caddy-security@v1.1.15

 FROM caddy:latest

Initial config

{
  "admin": {
    "config": {
      "load_delay": "1s",
      "load": {
        "module": "http",
        "url": "http://caddy-config.caddy:4000/config"
      }
    }
  }
}

External config

{
  "admin": {
    "config": {
      "load_delay": "60s",
      "load": {
        "module": "http",
        "url": "http://caddy-config.caddy:4000/config"
      }
    }
  },
  "apps": {
    ...
  }
}

Logs

Logs don't show anything weird. There are only info logs about the configuration not changing (correct behavior).

v2.6.1

v2.6.2

Please let me know if you need any additional information,

Thank you,

Chris

[mholt]

Thanks, that's very interesting.

Can you please provide the full configs (without modifications, except you can redact private keys or passwords only please) as well as a CPU and memory profile:

Go to localhost:2019/debug/pprof (on the server) and start by collecting a goroutine dump. Then collect a CPU profile and memory/heap profile. They can take about 30s to capture. It should download a file after that delay. Then please upload them here along with the goroutine dump. Try to do this near the peak of memory use (or at least not right after a restart).

Could you also enable access/request and debug level logs if you haven't already?

And if you have time you could do a git bisect to determine which commit causes the bug? That would be extremely helpful and certainly speed up a patch.

[francislavoie]

My hunch is that it has to do with #5077 but I didn't dig

[mholt]

@francislavoie Possibly, although now that I look at it, I think it would only leak if the connection failed or the response status was an error status:

https://github.com/caddyserver/caddy/pull/5077/files#diff-30c30eb9f50b002df80f2b88d4ecf25bf2f2b80721f414fc940bea0d644c0216R123-R128

It seems unlikely for that to be occurring all the time. I'd think the logs would mention that. (But in any case, we should probably close the response body in those error returns.)

[ChrisLahaye]

The retry loop did not stop iterating on success, instead, it was creating all requests regardless of whether there is an error or not. I believe the leak is caused by the fact that the body of these requests (potentially excluding the last) was never read/closed.

I have opened a PR which fixes this issue as well as the memory leak.

[mholt]

Thank you for the contribution 😀