Why doesn't ParseMultipartForm leak file handles?

user_16998 0
Viewed 211

I’ve got an http endpoint which calls net/http.(*Request).FormFile to read a file uploaded. I noticed the returned *multipart.File is never closed with Close(). That is fine for small files as it is a no-op, but It appears that https://golang.org/src/net/http/request.go#L1369 r.ParseMultipartForm will copy the file out of memory and into a temp file if the file is larger than 32MB. You can see the os.Open call here: https://golang.org/src/mime/multipart/formdata.go?s=3614:3656#L146

AFAICT this would leak file handles, but when I examine the process, I do not see leaking file handles. Where is this cleaned up?

UPDATE: Here is a complete program for testing: https://play.golang.org/p/79_kt46t1PQ

go
1 Answers

The multipart.Form declares a RemoveAll() method, which calls os.Remove(fh.tmpfile).

This method is called either on an error during multipart.Reader.ReadForm or after the request has been served:

    if w.req.MultipartForm != nil {
        w.req.MultipartForm.RemoveAll()
    }

The above is located at https://golang.org/src/net/http/server.go#L1957

Edit:

Actually you might be on to something. There is an open issue mentioning this. In the issue it is noted that RemoveAll gets called at the end of the request, but the files are reportedly still there.

As you also commented, this might be related to the os.Remove call being implemented (on Unix) with unlink syscall, which:

If the name was the last link to a file but any processes still have the file open, the file will remain in existence until the last file descriptor referring to it is closed.

So to wrap up, I think you are supposed to call Close() yourself on the multipart.File, as suggested in the same thread:

file, _, _ := r.FormFile("file")
defer file.Close()

It could be a documentation bug, though some could argue that closing files after usage is obvious, though in this particular case, the docs might be more explicit. Anyway, at this point I guess you could ask for further clarification on the linked issue.

user_41088030
Related