[issue46785] On Windows, os.stat() can fail if called while another process is creating or deleting the file

[Steve Dower]

Steve Dower  added the comment:

> It would probably be better to skip tests if the filesystem of the current 
> working directory doesn't support the test, 

Yes, this would be good. Then whoever is configuring the test runner can 
move where tests are run to make sure it is supported. There are command 
line options specifically for this, that also correctly handle 
multiprocessing.

Tests that bypass the CWD make this unfixable by the runner, which is 
why they should just use CWD.

--

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue46785] On Windows, os.stat() can fail if called while another process is creating or deleting the file

[Eryk Sun]

Eryk Sun  added the comment:

I was following the pattern of StatAttributeTests.test_access_denied(), which 
uses the current user's temp directory to get a filesystem that supports 
security. 

It would probably be better to skip tests if the filesystem of the current 
working directory doesn't support the test, e.g. if it needs NTFS or needs 
support for security, hard links, or reparse points. For example, 
Win32JunctionTests should be skipped if reparse points aren't supported. The 
os_helper module could implement a function that calls GetVolumeInformationW() 
to get the filesystem name (e.g. "Ntfs") and flags (e.g. FILE_PERSISTENT_ACLS, 
FILE_SUPPORTS_HARD_LINKS, FILE_SUPPORTS_REPARSE_POINTS).

--

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue46785] On Windows, os.stat() can fail if called while another process is creating or deleting the file

[Steve Dower]

Steve Dower  added the comment:

>> Why can't the filename of the "foo"-like file in the test be
>> simply os_helper.TESTFN, as done in some other tests?
> 
> I suppose the current working directory will be fine. I was looking to keep 
> the test on a NTFS filesystem, with known behavior, but there's no hard 
> guarantee that the user's temp directory is on the system volume.

All tests should use the current working directory, or the test helper 
for getting other directories. *Do not look up other directories*, 
because it prevents test runners from ensuring that tests run in valid 
locations (and skips implicit tests for directories with spaces/Unicode 
characters/etc.)

--

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue46785] On Windows, os.stat() can fail if called while another process is creating or deleting the file

[Antony Lee]

Change by Antony Lee :


--
nosy:  -Antony.Lee

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue46785] On Windows, os.stat() can fail if called while another process is creating or deleting the file

[Itai Steinherz]

Itai Steinherz  added the comment:

Interesting, thanks again :)

--

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue46785] On Windows, os.stat() can fail if called while another process is creating or deleting the file

[Eryk Sun]

Eryk Sun  added the comment:

> Why catch ERROR_NOT_READY and ERROR_BAD_NET_NAME as well?

When os.stat() falls back on FindFirstFileW(), an error that means the file 
doesn't exist should be kept. ERROR_BAD_NET_NAME is an obvious error to keep 
because it's already mapped to ENOENT (i.e. file not found). This error 
typically means that share was removed or the network went down. 
ERROR_NOT_READY typically means that the media was removed from a volume (e.g. 
ejected disk). pathlib.Path.exists() returns False for ERROR_NOT_READY.

> Why can't the filename of the "foo"-like file in the test be 
> simply os_helper.TESTFN, as done in some other tests?

I suppose the current working directory will be fine. I was looking to keep the 
test on a NTFS filesystem, with known behavior, but there's no hard guarantee 
that the user's temp directory is on the system volume.

> I noticed some code snippets used in tests are wrapped in a 
> seemingly-redundant `if 1: ...` statement. Why is that?

The `if 1` test is logically redundant but convenient for using a multiline 
string that has an arbitrary indentation level. For example:

>>> exec('''
... print(42)
... ''')
Traceback (most recent call last):
  File "", line 1, in 
  File "", line 2
print(42)
IndentationError: unexpected indent

>>> exec('''if 1:
... print(42)
... ''')
42

--

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue46785] On Windows, os.stat() can fail if called while another process is creating or deleting the file

[Itai Steinherz]

Change by Itai Steinherz :


--
keywords: +patch
pull_requests: +29956
stage: needs patch -> patch review
pull_request: https://github.com/python/cpython/pull/31858

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue46785] On Windows, os.stat() can fail if called while another process is creating or deleting the file

[Itai Steinherz]

Itai Steinherz  added the comment:

Thanks for the comprehensive reply, Eryk!

I have a few questions regarding your suggestion:

1. Why catch ERROR_NOT_READY and ERROR_BAD_NET_NAME as well? How do you know 
that FindFirstFileW() may return them?
2. Why can't the filename of the "foo"-like file in the test be simply 
os_helper.TESTFN, as done in some other tests?
3. I noticed some code snippets used in tests are wrapped in a 
seemingly-redundant `if 1: ...` statement. Why is that?

--

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue46785] On Windows, os.stat() can fail if called while another process is creating or deleting the file

[Eryk Sun]

Eryk Sun  added the comment:

Itai, you can add a test to Win32NtTests in Lib/test/test_os.py. Maybe spawn a 
child process that creates and unlinks a file in a loop. In the parent process 
execute a loop that tries to stat the file and ignores errors when the file or 
path isn't found. For example:

