Code

Opened 20 months ago

Closed 7 months ago

Last modified 7 months ago

#18744 closed Cleanup/optimization (fixed)

NamedTemporaryFile opened in read mode cannot be written to by another process on Windows

Reported by: ty@… Owned by: marfire
Component: File uploads/storage Version: master
Severity: Normal Keywords:
Cc: k@… Triage Stage: Accepted
Has patch: yes Needs documentation: no
Needs tests: no Patch needs improvement: no
Easy pickings: no UI/UX: no

Description

The documentation for NamedTemporaryFile says that it exists so that on Windows so other processes can open the file, which is not possible using Python's implementation of NamedTemporaryFile because of the O_TEMPORARY flag that is passed by default.

However, if the temporary file is opened in read mode (r+), another process cannot write to the file, unless the temp file is closed before the outside process is called. This comes up with django_compressor when trying to use lessc (either https://github.com/duncansmart/less.js-windows or http://www.dotlesscss.org/).

I've attached a test that demonstrates the problem. On *nix, this test should pass.

Attachments (2)

test.py (1011 bytes) - added by ty@… 20 months ago.
test.less (51 bytes) - added by ty@… 20 months ago.

Download all attachments as: .zip

Change History (9)

Changed 20 months ago by ty@…

Changed 20 months ago by ty@…

comment:1 Changed 20 months ago by ramiro

  • Needs documentation unset
  • Needs tests unset
  • Patch needs improvement unset
  • Triage Stage changed from Unreviewed to Design decision needed

A more general question to the reporter of this ticket: What's the use case of having one piece of software (Django in this case) having a file open for reading (and possibly already reading from it) and then another process opening the same file for writing (and possibly writing to it)? This with independence of platform. Is there even an expected deterministic behavior such a scenarios should yield?

comment:2 Changed 20 months ago by Tyler G. Hicks-Wright <ty@…>

I ran into the bug in Jezdez's django_compressor (https://github.com/jezdez/django_compressor), which creates a temporary file for various CSS and Javascript preprocessors (e.g. Less, Sass, CoffeeScript) to write to. See https://github.com/jezdez/django_compressor/blob/develop/compressor/filters/base.py#L113.

It uses NamedTemporaryFile, with the expectation that the Windows version will behave identically to the *nix version, as the documentation states. However, many external processes will fail with some sort of permission error when they try to write to the temporary file that was created via NamedTemporaryFile and not closed.

My guess is that this will take some work to fix for Windows, given how Windows file handling works. If it's more work than it's worth, then at least the documentation should be updated to point out this deficiency.

comment:3 Changed 13 months ago by jacob

  • Triage Stage changed from Design decision needed to Accepted

comment:4 Changed 7 months ago by marfire

  • Cc k@… added
  • Component changed from Core (Other) to File uploads/storage
  • Has patch set
  • Owner changed from nobody to marfire
  • Status changed from new to assigned
  • Type changed from Bug to Cleanup/optimization
  • Version changed from 1.4 to master

As far as I can tell, the issue isn't about Django or NamedTemporaryFile but about how Windows deals with open files in general (or at least with how Python opens files on Windows). Django's NamedTemporaryFile is there to fix what is basically a bug in the Python implementation that prevents you from reopening the file in the same process on Windows; it does not address the more general problem of opening the same file for writing and reading in multiple processes on Windows. Even if we could figure out how to do that, this wouldn't be the right place to put it since the behavior would then be different depending on whether the file was opened here, or by open(), tempfile.mkstemp(), etc.

The documentation could probably stand to be clearer (for one thing, the link is broken), so I've attached a patch with an updated module docstring: https://github.com/django/django/pull/1640. If it's acceptable I will update the existing wiki page correspondingly.

However, we should really be thinking of this as code for internal use rather than as a tool provided and supported by Django. In addition to the confusion here, it's also the case that NamedTemporaryFile doesn't support the full set of keyword arguments on either 2.6+ or 3.0+. This might be fine given the way that Django uses it, but it wouldn't be good enough if trying to provide a proper drop-in replacement for Python's version.

Version 0, edited 7 months ago by marfire (next)

comment:5 Changed 7 months ago by ty@…

I agree, this is a reasonable fix, just to make it clear that this can't be considered the same on Windows as *nix.

Thanks!

comment:6 Changed 7 months ago by Tim Graham <timograham@…>

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

In 59a34c43a8c3d62eaa400d48a9c26ed5400fc647:

Fixed #18744 -- Updated docstring to highlight limitations of NamedTemporaryFile

  • Noted that this does not allow for reading and writing the same open

file in different processes under Windows.

match the Python version.

comment:7 Changed 7 months ago by marfire

Since it exists, I updated the wiki entry. In my opinion it would be better to just delete it, though, as it's not really referenced anywhere; doesn't contain anything beyond what is in the module docstring; and concerns code that is meant for internal use.

Add Comment

Modify Ticket

Change Properties
<Author field>
Action
as closed
as The resolution will be set. Next status will be 'closed'
The resolution will be deleted. Next status will be 'new'
Author


E-mail address and user name can be saved in the Preferences.

 
Note: See TracTickets for help on using tickets.