Close GetObject bodies to avoid leaking memory #70
Conversation
WalkthroughThe recent changes to the Changes
Assessment against linked issues
Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 0
Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Files selected for processing (1)
- s3proxy.go (3 hunks)
Additional comments not posted (3)
s3proxy.go (3)
346-346: Good practice: Close the response body.The
defer obj.Body.Close()statement ensures that the response body is properly closed, preventing memory leaks.
452-452: Good practice: Close the response body.The
defer obj.Body.Close()statement ensures that the response body is properly closed if an index file is found, preventing memory leaks.
510-510: Good practice: Close the response body.The
defer obj.Body.Close()statement ensures that the response body is properly closed, preventing memory leaks.
|
Eh... this change didn't really fix the issue. Although it also didn't make it worse so it might be worth merging. That being said I've migrated to caddy-fs-s3 instead so I'm going to withdraw this PR. |
Calls to the
GetObjectmethod of the S3 client require that theBodybe closed to avoid leaking memory. The returned struct is anhttp.Responsereturned by anhttp.Client(verified by digging into the AWS SDK dependency chain) and is guaranteed to always have a non-nilBody.The only way I've been able to spot this issue is in prod after a few days of load. I'm testing this patch now and it already appears to be holding Caddy memory usage much more steady.
Related to and should fix #64.
Summary by CodeRabbit