tempfile.NamedTemporaryFile not particularly useful on Windows

[dabrahams]

BPO	14243
Nosy	@pfmoore, @jaraco, @ncoghlan, @pitrou, @ericvsmith, @tjguk, @jwilk, @merwok, @bitdancer, @njsmith, @methane, @dabrahams, @ethanfurman, @sorcio, @vadmium, @zware, @dlenski, @eryksun, @zooba, @ethanhs, @Ev2geny
PRs	bpo-14243: Optionally delete NamedTemporaryFile on content manager exit but not on file close #22431 gh-58451: Add optional delete_on_close parameter to NamedTemporaryFile #97015
Files	ntempfile.py share.py

^{Note: these values reflect the state of the issue at the time it was migrated and might not reflect the current state.}

Show more details

GitHub fields:

assignee = None
closed_at = None
created_at = <Date 2012-03-10.02:14:08.216>
labels = ['type-feature', 'library', '3.10', 'OS-windows']
title = 'tempfile.NamedTemporaryFile not particularly useful on Windows'
updated_at = <Date 2021-05-01.01:32:51.852>
user = 'https://github.com/dabrahams'

bugs.python.org fields:

activity = <Date 2021-05-01.01:32:51.852>
actor = 'methane'
assignee = 'none'
closed = False
closed_date = None
closer = None
components = ['Library (Lib)', 'Windows']
creation = <Date 2012-03-10.02:14:08.216>
creator = 'dabrahams'
dependencies = []
files = ['26215', '26229']
hgrepos = []
issue_num = 14243
keywords = ['patch']
message_count = 83.0
messages = ['155278', '155309', '155316', '155317', '155333', '155365', '155374', '155375', '155457', '157639', '157925', '157927', '157946', '157947', '157948', '157949', '157952', '164358', '164369', '164375', '164392', '164433', '164487', '164495', '164496', '164497', '164503', '164504', '164505', '164509', '164514', '164617', '184053', '184056', '184088', '228371', '244762', '288473', '288504', '288520', '288521', '288538', '288540', '376593', '376645', '376649', '376656', '376659', '376662', '376663', '376664', '376665', '376666', '376684', '376697', '376702', '377580', '390498', '390805', '390806', '390810', '390814', '390888', '390894', '390899', '390903', '390906', '390908', '390911', '392252', '392253', '392257', '392442', '392447', '392470', '392479', '392480', '392483', '392484', '392505', '392511', '392515', '392559']
nosy_count = 26.0
nosy_names = ['paul.moore', 'jaraco', 'ncoghlan', 'pitrou', 'eric.smith', 'tim.golden', 'jwilk', 'eric.araujo', 'r.david.murray', 'njs', 'methane', 'dabrahams', 'ethan.furman', 'davide.rizzo', 'sbt', 'Gabi.Davar', 'martin.panter', 'piotr.dobrogost', 'zach.ware', 'dlenski', 'eryksun', 'steve.dower', 'Carl Osterwisch', 'ethan smith', 'ev2geny', 'chary314']
pr_nums = ['22431']
priority = 'normal'
resolution = None
stage = 'patch review'
status = 'open'
superseder = None
type = 'enhancement'
url = 'https://bugs.python.org/issue14243'
versions = ['Python 3.10']

[dabrahams]

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()."

[pitrou]

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

[ncoghlan]

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.

[pitrou]

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.

[dabrahams]

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:
dabrahams/zeroinstall@d76de03#L3R44
(an optional argument to close() that prevents deletion).

[ncoghlan]

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.

[dabrahams]

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.

[ncoghlan]

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().

[dabrahams]

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.

[bitdancer]

See bpo-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.

[ncoghlan]

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 bpo-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