Feature/improvements to the windows filesystem implementation #210
Conversation
| // NB: We have never seen `ERROR_MORE_DATA` during development, | ||
| // it seems it is never raised. But according to the docs is should be set | ||
| // and we should proceed with the happy path. |
There was a problem hiding this comment.
That's because you've never attempted to read directories containing filenames that are more than 256 bytes long. Please remove this comment, as it is a valid case that we should handle.
There was a problem hiding this comment.
Aha, that makes sense. I will setup a test to verify that this works. Have you used this API before, or know of good documentation? I have mostly been tinkering with it manually to see what pops back out.
https://learn.microsoft.com/en-us/windows/win32/api/winbase/nf-winbase-getfileinformationbyhandleex
https://github.com/microsoft/Windows-classic-samples/blob/main/Samples/Win7Samples/winbase/io/extendedfileapis/ExtendedFileAPIs.cpp#L468
There was a problem hiding this comment.
I don't know anything about it. I don't use Windows myself. ;-)
There was a problem hiding this comment.
I'll need some more time to tinker with this.
I now have a manual test that hits the ERROR_MORE_DATA for a 250-character filename and a small initial buffer.
Will wire that up.
Have not been able to test against the long-file path API as it is hard to actually create such files. I'm not very familiar with the special APIs either.
| // would fill. Any larger allocation would be unused and lead to | ||
| // resource exhaustion. We have not found good documentation for how to | ||
| // use this. | ||
| outBufferSize = clamp(outBufferSize*2, 62000) |
There was a problem hiding this comment.
Instead of doing this, why don't we simply use a buffer that is twice as big as what the last call to GetFileInformationByHandleEx() filled? So if we give it a 4 KB buffer and the kernel fills the entire 4 KB, then the next time we call it with 8 KB. Then we don't need to make guesses about what the kernel's limit actually is.
| } | ||
| names = append(names, fileName) | ||
| continue |
There was a problem hiding this comment.
This statement isn't necessary, right? A loop always continues on its own.
stagnation
force-pushed
the
feature/improvements-to-the-windows-filesystem-implementation
branch
2 times, most recently
from
fb4b004 to
fa362db
Compare
The 'ERROR_MORE_DATA' error is only raised if a single entry could not fit into the buffer, not that there are no more entries. This instead polls until the bookend 'ERROR_NO_MORE_DATA' is raised instead.
This fixes execution errors where some programs fail to execute while a source file is hardlinked into another scheduled actions. Many compilers restrict the sharing mode for the files they open, 'cl.exe' for instance does not allow other processes to have open WRITE handles to its source files, and fails with SHARING_VIOLATION if such handles are open. The hardlinking file fetcher does not need this permission and we solve many crashes by removing it.
stagnation
force-pushed
the
feature/improvements-to-the-windows-filesystem-implementation
branch
from
fa362db to
d0d76db
Compare
stagnation
deleted the
feature/improvements-to-the-windows-filesystem-implementation
branch
This fixes some issues we found when developing windows runners.