Opened 15 years ago

Closed 12 years ago

Last modified 12 years ago

#6456 closed (fixed)

FileField deletes files even when model instance is deleted in a transaction that is rolled back

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


I've got a model with a few FileFields. I've got code resembling this:

class SomeObject(models.Model):

class RelatedFile(models.Model):
    filename = models.FileField(...)
    owner = models.ForeignKey(SomeObject, related_name='related_files')

def update_files(request, id):
    obj = get_object_or_404(SomeObject, pk=id)
    ... more code - which depending on external conditions can raise an exception ...

Since the FileField hooks the RelatedFile's post_delete signal, it deletes the original files when the delete() method is called. Then if the transaction is rolled back, the rows in the database, not being deleted after all, point to nonexistent files.

Change History (9)

comment:1 Changed 15 years ago by Philippe Raoult

Triage Stage: UnreviewedAccepted

right now there's no signal for commit/rollback, and I guess we'd need that to make sure the files aren't deleted yet. See #5415 for the implementation of something similar.

comment:2 Changed 14 years ago by Marty Alchin

Component: Core frameworkFile uploads/storage

comment:3 Changed 12 years ago by Flo Ledermann

Needs documentation: set

This is a severe dataloss bug that has bitten me badly recently. Suggesting raising the priority of this, although it's too late for 1.3 now. I'll try to at least provide a test case and a doc patch as soon as i find some time.

comment:4 Changed 12 years ago by Russell Keith-Magee

milestone: 1.3

It's not too late at all. 1.3 final won't be released until mid January at the earliest.

I think this adds weight to Malcolm's argument at DjangoCon that FileField shouldn't *ever* delete a file, and the cleanup task should be left to an external process.

comment:5 Changed 12 years ago by Jannis Leidel

Couldn't agree more with Malcolm's argument.

comment:6 Changed 12 years ago by Flo Ledermann

For what it's worth, as someone who has been bitten by this bug, I absolutely agree that FileField should never delete files in the filesystem, especially if we cannot guarantee transactional integrity (which we probably cannot given the mechanisms we currently have).

comment:7 Changed 12 years ago by Carl Meyer

Resolution: fixed
Status: newclosed

(In [15321]) Fixed #6456 - Excised FileField file deletion to avoid data loss. Thanks to durdinator for the report.

comment:8 Changed 12 years ago by Carl Meyer

(In [15322]) [1.2.X] Fixed #6456 - Excised FileField file deletion to avoid data loss. Thanks to durdinator for the report.

Backport of r15321 from trunk. Backported despite slight backwards-incompatibility due to potential for data loss.

comment:9 Changed 12 years ago by Jacob

milestone: 1.3

Milestone 1.3 deleted

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