fix(nest): Resource leak due to lack of closing HTTP response bodies

[cthach]

Problem?

Looks like we're not closing the response bodies of HTTP responses and they must always be closed per docs.

// The http Client and Transport guarantee that Body is always
// non-nil, even on responses without a body or responses with
// a zero-length body. It is the caller's responsibility to
// close Body. The default HTTP client's Transport may not
// reuse HTTP/1.x "keep-alive" TCP connections if the Body is
// not read to completion and closed.

The leak compounds due to the ExtendStream method being called repeatedly to extend the life of the WebRTC stream (I think this happens every 5-10 minutes). Add in multiple concurrent Nest cam streams and you may exhaust system resources (e.g., file descriptors).

I only discovered this issue because my Nest camera streams would go down in Frigate after a few hours. The only way to bring them back up was to restart Frigate and go2rtc.

Solution

Defer the closure of HTTP response bodies after they are read or if a block returns.

How was this tested?

Deployed by building a custom version of Frigate using a Dockerfile and ensuring no regressions were introduced.

# Compile go2rtc
CGO_ENABLED=0 go build -ldflags "-s -w" -trimpath

# Override go2rtc in Frigate image
cat << EOF > Dockerfile
FROM ghcr.io/blakeblackshear/frigate:0.14.0-rc2

COPY go2rtc /usr/local/go2rtc/bin
EOF

# Build Frigate image
docker build -t ghcr.io/blakeblackshear/frigate:0.14.0-rc2-fix-nest .

# Deploy image and ensure Nest streams comes up

[felipecrs]

Btw you can just mount /config/go2rtc binary into frigate container and It Will use It.

[cthach]

Btw you can just mount /config/go2rtc binary into frigate container and It Will use It.

I didn't know that, thanks! It'll make testing easier in the future.

I have some additional changes I'm planning to propose soon in separate PRs.

[AlexxIT]

Thanks!

[AlexxIT]

https://github.com/AlexxIT/go2rtc/releases/tag/v1.9.5