Opened 11 years ago

Last modified 9 months ago

#21602 new Cleanup/optimization

FileSystemStorage._save() Should Save to a Temporary Filename and Rename to Attempt to be Atomic

Reported by: Kevin Stone Owned by: nobody
Component: File uploads/storage Version: dev
Severity: Normal Keywords:
Cc: bcail Triage Stage: Accepted
Has patch: no Needs documentation: no
Needs tests: no Patch needs improvement: no
Easy pickings: no UI/UX: no
Pull Requests:How to create a pull request

Description

When using FileSystemStorage based backend with say, staticfiles, creates a race condition where it's writing the intended destination file which can lead to race conditions where your web server can serve incomplete assets while they're being written.

Another case happens with post-processing since CachedStaticFilesStorage actually deletes the hash-named file before saving over it.

These issue should be circumventing by directing the writing to a temporary file name and then atomically renaming it to the intended destination. Even better, FileSystemStorage._save() has to site in a While-loop because of race conditions in get_available_name(). Instead, it could defer calling get_available_name until it's prepared to rename the file to its destination.

I'm working on implementing these changes in my own sub-class of FileSystemStorage (and CachedStaticFilesStorage to avoid the delete() call) and would be apply to submit them as a patch if there's interest.

According to the ticket's flags, the next step(s) to move this issue forward are:

  • To provide a patch by sending a pull request. Claim the ticket when you start working so that someone else doesn't duplicate effort. Before sending a pull request, review your work against the patch review checklist. Check the "Has patch" flag on the ticket after sending a pull request and include a link to the pull request in the ticket comment when making that update. The usual format is: [https://github.com/django/django/pull/#### PR].

Change History (4)

comment:1 by Alex Gaynor, 11 years ago

Triage Stage: UnreviewedAccepted

Agreed that atomic rename should be used.

comment:2 by Tim Graham, 11 years ago

Type: UncategorizedCleanup/optimization

comment:3 by Tim Graham, 8 years ago

This is also suggested in #27334 (closed as duplicate).

comment:4 by bcail, 9 months ago

Cc: bcail added

This should be easier to implement once the OS_OPEN_FLAGS are removed (they're currently deprecated).

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