#22680 closed Bug (fixed)
I/O operation on closed file
Reported by: | Owned by: | nobody | |
---|---|---|---|
Component: | File uploads/storage | Version: | 1.7-beta-2 |
Severity: | Release blocker | Keywords: | |
Cc: | Florian Apolloner, Tim Graham, mortas.11@… | 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
Save method of file field checks contents size on closed file.
class Author(models.Model): photo_url = models.URLField(null=True, blank=True, max_length=1024,verbose_name = _('Original photo url')) photo_copy = models.ImageField(upload_to='photos/%Y/%m/%d/authors',null=True, blank=True, verbose_name = _('Thumbnail')) def save(self, *args, **kwargs): if not self.photo_copy and self.photo_url: img_temp = NamedTemporaryFile(delete=True) img_temp.write(urllib2.urlopen(self.photo_url).read()) img_temp.flush() self.photo_copy.save('filename.jpg', File(img_temp),save=False) super(Author, self).save(*args, **kwargs)
Results in:
ValueError: I/O operation on closed file
django.db.models.fileds.files.py line 93
self._size = content.size
Change History (18)
comment:1 by , 11 years ago
Severity: | Release blocker → Normal |
---|
comment:2 by , 11 years ago
More info could be find here https://groups.google.com/forum/#!topic/django-users/SXMDKFFnoSc
This bug occured just after migration from dajngo 1.6 to django 1.7.
My traceback:
Traceback (most recent call last): File "/home/hodza/projects/pinstream/pins/management/commands/import.py", line 85, in handle author.save() File "/home/hodza/projects/pinstream/pins/models.py", line 199, in save self.photo_copy.save(img_filename, File(img_temp),save=False) File "/home/hodza/projects/pinstream/venv/src/django/django/db/models/fields/files.py", line 93, in save self._size = content.size File "/home/hodza/projects/pinstream/venv/src/django/django/core/files/base.py", line 46, in _get_size pos = self.file.tell() ValueError: I/O operation on closed file
comment:3 by , 11 years ago
Created simple script to reproduce error:
from django.db import models from django.db.models.fields.files import FieldFile from django.core.files.temp import NamedTemporaryFile from django.core.files import File from django.core.files.storage import FileSystemStorage fs = FileSystemStorage(location='.') field = models.FileField(storage=fs, name="field") class Model(object): field = "" instance = Model() temp = NamedTemporaryFile(delete=True) field_file = FieldFile(instance,field,"field") field_file.save('file', File(temp),save=False)
comment:4 by , 11 years ago
Severity: | Normal → Release blocker |
---|
This looks like a regression, I'm marking it as a release blocker to make sure we investigate it before the 1.7 release.
comment:5 by , 11 years ago
I am not sure that closing files as part of _save is a good choice at all, imo it should be up to the caller to close them (which we currently don't do either). I think we need a mechanism to close stuff from request.FILES at the end of the request.
comment:6 by , 11 years ago
Cc: | added |
---|
comment:7 by , 11 years ago
Cc: | added; removed |
---|
comment:8 by , 11 years ago
Triage Stage: | Unreviewed → Accepted |
---|
Original ticket is #21057; commmit: 30fc49a7ca0d030c7855f31ed44395903fa6abdd.
comment:9 by , 11 years ago
How about instead of setting the cached file size from the intermediate content, we just ask the newly-saved file in the underlying storage? I think that's a simple fix that doesn't require callers to know about and clean up framework debris. The test script above, and of course the test suite, look OK with it.
comment:10 by , 11 years ago
@john: That just masks the original issue; I've got a more extensive patch here [As a sideeffect it fixes the ResourceWarnings in file_upload tests since the files are actually closed now :)]: https://github.com/apollo13/django/compare/fdleakage
comment:11 by , 10 years ago
Converted into a PR: https://github.com/django/django/pull/2747/ -- review welcome.
comment:12 by , 10 years ago
Cc: | added |
---|
comment:13 by , 10 years ago
Version: | master → 1.7-beta-2 |
---|
Changing version so this is obviously a 1.7 blocker.
comment:14 by , 10 years ago
Has patch: | set |
---|---|
Triage Stage: | Accepted → Ready for checkin |
comment:15 by , 10 years ago
Resolution: | → fixed |
---|---|
Status: | new → closed |
I don't see why this would be a release blocker.
Can you provide the full traceback please? I suspect the exception appears inside
File._get_size_from_underlying_file
but I'm not sure where exactly.