Django

Code

Ticket #7658 (closed: fixed)

Opened 5 months ago

Last modified 4 months ago

Streaming file uploads fail on Windows XP

Reported by: justin.driscoll@gmail.com Assigned to: axiak
Milestone: Component: Core framework
Version: SVN Keywords: 2070-fix
Cc: prigun@gmail.com Triage Stage: Ready for checkin
Has patch: 1 Needs documentation: 0
Needs tests: 1 Patch needs improvement: 1

Description

When you attempt to upload a file larger than the memory cutoff it returns a WindowsError? when it attempts to delete the temporary file which still has a handle from the python process because it is not being closed.

Attachments

file_upload_patch.patch (372 bytes) - added by justin.driscoll@gmail.com on 07/07/08 08:34:00.
Adds a "close" method to TemporaryUploadedFile?.
tempfile.diff (2.9 kB) - added by axiak on 07/08/08 15:26:12.
New temp module for Windows support
tempfile.2.diff (1.0 kB) - added by axiak on 07/08/08 18:31:44.
Updated tempfile diff to fix AttributeError? problem.
tempfile.3.diff (3.0 kB) - added by axiak on 07/08/08 18:41:25.
Updated tempfile diff to fix AttributeError? problem.

Change History

07/07/08 08:34:00 changed by justin.driscoll@gmail.com

  • attachment file_upload_patch.patch added.

Adds a "close" method to TemporaryUploadedFile?.

07/07/08 08:38:21 changed by justin.driscoll@gmail.com

  • needs_better_patch changed.
  • summary deleted.
  • needs_tests changed.
  • needs_docs changed.

I would also ask the relevant devs to consider adding some exception handling to the file_move_safe function so that even if os.remove(old_file_name) fails the application can continue.

try:

os.remove(old_file_name)

except:

# log the error and continue execution

07/07/08 08:39:59 changed by justin.driscoll@gmail.com

  • needs_tests set to 1.
  • summary set to Streaming file uploads fail on Windows XP.

Last comment reset the summary, etc. Sorry.

07/08/08 13:32:29 changed by axiak

  • keywords set to 2070-fix.
  • needs_better_patch set to 1.
  • owner changed from nobody to axiak.
  • stage changed from Unreviewed to Accepted.

07/08/08 13:39:35 changed by axiak

This should be fixed as of [7859], can you please confirm this?

07/08/08 14:09:43 changed by justin.driscoll@gmail.com

Just updated to the latest revs for both trunk and newforms-admin (7870). Still get the error above in newforms admin but in trunk I now get a IOError "No such file or directory". Here's a snippet of the traceback where it fails:

File "C:\home\Projects\Django\libs\django-trunk\django\core\files\move.py" in file_move_safe

  1. file_move(old_file_name, new_file_name)

File "C:\Python25\lib\shutil.py" in move

  1. copy2(src,dst)

File "C:\Python25\lib\shutil.py" in copy2

  1. copyfile(src, dst)

File "C:\Python25\lib\shutil.py" in copyfile

  1. fsrc = open(src, 'rb')

07/08/08 14:53:19 changed by axiak

So it appears there's a reason not to use tempfile.NamedTemporaryFile() on windows. I'm going to create my own NamedTemporaryFile() function for windows.

07/08/08 15:26:12 changed by axiak

  • attachment tempfile.diff added.

New temp module for Windows support

07/08/08 15:27:16 changed by axiak

  • stage changed from Accepted to Ready for checkin.

Someone else should probably look at this, but I think this fixes the problems with Windows.

(follow-up: ↓ 9 ) 07/08/08 16:00:58 changed by justin.driscoll@gmail.com

Still getting the IOError on streaming uploads after applying the new patch.

(in reply to: ↑ 8 ) 07/08/08 16:03:42 changed by axiak

Replying to justin.driscoll@gmail.com:

Still getting the IOError on streaming uploads after applying the new patch.

Yeah. That's bug #7675.

07/08/08 18:31:44 changed by axiak

  • attachment tempfile.2.diff added.

Updated tempfile diff to fix AttributeError? problem.

07/08/08 18:41:25 changed by axiak

  • attachment tempfile.3.diff added.

Updated tempfile diff to fix AttributeError? problem.

07/11/08 08:37:26 changed by justin.driscoll@gmail.com

Just updated to newforms-admin rev 7888 which from what I can see is inline with trunk and now there appears to be an issue with ImageField? and streaming uploads as the PIL check for a valid image fails on streamed file. Should I file this as a new issue?

07/11/08 08:40:43 changed by Alex

Justin, there is already a ticket open for that problem, #7673.

07/11/08 08:48:34 changed by anonymous

Thanks, I ran a search but didn't find anything. Wanted to ask before I filed a dupe.

07/15/08 05:45:02 changed by andrewsk

  • cc set to prigun@gmail.com.

It looks like close method for the TemporaryUploadFile? deletes file on Linux and Mac systems
(didn't have a chance to test it on FreeBSD yet), see #7683

07/26/08 17:48:51 changed by mtredinnick

  • status changed from new to closed.
  • resolution set to fixed.

(In [8096]) Fixed #7658 -- Added some Windows-specific tempfile handling. The standard stuff doesn't work with the way Django's file uploading code wants to operate. Patch from Mike Axiak.

08/11/08 09:43:25 changed by justin.driscoll@gmail.com

  • status changed from closed to reopened.
  • resolution deleted.

As of Django r8301 I am still getting a WindowsError? when streaming uploads are used:

[Error 32] The process cannot access the file because it is being used by another process: 'c:\\docume~1\\jdrisc~1\\locals~1\\temp\\eu5ium.upload'

08/14/08 17:48:16 changed by julien

Same issue is reported in #8203.

08/15/08 01:09:28 changed by julien

  • status changed from reopened to closed.
  • resolution set to fixed.

Re-resolving as 'fixed', since the newly reported issue is a *new* one and most likely the same as in #8203. I consider it's a new issue since the problem is now not with the streaming upload, namely, but within the storage phase.

Add/Change #7658 (Streaming file uploads fail on Windows XP)




Change Properties
Action