NamedTemporaryFile is too hard to use portably when you need to open the file by name after writing it.  To do that, you need to close the file first (on Windows), which means you have to pass delete=False, which in turn means that you get no help in cleaning up the actual file resource, which as you can see from the code in tempfile.py is devilishly hard to do correctly.  The fact that it's different on posix (you can open the file for reading by name without closing it first) makes this problem worse. What we really need for this use-case is a way to say, "delete on __del__ but not on close()."

This is quite silly indeed, and is due to the use of O_TEMPORARY in the file creation flags.

What's the proposal here? If delete is True, close() must delete the file. It is not acceptable for close() and __del__() to behave differently.

OTOH, if the proposal is merely to change the way the file is opened on Windows so that it can be opened again without closing it first, that sounds fine.

> OTOH, if the proposal is merely to change the way the file is opened
> on Windows so that it can be opened again without closing it first,
> that sounds fine.

That would be my proposal. It probably needs getting rid of O_TEMPORARY,
exposing CreateFile and _open_osfhandle, and using the FILE_SHARE_DELETE
open mode.

I disagree that it's unacceptable for close() and __del__() to behave differently.  The acceptable difference would be that __del__() closes (if necessary) /and/ deletes the file on disk, while close() merely closes the file.

If you can in fact "change the way the file is opened on Windows so that it can be opened again without closing it first," that would be fine with me.  It isn't clear to me that Windows supports that option, but I'm not an expert.

Another possibility, of course, is something like what's implemented in:
https://github.com/dabrahams/zeroinstall/commit/d76de038ef51bd1dae36280f8743e06c7154b44a#L3R44
(an optional argument to close() that prevents deletion).

The whole point of close() methods is to offer deterministic resource management to applications that need it. Pointing out to applications when they're relying on CPython's refcounting for prompt resource cleanup is why many of the standard types now trigger ResourceWarning for any application that relies on the GC to clean up such external resources in __del__.

So, no, we're not going to back away from the explicit guarantee in the NamedTemporaryFile docs: "If delete is true (the default), the file is deleted as soon as it is closed." (Especially since doing so would also breach backward compatibility guarantees)

However, you're right that the exclusive read lock in the current implementation makes the default behaviour of NamedTemporaryFile significantly less useful on Windows than it is on POSIX systems, so the implementation should be changed to behave more like the POSIX variant.

If file.close() "offers deterministic resource management," then you have to consider the file's open/closed state to be a resource separate from its existence.  A NamedTemporaryFile whose close() deterministically managed the open/closed state but not the existence of the file would be consistent with file.  That said, I understand the move toward deprecating (in the informal sense) cleanups that rely on GC.

I'm not suggesting breaking backward compatibility, either.  I'm suggesting that it might make sense to allow an explicit close-without-delete as an /extension/ of the current interface.  Given the move away from GC-cleanups, you'd probably want an explicit unlink() method as well in that case.

Dave, decoupling the lifecycle of the created file from the object that created it is exactly what delete=False already covers. The complicated dance in NamedTemporaryFile is only to make *__del__* work  a bit more reliably during process shutdown (due to some messy internal problems with what CPython is doing at that point). If you're doing deterministic cleanup (even via atexit), you don't need any of that - you can just use os.unlink().

Nick, not to belabor this, but I guess you don't understand the use-case in question very well, or you'd see that delete=False doesn't cover it.

The use case is this: I have to write a test for a function that takes a filename as a parameter and opens and reads from the file with that name.  The test should conjure up an appropriate file, call the function, check the results, and clean up the file afterwards.  It doesn't matter when the file gets cleaned up, as long as it is cleaned up "eventually."  Having to explicitly delete the file is exactly the kind of boilerplate one wants to avoid in situations like this.

Even if Windows allows a file to be opened for reading (in some circumstances) when it is already open for writing, it isn't hard to imagine that Python might someday have to support an OS that didn't allow it under any circumstances.  It is also a bit perverse to have to keep the file open for writing after you're definitively done writing it, just to prevent it from being deleted prematurely.  I can understand most of the arguments you make against close-without-delete, except those (like the above) that seem to come from a "you shouldn't want that; it's just wrong" stance.

