Opened 2 years ago

Closed 23 months ago

Last modified 14 months ago

#21057 closed Bug (fixed)

FileSystemStorage can leave temporary files around

Reported by: john Owned by: nobody
Component: File uploads/storage Version: 1.5
Severity: Normal Keywords:
Cc: Triage Stage: Unreviewed
Has patch: yes Needs documentation: no
Needs tests: no Patch needs improvement: no
Easy pickings: no UI/UX: no

Description

FileSystemStorage._save doesn't close the file passed in the content argument, unless it has a 'temporary_file_path' attribute. When you upload a file bigger than FILE_UPLOAD_MAX_MEMORY_SIZE via a FileField on a ModelForm, content is its FieldFile, not the underlying NamedTemporaryFile. It doesn't have a temporary_file_path attribute, so is treated like an upload that has to be copied, not moved, and is never explicitly closed.

This is probably OK most of the time, as the NamedTemporaryFile should eventually be automatically closed and deleted. It certainly seems to be handled properly when running under the dev server. However, under uWSGI the temporary files never seem to be removed. I'm not sure yet why this is, but adding an explicit close solves the problem.

A slight improvement might be also having FieldFile pass its underlying file to the storage save() method, as then not only would the temporary file be cleaned up, but it could just be moved instead of copied, saving some work.

Change History (4)

comment:1 Changed 2 years ago by john

  • Has patch set
  • Needs documentation unset
  • Needs tests unset
  • Patch needs improvement unset

comment:2 Changed 23 months ago by Tim Graham <timograham@…>

  • Resolution set to fixed
  • Status changed from new to closed

In 30fc49a7ca0d030c7855f31ed44395903fa6abdd:

Fixed #21057 -- Prevented FileSystemStorage from leaving temporary files.

comment:3 Changed 23 months ago by john

Thanks for the quick response, Tim.

comment:4 Changed 14 months ago by timo

This has caused a regression, see #22680.

Note: See TracTickets for help on using tickets.
Back to Top