Articles / Go

653 words

  • go
  • net-http
  • uploads
  • multipart
  • temp-files
  • debugging

Why Go Upload Handlers Leave Temp Files After a Disconnect

You look in your upload directory after a week and find thousands of half-written files. Every one is from a client that dropped mid-upload. The handler returned an error, the request finished, and the file stayed anyway.

The standard library is not the culprit in most of these cases. The leak is almost always in the bit of code you wrote around it.

What net/http cleans up for you

If you call r.ParseMultipartForm or r.FormFile, parts larger than the memory limit are spilled to os.TempDir() as multipart-* files. Go tracks those and removes them for you in two places, as far as I can tell from reading the source:

  • inside mime/multipart, if reading the form fails part way through (a disconnect, for instance);
  • in the server's request-finishing code, after your handler returns normally.

So a plain ParseMultipartForm handler survives a disconnect fine. Worth checking net/http/server.go for the exact behaviour in your Go version rather than taking my word for it.

Where the leaks actually come from

There are three common ways files get left behind:

  1. Your own temp file with no cleanup on the error path. You stream with MultipartReader, create a file, io.Copy fails with unexpected EOF, and you return before the os.Remove you only wrote for the success path.
  2. A panic in the handler. The server recovers and closes the connection, but the normal request-finishing path does not run, so Go's own temp files for that request stay behind.
  3. The process dies. A SIGKILL, an OOM kill or a deploy mid-upload skips every defer you have.

The first is by far the most common, and it is the one that looks like "disconnect leaves files", because disconnects are what trigger the error path.

The buggy shape

tmp, err := os.CreateTemp(uploadDir, "upload-*")
if err != nil {
	http.Error(w, "server error", 500)
	return
}
if _, err := io.Copy(tmp, part); err != nil {
	http.Error(w, "upload failed", 400)
	return // tmp is still on disk, and still open
}
tmp.Close()
os.Rename(tmp.Name(), finalPath)

Two bugs for the price of one: the file is not removed, and the descriptor is not closed either. Under a steady trickle of flaky mobile clients you leak both disk and file descriptors.

Quick detour: the context does not help here

You might expect r.Context() cancellation to trigger some cleanup. It does not delete anything. The context is cancelled when the client goes away, and your io.Copy returns an error because the body read fails; that error return is the only signal you get. Cleanup is entirely your job on that path.

A shape that cannot leak

Put the temp file's whole life in one function with a named error return, and let a single defer decide whether it survives.

func save(src io.Reader, dst string) (err error) {
	tmp, err := os.CreateTemp(filepath.Dir(dst), "upload-*.part")
	if err != nil {
		return err
	}
	defer func() {
		if err != nil {
			tmp.Close()
			os.Remove(tmp.Name())
		}
	}()

	if _, err = io.Copy(tmp, src); err != nil {
		return err
	}
	if err = tmp.Sync(); err != nil {
		return err
	}
	if err = tmp.Close(); err != nil {
		return err
	}
	return os.Rename(tmp.Name(), dst)
}

Any failure, including the rename, removes the file. The temp file lives in the same directory as the destination so Rename stays on one filesystem and is atomic. The .part suffix means a half-written file can never be mistaken for a finished one.

The handler around it

func upload(w http.ResponseWriter, r *http.Request) {
	r.Body = http.MaxBytesReader(w, r.Body, 100<<20)
	mr, err := r.MultipartReader()
	if err != nil {
		http.Error(w, "bad request", http.StatusBadRequest)
		return
	}
	for {
		part, err := mr.NextPart()
		if err == io.EOF {
			break
		}
		if err != nil {
			http.Error(w, "upload failed", http.StatusBadRequest)
			return
		}
		if part.FormName() != "file" {
			continue
		}
		if err := save(part, newFinalPath()); err != nil {
			http.Error(w, "upload failed", http.StatusBadRequest)
			return
		}
	}
	w.WriteHeader(http.StatusCreated)
}

newFinalPath is whatever generates a server-chosen name, in your upload directory. Never build it from part.FileName(); that is client input.

Handling the cases defer cannot reach

A panic is covered by the same defer, but only if err is set. On a panic the named return is still nil, so the file would survive. If you care, set a done boolean after the rename and have the defer test !done instead of err != nil. That covers panics too.

For a killed process, nothing in-process helps. Sweep on startup, and ideally on a timer:

func sweep(dir string, maxAge time.Duration) error {
	entries, err := os.ReadDir(dir)
	if err != nil {
		return err
	}
	for _, e := range entries {
		if !strings.HasSuffix(e.Name(), ".part") {
			continue
		}
		info, err := e.Info()
		if err != nil {
			continue
		}
		if time.Since(info.ModTime()) > maxAge {
			os.Remove(filepath.Join(dir, e.Name()))
		}
	}
	return nil
}

Choose maxAge longer than your slowest legitimate upload, or you will delete files that are still being written. An hour is a reasonable start for small files, far too short for multi-gigabyte ones on slow links.

Check it actually works

Start an upload with a large file and kill the client partway:

head -c 50M /dev/urandom | curl -F file=@- http://localhost:8080/upload &
sleep 1; kill %1
ls -la uploads/

Before the fix you will see a stray upload-* file. After it, the directory should be empty. Run it a few times; leaks that depend on timing like to hide on the first attempt.

← All articles