See issue 14514 for an alternate proposal to solve this.  I did search before I opened that issue, but search is currently somewhat broken and I did not find this issue.  I'm not marking it as a dup because my proposal is really a new feature.

I agree we need to add something here to better support the idiom where the "close" and "delete" operations on a NamedTemporaryFile are decoupled without the delete becoming a completely independent call to os.unlink().

I agree with RDM's proposal in issue 14514 that the replacement should be "delete on __exit__ but not on close". As with generator context managers, I'd also add in the "last ditch" cleanup behaviour in __del__.

Converting the issue to a feature request for 3.3 - there's no bug here, just an interaction with Windows that makes the existing behavioural options inconvenient.

After all, you can currently get deterministic cleanup (with a __del__ fallback) via:

  @contextmanager
  def named_temp(name):
    f = NamedTemporaryFile(name, delete=False)
    try:
        yield f
    finally:
        try:
            os.unlink(name)
        except OSError:
            pass

You need to be careful to make sure you keep the CM alive (or it will delete the file behind your back), but the idiom RDM described in the other issues handles that for you:

  with named_temp(fname) as f:
     data = "Data\n"
     f.write(data)
     f.close() # Windows compatibility
     with open(fname) as f:
         self.assertEqual(f.read(), data)

As far as the API goes, I'm inclined to make a CM with the above behavour available as a new class method on NamedTemporaryFile:

  with NamedTemporaryFile.delete_after(fname) as f:
      # As per the workaround

Although, for the stdlib version, I wouldn't suppress the OS Error (I'd follow what we currently do for TemporaryDirectory)

"delete_after" what? I know it is somewhat implicit in the fact that it is a context manager call, but that is not the only context the method name will be seen in. (eg: 'dir' list of methods, doc index, etc).  Even as a context manager my first thought in reading it was "delete after what?", and then I went, "oh, right".

How about "delete_on_exit"?

