!buildbot windows

🤖 New build scheduled with the buildbot fleet by @zooba for commit d7a4f36 🤖

The command will test the builders whose names match following regular expression: windows

The builders matched are:

• AMD64 Windows10 PR
• ARM64 Windows PR
• ARM64 Windows Non-Debug PR
• AMD64 Windows11 Bigmem PR
• AMD64 Windows11 Non-Debug PR
• AMD64 Windows Server 2022 NoGIL PR
• AMD64 Windows11 Refleaks PR

Looks like it's failing and needing to retry on GitHub as well, so no need to bother with buildbots.

Yeah. The buildbot did fail as well (had to wait as test_os ended up right at the very end of the tests). Weird, especially how it somehow passes on the next attempt. I wonder if dumping any stdout from icalcs might yield clues?

Edit: Oh, nevermind, you already did that...

test_stat_inaccessible_file (test.test_os.Win32NtTests.test_stat_inaccessible_file) ... processed file: D:\a\cpython\cpython\build\test_python_2432\x91\@test_2432_tmp\x91
Successfully processed 1 files; Failed processing 0 files
File: D:\a\cpython\cpython\build\test_python_2432�\@test_2432_tmp�
stat with access: os.stat_result(st_mode=33206, st_ino=26458647810816196, st_dev=13736023499467151587, st_nlink=1, st_uid=0, st_gid=0, st_size=9, st_atime=1705354784, st_mtime=1705354784, st_ctime=1705354776)
 without access: os.stat_result(st_mode=33206, st_ino=26458647810816196, st_dev=13736023499467151587, st_nlink=1, st_uid=0, st_gid=0, st_size=9, st_atime=1705354784, st_mtime=1705354784, st_ctime=1705354776)
FAIL

Well, that shows nothing directly. I guess there could be a race going on? I'll add a second icacls call to print its settings.

Maybe some short-lived filesystem metadata caching or something? But then why does the second always succeed? Maybe just the extra load with the parallel testing is increasing latency for the updated stat to be available to the test.

BTW, I can also fail this manually/interactively. Actually, so far I've yet to be able to get it to pass. The second icacls output shows no acls, yet the stat calls get the data. That has to be from stale metadata, right?

I just realised that I'm running on ReFS locally, and so it may be NTFS related. I'll try that and see if I can repro locally, which ought to help.

Taking a guess here that we might be sharing a file, or another test is leaking a handle, and so we still have access to the file because it's still open. Using a different filename ought to rule that out.

Custom filename still fails for me. I also tried pushing the initial file creation into a subprocess and removing the first stat call, so technically the first stat attempt was on the inaccessible file but somehow that works too. I'm very confused, and have actually started looking at the actual stat code...

So here's an interesting data point. I commented out the file removal code in the test, and then hacked the test so it used the existing file from the prior run (icacls showing no acls). The stat() call still succeeded (and the inode in fact the correct file inode).

I'm going to try manually setting up the file permissions as in the original issue to see if it behaves differently.

Hmm, I can't reproduce the problem as per the original issue. This is also on Windows 10 Enterprise, but 22H2 rather than 21H2. But my stat() works fine, and returns correct time values (and ino/dev), even without any access. Not quite sure what to make of things; clearly there must be something else different than the reporter.

Although this is with the latest 3.13 trunk, so I should probably build a copy of 3.12 for comparison.

Don't get too distracted by icacls - we're clearly setting up the file correctly.

What's happening is that the Python process is still able to access it, which should mean that either we're running with extra permissions in GitHub/Buildbots, or something else in the test suite is enabling it.

I've added a check so we can skip if changing the ACLs doesn't actually lock us out of the file (with retry and delay, just in case it's taking some time)

Yeah, except this last time I did everything manually and from explorer, just as in the ticket, and then just interactively used stat(). So I created a new file, removed inheritance in explorer (with a big warning about nobody having access), and then successfully got a stat() response from an interactive python. Worked fine (albeit with latest 3.x trunk)

my stat() works fine, and returns correct time values (and ino/dev), even without any access

You must have access, then, which is the problem we have here! When I run it, that icacls call locks me out, so I can't get full stat results. Everything but ino/dev we can get from the directory metadata, so when you have access to the directory but not the file, we should get that much (which is a pretty rare situation in practice, so I'm not hugely concerned).

That makes sense, but it sure looks like I have no more access than in the original ticket. The security tab in the properties window for the file shows:

No groups or users have permission to access this object. 
However, the owner of this object can assign permissions.

and nothing is checked anywhere in the permissions section. icacls shows nothing.

Solved it.

The combination of running as admin, passing FILE_FLAG_BACKUP_SEMANTICS and already having run test_create_junction are causing it.

Running as admin (I assume) means we have SE_BACKUP_NAME in our current security token, and _winapi.CreateJunction adds SE_RESTORE_NAME. Together, this means that we are allowed to bypass ACLs when opening a file with FILE_FLAG_BACKUP_SEMANTICS, which is what stat uses (in case a directory is being referenced).

If you skip the junction tests, you should see the correct behaviour. It passes on retry because the junction tests aren't re-run, and because they're all running in new processes. My extra code checking for inaccessible file doesn't help because regular open() doesn't pass FILE_FLAG_BACKUP_SEMANTICS and so it is correctly blocked.

So we probably want to restore the regular privilege level after creating a junction. I'll take a look at it tomorrow.

Oh, of course, not sure how I didn't immediately see that ... not! Nice catch.

Not entirely sure it matches all of my local testing, as I've only been running this one test interactively and from a new process, but it sounds like a solid theory for the buildbot runs, while my interactive tests couldn't explain why they ever worked. So I'm going to assume that there's some unrelated factor with my interactive security, which isn't worth worrying about.

I'll take a look at it tomorrow.

I lied, I did it tonight (well, it's almost tomorrow...). Created #114096 for it too, since this is NEWSworthy.

!buildbot windows

🤖 New build scheduled with the buildbot fleet by @zooba for commit bf3a59e 🤖

The command will test the builders whose names match following regular expression: windows

The builders matched are:

• AMD64 Windows10 PR
• ARM64 Windows PR
• ARM64 Windows Non-Debug PR
• AMD64 Windows11 Bigmem PR
• AMD64 Windows11 Non-Debug PR
• AMD64 Windows Server 2022 NoGIL PR
• AMD64 Windows11 Refleaks PR

Yep, looks like that did it. At least on GitHub - it's behind another test run on the buildbot - I'll monitor, but I expect it'll be fine. Sorry for the bit of snowballing effort this produced.

Buildbot looks fine too (the release builds are another known issue, hopefully fixed soon).

Thanks @zooba for the PR 🌮🎉.. I'm working now to backport this PR to: 3.11, 3.12.
🐍🍒⛏🤖

Sorry, @zooba, I could not cleanly backport this to 3.11 due to a conflict.
Please backport using cherry_picker on command line.

cherry_picker de4ced54eb08e8630e3b6c13436d4ecc3fb14708 3.11

GH-114134 is a backport of this pull request to the 3.12 branch.

GH-114135 is a backport of this pull request to the 3.11 branch.