This issue tracker has been migrated to GitHub, and is currently read-only.
For more information, see the GitHub FAQs in the Python's Developer Guide.

Author izbyshev
Recipients eryksun, izbyshev, paul.moore, steve.dower, tim.golden, vstinner, zach.ware
Date 2021-01-22.18:51:13
SpamBayes Score -1.0
Marked as misclassified Yes
Message-id <1611341473.71.0.74295883981.issue42606@roundup.psfhosted.org>
In-reply-to
Content
> FYI, here are the access rights applicable to files

Thanks, I checked that mapping in headers when I was writing _Py_wopen_noraise() as well. But I've found a catch via ProcessHacker: CreateFile() with GENERIC_WRITE (or FILE_GENERIC_WRITE) additionally grants FILE_READ_ATTRIBUTES for some reason. This is why I add FILE_READ_ATTRIBUTES to FILE_GENERIC_WRITE for DuplicateHandle() -- otherwise, the duplicated handle didn't have the same rights in my testing. So basically I have to deal with (a) _wopen() not guaranteeing contractually any specific access rights and (b) CreateFile() not respecting the specified access rights exactly. This is where my distrust and my original question about "default" access rights come from.

> I overlooked a case that's a complication. For O_TEMPORARY, the open uses FILE_FLAG_DELETE_ON_CLOSE; adds DELETE to the requested access; and adds FILE_SHARE_DELETE to the share mode. 
> _Py_wopen_noraise() can easily keep the required DELETE access. The complication is that you have to be careful not to close the original file descriptor until after you've successfully created the duplicate file descriptor. If it fails, you have to return the original file descriptor from _wopen(). If you close all handles for the kernel File and fail the call, the side effect of deleting the file is unacceptable. 

Good catch! But now I realize that the problem with undoing the side effect applies to O_CREAT and O_TRUNC too: we can create and/or truncate the file, but then fail. Even if we could assume that DuplicateHandle() can't fail, _open_osfhandle() can still fail with EMFILE. And since there is no public function in MSVCRT to replace an underlying handle of an existing fd, we can't preallocate an fd to avoid this. There would be no problem if we could just reduce access rights of an existing handle, but it seems there is no such API.

I don't like the idea of silently dropping the atomic append guarantee in case of a failure, so I'm not sure how to proceed with the current approach.

Moreover, the same issue would apply even in case of direct implementation of os.open()/open() via CreateFile() because we still need to wrap the handle with an fd, and this may fail with EMFILE. For O_CREAT/O_TRUNC, it seems like it could be reasonably avoided:

* Truncation can simply be deferred until we have the fd and then performed manually.

* To undo the file creation, we could use GetLastError() to learn whether CreateFile() with OPEN_ALWAYS actually created the file, and then delete it on failure (still non-atomic, and deletion can fail, but probably better than nothing).

But I still don't know how to deal with O_TEMPORARY, unless there is a way to unset FILE_DELETE_ON_CLOSE on a handle.

As an aside, it's also very surprising to me that O_TEMPORARY is allowed for existing files at all. If not for that, there would be no issue on failure apart from non-atomicity.

Maybe I should forgo the idea of supporting O_APPEND for os.open(), and instead just support it in FileIO via WriteFile()-with-OVERLAPPED approach...
History
Date User Action Args
2021-01-22 18:51:13izbyshevsetrecipients: + izbyshev, paul.moore, vstinner, tim.golden, zach.ware, eryksun, steve.dower
2021-01-22 18:51:13izbyshevsetmessageid: <1611341473.71.0.74295883981.issue42606@roundup.psfhosted.org>
2021-01-22 18:51:13izbyshevlinkissue42606 messages
2021-01-22 18:51:13izbyshevcreate