@support.requires_subprocess()
def test_stat_unlink_race(self):
# bpo-46785: the implementation of os.stat() falls back on reading
# the parent directory if CreateFileW() fails with a permission
# error. If reading the parent directory fails because the file or
# directory is subsequently unlinked or because the volume or
# share is no longer available, then the original permission error
# should not be restored.
fname = os.path.join(os.environ['TEMP'], os_helper.TESTFN + '_46785')
self.addCleanup(os_helper.unlink, fname)
command = '''if 1:
import os
import sys
fname = sys.argv[1]
while True:
try:
with open(fname, "w") as f:
pass
except OSError:
pass
try:
os.remove(fname)
except OSError:
pass
'''
ignored_errors = (
2,  # ERROR_FILE_NOT_FOUND
3,  # ERROR_PATH_NOT_FOUND
21, # ERROR_NOT_READY
67, # ERROR_BAD_NET_NAME
)
deadline = time.time() + 5
p = subprocess.Popen([sys.executable, '-c', command, fname])
try:
while time.time() < deadline:
try:
os.stat(fname)
except OSError as e:
if e.winerror not in ignored_errors:
raise
finally:
p.terminate()

As the above test shows, I think the error should also be kept if 
attributes_from_dir() fails with ERROR_NOT_READY or ERROR_BAD_NET_NAME. For 
example:

switch (error) {
case ERROR_ACCESS_DENIED: /* Cannot sync or read attributes. */
case ERROR_SHARING_VIOLATION: /* It's a paging file. */
/* Try reading the parent directory. */
if (!attributes_from_dir(path, &fileInfo, &tagInfo.ReparseTag)) {
/* Cannot read the parent directory. */
switch (GetLastError()) {
// keep these error codes
case ERROR_FILE_NOT_FOUND:
case ERROR_PATH_NOT_FOUND:
case ERROR_NOT_READY:
case ERROR_BAD_NET_NAME:
break;
// restore the error from CreateFileW()
default:
SetLastError(error);
}
return -1;
}

--

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue46785] On Windows, os.stat() can fail if called while another process is creating or deleting the file

[Itai Steinherz]

Itai Steinherz  added the comment:

I'd like to work on this, however I'm not sure how this could be unit-tested. 
Any ideas?

--
nosy: +itaisteinherz

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue46785] On Windows, os.stat() can fail if called while another process is creating or deleting the file

[Eryk Sun]

Eryk Sun  added the comment:

Windows filesystems disallow new opens for a file that has its delete 
disposition set (i.e. the file is marked for deletion). For example, 
CreateFileW() fails with ERROR_ACCESS_DENIED (5) in this case. A file with its 
delete disposition set is still visibly linked in its parent directory. 

The trigger to unlink a file that's marked for deletion depends on the delete 
mode. In Windows 10, DeleteFileW() uses a POSIX delete if it's supported by the 
filesystem (e.g. NTFS). This delete mode unlinks the file as soon the file 
object that was used to set the delete disposition is closed. There's still a 
small window of time, however, in which attempts to open the file will fail 
with an access-denied error.

In os.stat(), if CreateFileW() fails with access denied, FindFirstFileW() is 
called to try to get the stat data from the file's parent directory. But in 
this case it's likely that the file has been unlinked from the directory by the 
time FindFirstFileW() is called.

The original error code from CreateFileW() gets restored if FindFirstFileW() 
fails. This is generally the right thing to do. However, if FindFirstFileW() 
fails with ERROR_FILE_NOT_FOUND (2) or ERROR_PATH_NOT_FOUND (3), then I suggest 
that the previous error code should not be restored.

For example:

switch (error) {
case ERROR_ACCESS_DENIED: /* Cannot sync or read attributes. */
case ERROR_SHARING_VIOLATION: /* It's a paging file. */
/* Try reading the parent directory. */
if (!attributes_from_dir(path, &fileInfo, &tagInfo.ReparseTag)) {
/* Cannot read the parent directory. */
DWORD dir_error = GetLastError();
if (dir_error != ERROR_FILE_NOT_FOUND &&
  dir_error != ERROR_PATH_NOT_FOUND) {
SetLastError(error);
}
return -1;
}

--
nosy: +eryksun
stage:  -> needs patch
type:  -> behavior
versions: +Python 3.11, Python 3.9

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue46785] On Windows, os.stat() can fail if called while another process is creating or deleting the file

[Antony Lee]

New submission from Antony Lee :

In a first Python process, repeatedly create and delete a file:

from pathlib import Path
while True:
Path("foo").touch(); Path("foo").unlink()

In another process, repeatedly check for the path's existence:

from pathlib import Path
while True: print(Path("foo").exists())

On Linux, the second process prints a random series of True and False.  On 
Windows, it quickly fails after a few dozen iterations (likely 
machine-dependent) with

PermissionError: [WinError 5] Access is denied: 'foo'

which is actually raised by the stat() call.

I would suggest that this is not really desirable behavior?

--
components: Windows
messages: 413468
nosy: Antony.Lee, paul.moore, steve.dower, tim.golden, zach.ware
priority: normal
severity: normal
status: open
title: On Windows, os.stat() can fail if called while another process is 
creating or deleting the file
versions: Python 3.10

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com