Opened 4 years ago

Closed 4 years ago

Last modified 4 years ago

#16108 closed Bug (fixed)

File removal race condition in FileSystemStorage

Reported by: aaugustin Owned by: nobody
Component: File uploads/storage Version: 1.3
Severity: Normal Keywords: FileSystemStorage file deletion race condition
Cc: Triage Stage: Ready for checkin
Has patch: yes Needs documentation: no
Needs tests: no Patch needs improvement: no
Easy pickings: no UI/UX: no

Description

#16082 describes a race condition in directory creation. It was fixed, however, the same race condition still exists in the file removal code, a few lines below.

While this bug is unlikely to show up in production, it's probably worth fixing in the same way, for the sake of consistency and robustness.

Attached patch proposes a fix similar to r16280. Also, it improves slightly the test case added at that revision.

Attachments (1)

django-file-storage-remove-race.patch (3.9 KB) - added by aaugustin 4 years ago.

Download all attachments as: .zip

Change History (9)

Changed 4 years ago by aaugustin

comment:1 Changed 4 years ago by jezdez

  • Needs documentation unset
  • Needs tests unset
  • Patch needs improvement unset
  • Triage Stage changed from Unreviewed to Ready for checkin

comment:2 Changed 4 years ago by pjdelport

  • Keywords FileSystemStorage file deletion race condition added

Looks good! Thank you. :)

comment:3 Changed 4 years ago by jezdez

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

In [16287]:

Fixed #16108 -- Fixed another race condition in the FileSystemStorage backend with regard to deleting a file. Refs #16082, too. Thanks, Aymeric Augustin.

comment:4 Changed 4 years ago by mlavin

  • UI/UX unset

I know this is fixed in trunk but for what it's worth I have seen this multiple times in production this week. We are using django-compressor for our CSS and JS which delete files in get_available_name https://github.com/jezdez/django_compressor/blob/develop/compressor/storage.py#L36. When new CSS/JS is deployed there is a race to create the new cached files and this error appears.

comment:5 Changed 4 years ago by jezdez

Thanks for reporting that, would you mind to opening a ticket in the django_compressor issue tracker, too?

comment:6 Changed 4 years ago by pjdelport

Can this (and #16082) be included for 1.3.2?

(Is there process to follow to request this, for release management?)

comment:7 Changed 4 years ago by mlavin

comment:8 Changed 4 years ago by daniel@…

+1 for getting this into 1.3.2. I've also seen this in production with django_compressor.

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