I/O operation on closed file

[shelinanton@…]

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

[Aymeric Augustin]

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.

[shelinanton@…]

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

[shelinanton@…]

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)

[Aymeric Augustin]

This looks like a regression, I'm marking it as a release blocker to make sure we investigate it before the 1.7 release.

[Florian Apolloner]

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.

[Tim Graham]

Original ticket is #21057; commmit: 30fc49a7ca0d030c7855f31ed44395903fa6abdd.

[John Hensley]

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.

​https://github.com/django/django/pull/2712

[Florian Apolloner]

@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

[anonymous]

Converted into a PR: ​https://github.com/django/django/pull/2747/ -- review welcome.

[Andrew Godwin]

Changing version so this is obviously a 1.7 blocker.

[Florian Apolloner <florian@…>]

In e2efc8965edf684aaf48621680ef54b84e116576:

Fixed #22680 -- I/O operation on closed file.

This patch is two-fold; first it ensure that Django does close everything in
request.FILES at the end of the request and secondly the storage system should
no longer close any files during save, it's up to the caller to handle that --
or let Django close the files at the end of the request.

[Florian Apolloner <florian@…>]

In 1ff11304dcebbabdede8ef3d659ca0e54055e2fd:

[1.7.x] Fixed #22680 -- I/O operation on closed file.

This patch is two-fold; first it ensure that Django does close everything in
request.FILES at the end of the request and secondly the storage system should
no longer close any files during save, it's up to the caller to handle that --
or let Django close the files at the end of the request.

Backport of e2efc8965edf684aaf48621680ef54b84e116576 from master.

[Loic Bistuer <loic.bistuer@…>]

In 6e8d614acd1b65a1ae472da7db88a7b2751dc388:

Made the vendored NamedTemporaryFile work as a context manager. Refs #22680.

This fixes a regression on Windows introduced by b7de5f5.

Thanks Tim Graham for the report and review.

[Tim Graham <timograham@…>]

In d9eef1f4f7b4401a0ff6e3feb3352b6e661f705a:

[1.7.x] Made the vendored NamedTemporaryFile work as a context manager. Refs #22680.

This fixes a regression on Windows introduced by b7de5f5.

Thanks Tim Graham for the report and review.

Backport of 6e8d614acd from master