By the way, I still think it would be nicer just to have the context manager work as expected with delete=True (ie: doesn't delete until the end of the context manager, whether the file is closed or not).  I'm OK with being voted down on that, though.

> By the way, I still think it would be nicer just to have the context
> manager work as expected with delete=True (ie: doesn't delete until
> the end of the context manager, whether the file is closed or not).
> I'm OK with being voted down on that, though.

Indeed, the current behaviour under Windows seems to be kind of a
nuisance, and having to call a separate method doesn't sound very
user-friendly.

I agree. If the primary usage of the class does not work well on Windows, developers will continue to write code using the primary usage because it works on their unix system, and it will continue to cause failures when run on windows. Because Python should run cross-platform, I consider this a bug in the implementation and would prefer it be adapted such that the primary use case works well on all major platforms.

If there is a separate class method for different behavior, it should be for the specialized behavior, not for the preferred, portable behavior.

I recognize there are backward-compatibility issues here, so maybe it's necessary to deprecate NamedTemporaryFile in favor of a replacement.

Well, fixing NamedTemporaryFile in either of the ways we've discussed isn't going to fix people writing non-portable code.  A unix coder isn't necessarily going to close the file before reading it.  However, it would at least significantly increase the odds that the code would be portable, while the current situation *ensures* that the code is not portable.

Daniel. If you have any interest in this issue, would you mind 
summarising the state of affairs, please? I have no direct interest in 
the result but I'm happy to commit a patch or even to work one up if 
somone can come up with a single, concrete suggestion.

Tim Golden,
My preferred solution would be to replace the binary delete argument of the current NamedTemporaryFile implementation with finer-grained options:
   delete=False         # don't delete
   delete=True          # delete after file closed, current behavior
   delete=AFTER_CLOSE   # delete after file closed
   delete=AFTER_CM_EXIT # delete after context manager exits
   delete=AFTER_CM_EXIT_NO_EXCEPTION # delete after CM exit, unless this is due to an exception

I have implemented a Windows-friendly solution to the latter case using Nick Coghlan's code.  My version does not delete the file until the context manager exits, and *if* the context manager exits due to an exception it leaves the file in place and reports its location, to aid me in debugging.

Daniel, Nick, shouldn't the context manager yield f within a with block?

Rather than add a NamedTemporaryFile.delete_after() classmethod, would it not be simpler to just add a close_without_unlink() method to NamedTemporaryFile?

    with NamedTemporaryFile() as f:
        <write to f>
        f.close_without_unlink()
        with open(f.name, 'rb') as f:
            <read from f>

Davide, the @contextlib.contextmanager decorator effectively wraps the
yield statement in the necessary glue so that everything prior to the yield
statement occurs in the __enter__() method of the contextmanager, while
everything subsequent occurs in the __exit__() method.

On Sat, Jun 30, 2012 at 1:46 AM, Davide Rizzo <report@bugs.python.org>wrote:

>
> Davide Rizzo <sorcio@gmail.com> added the comment:
>
> Daniel, Nick, shouldn't the context manager yield f within a with block?
>
> ----------
> nosy: +davide.rizzo
>
> _______________________________________
> Python tracker <report@bugs.python.org>
> <http://bugs.python.org/issue14243>
> _______________________________________
>

Richard, I think the problem with this is that it spreads the non-portable
or OS-dependent parts of the code over several places rather than
concentrating them all in one place.

After close_without_unlink(), what would happen when the context manager
exits or when the object is garbage collected?  Would it then get unlinked?

My preference would be to specify the behavior of close/__exit__/GC
operations at the time of the NamedTemporaryFile creation, so that the rest
of the code can be left unchanged.

The webpage

    http://msdn.microsoft.com/en-us/library/aa273350(v=vs.60).aspx

describes the sopen() function which is like open() but has an extra shflag parameter for specifying the sharing allowed.

If sopen() and the associated constants SH_DENYRD, SH_DENYWR, SH_DENYRW and SH_DENYNO were exposed in the os module, then maybe tempfile could use os.sopen() on Windows instead of os.open() to allow the file to be reopened without closing.

> If sopen() and the associated constants SH_DENYRD, SH_DENYWR, SH_DENYRW 
> and SH_DENYNO were exposed in the os module, then maybe tempfile could 
> use os.sopen() on Windows instead of os.open() to allow the file to be 
> reopened without closing.

Sounds like a good way forward.

On 01/07/2012 21:37, Antoine Pitrou wrote:
>
> Antoine Pitrou <pitrou@free.fr> added the comment:
>
>> If sopen() and the associated constants SH_DENYRD, SH_DENYWR, SH_DENYRW
>> and SH_DENYNO were exposed in the os module, then maybe tempfile could
>> use os.sopen() on Windows instead of os.open() to allow the file to be
>> reopened without closing.
>
> Sounds like a good way forward.

Agreed. Richard: do you have time to put something together?
I'm happy to try if you don't.

> Agreed. Richard: do you have time to put something together?
> I'm happy to try if you don't.

I'm looking into it.

Unfortunately, it seems that you need to use non-default flags when reopening a shared file.  Eg, if the file is currently opened with SH_DENYNO and O_TEMPORARY, then you must reopen it using SH_DENYNO and O_TEMPORARY.

However, I have an initial implementation of os.sopen() which makes the following work:

  import os, tempfile

  FNAME = "foo.txt"
  DATA = "hello bob"

  def opener(name, flag, mode=0o777):
      return os.sopen(name, flag | os.O_TEMPORARY, os.SH_DENYNO, mode)

  with open(FNAME, "w", opener=opener) as f:
      f.write(DATA)
      f.flush()
      with open(FNAME, "r", opener=opener) as f:
          assert f.read() == DATA

  assert not os.path.exists(FNAME)

BTW, Maybe it would be better to add a keyword-only shareflag argument to os.open() rather than add os.sopen().

I checked the source in

   c:/Program Files (x86)/Microsoft Visual Studio 10.0/VC/crt/src/open.c

and it seems that on Windows open() is more or less implemented as a wrapper of sopen(..., ..., SH_DENYNO, ...).

So the only reason that trying to reopen a NamedTemporaryFile fails on Windows is because when we reopen we need to use O_TEMPORARY.

The following works for unmodified python:

  import os, tempfile

  DATA = b"hello bob"

  def temp_opener(name, flag, mode=0o777):
      return os.open(name, flag | os.O_TEMPORARY, mode)

  with tempfile.NamedTemporaryFile() as f:
      f.write(DATA)
      f.flush()
      with open(f.name, "rb", opener=temp_opener) as f:
          assert f.read() == DATA

  assert not os.path.exists(f.name)

So maybe we should just define tempfile.opener().

Alternatively, perhaps it would make sense to have a "reopen()" method on file objects that covers the necessary dance to reopen with the correct flags?

That would solve more problems than just this one (possibly including making it possible to "reopen" StringIO and BytesIO objects).

On 30/06/2012 06:45, Daniel Lenski wrote:
> My preferred solution would be to replace the binary delete argument of the current NamedTemporaryFile implementation with finer-grained options:
>    delete=False         # don't delete
>    delete=True          # delete after file closed, current behavior
>    delete=AFTER_CLOSE   # delete after file closed
>    delete=AFTER_CM_EXIT # delete after context manager exits
>    delete=AFTER_CM_EXIT_NO_EXCEPTION # delete after CM exit, unless this is due to an exception

I'm aware that Richard & others are fleshing out alternatives. But my
having asked you to propose something I wanted to come back on this
particular suggestion. I think it's just too complex an API. Not least
because, on Windows, we're making use of a filesystem feature which will
delete on closure regardless (so the implementation on Windows skips the
context-based delete).

I'm not sure what we'll end up with but I'm more inclined towards the
sort of method-based closer/reopener which is more explicit.

I wrote in an earlier message that a file opened with O_TEMPORARY must be reopened with O_TEMPORARY.  This is not quite accurate.

Using O_TEMPORARY causes the FILE_SHARE_DELETE sharing mode to be used, and a file currently opened with FILE_SHARE_DELETE can only be reopened with FILE_SHARE_DELETE.

Unfortunately using O_TEMPORARY is the only way allowed by msvcrt to get FILE_SHARE_DELETE, even though it also has the orthogonal effect of unlinking the file when all handles are closed.

The nice thing about FILE_SHARE_DELETE is that it gives Unix-like behaviour: the file can be renamed or deleted while you have an open handle, and you can still continue to use the handle.

Attached is a mostly untested attempt at writing replacements for open() and os.open() which use the FILE_SHARE_DELETE sharing mode.  Among other things, these can be used for reopening temporary files.

Even if tempfile does not use make use of this, I think something similar would be useful in the stdlib.

I have opened Issue #15244 with a patch to add a share module to the stdlib.

After monkey patching builtins.open(), io.open() and os.open() to be equivalents using FILE_SHARE_READ | FILE_SHARE_WRITE | FILE_SHARE_DELETE, the regression test suite still runs successfully.

@sbt

> (...) and it seems that on Windows open() is more or less implemented
> as a wrapper of sopen(..., ..., SH_DENYNO, ...).

> So the only reason that trying to reopen a NamedTemporaryFile fails on
> Windows is because when we reopen we need to use O_TEMPORARY.

Could you elaborate on this? What's the relation between SH_DENYNO argument to sopen() and O_TEMPORARY flag?

Sorry, I was not very clear.

If you use the O_TEMPORARY flag with open() to get a file handle, then the share mode used with the underlying CreateFile() function is 

    X = FILE_SHARE_READ | FILE_SHARE_WRITE | FILE_SHARE_DELETE

whereas, if you don't use O_TEMPORARY then the share mode is

    Y = FILE_SHARE_READ | FILE_SHARE_WRITE

While a handle is open with share mode X, you can only reopen the file if you also use share mode X.  Therefore (using the msvcrt) you can only reopen it using O_TEMPORARY.*


* sopen() does give some extra control over the share mode, but you still can't use it to get share mode X without also using O_TEMPORARY.

@sbt

Thanks for info. Also you mentioned looking at c:/Program Files (x86)/Microsoft Visual Studio 10.0/VC/crt/src/open.c What version of Visual Studio/SDK this file is available in?

Also I'd like to point out that this problem came up at Stack Overflow in question "How to create a temporary file that can be read by a subprocess?" (http://stackoverflow.com/q/15169101/95735)