Memory leak when using render.Download
alexei38 opened this issue · 4 comments
Description
Memory leak when using render.Download
Steps to Reproduce the Problem
- Create simple app
package main
import (
"net/http"
"os"
"github.com/gobuffalo/buffalo"
"github.com/gobuffalo/buffalo/render"
)
const testFilePath = "/tmp/testLargeFile"
func main() {
app := buffalo.New(buffalo.Options{})
app.GET("/", downloadHandler)
app.Serve()
}
func downloadHandler(c buffalo.Context) error {
f, _ := os.Open(testFilePath)
return c.Render(http.StatusOK, render.Download(c, f.Name(), f))
}
- run app
go run main.go
- create large file
dd if=/dev/zero of=/tmp/testLargeFile bs=1M count=20000
- download file
curl http://localhost:3000
Waiting for the OOM killer to come
Expected Behavior
The correct work is in http
func downloadHandler(c buffalo.Context) error {
http.ServeFile(c.Response(), c.Request(), testFilePath)
return nil
}
Actual Behavior
memory leak when working with large files
Info
-> Go: Checking installation
✓ The `go` executable was found on your system at: /home/amargasov/go/go1.17.8/bin/go
-> Go: Checking minimum version requirements
✓ Your version of Go, 1.17.5, meets the minimum requirements.
-> Go: Checking Package Management
✓ You are using Go Modules (`go`) for package management.
-> Go: Checking PATH
✓ Your PATH contains /home/amargasov/go/bin.
-> Node: Checking installation
✘ The `node` executable could not be found on your system.
For help setting up your Node environment please follow the instructions for you platform at:
https://nodejs.org/en/download/
-> NPM: Checking installation
✘ The `npm` executable could not be found on your system.
For help setting up your NPM environment please follow the instructions for you platform at:
https://docs.npmjs.com/getting-started/configuring-your-local-environment
-> Yarn: Checking installation
✘ The `yarnpkg` executable could not be found on your system.
For help setting up your Yarn environment please follow the instructions for you platform at:
https://yarnpkg.com/en/docs/install
-> PostgreSQL: Checking installation
✘ The `postgres` executable could not be found on your system.
For help setting up your Postgres environment please follow the instructions for you platform at:
https://www.postgresql.org/download/
-> MySQL: Checking installation
✘ The `mysql` executable could not be found on your system.
For help setting up your MySQL environment please follow the instructions for you platform at:
https://www.mysql.com/downloads/
-> SQLite3: Checking installation
✓ The `sqlite3` executable was found on your system at: /usr/bin/sqlite3
-> SQLite3: Checking minimum version requirements
✓ Your version of SQLite3, 3.31.1, meets the minimum requirements.
-> Cockroach: Checking installation
✘ The `cockroach` executable could not be found on your system.
For help setting up your Cockroach environment please follow the instructions for you platform at:
https://www.cockroachlabs.com/docs/stable/
-> Buffalo (CLI): Checking installation
✓ The `buffalo` executable was found on your system at: /usr/local/bin/buffalo
-> Buffalo (CLI): Checking minimum version requirements
✓ Your version of Buffalo (CLI), v0.18.2, meets the minimum requirements.
-> Buffalo: Application Details
Warning: It seems like it is not a buffalo app. (.buffalo.dev.yml not found)
Hi @alexei38,
Could you please explain the use case for this example? Buffalo App has dedicated function App.ServeFiles() to serve static files in a given http.FileSystem so you can use that function to serve large files. If the goal is serving large files in a directory, you can modify your app as:
func main() {
app := buffalo.New(buffalo.Options{})
app.ServeFiles("/", http.Dir("/tmp"))
app.Serve()
}Also, as you demonstrated, you can also use the standard method with function http.ServeFile() directly for specific file. (but it is not in a scope of buffalo but your application)
For the render.Download() function, it gets the name of file and io.Reader as arguments without size checking within it, so ensuring the size to prevent OOM (not memory leak) could be a responsibility of users.
Even though this function is not perfect for serving a large static file, it is still useful to serve dynamic content as a file e.g. downloading dynamically created .xls from a database table or similar purpose.
For more discussion, providing a detailed use case could be very helpful.
These are static large files on the disk
Before sending there are permission checks individually for each file and other checks.
Can't use app.ServeFiles (
This issue is stale because it has been open 30 days with no activity. Remove stale label or comment or this will be closed in 5 days.
This issue was closed because it has been stalled for 5 days with no activity.