-
Notifications
You must be signed in to change notification settings - Fork 5.2k
Don't assume all S_IFREG files are seekable #120736
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull Request Overview
This PR fixes an issue where regular files with S_IFREG type were incorrectly assumed to be seekable on all systems. The fix specifically addresses a problem on AzureLinux 3 where special files like /proc/net/route
are not seekable despite having the S_IFREG file type, causing assertions to fail in SafeFileHandle.Init.
- Adds a size check to only assume seekability for regular files with size > 0
- Preserves existing behavior for normal files while avoiding assumptions about zero-length special files
src/libraries/System.Private.CoreLib/src/Microsoft/Win32/SafeHandles/SafeFileHandle.Unix.cs
Outdated
Show resolved
Hide resolved
Tagging subscribers to this area: @dotnet/area-system-io |
cc @ManickaP |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The change is correct, but I would prefer to just modify the assertion. PTAL at my comment for full picture.
cc @tmds
src/libraries/System.Private.CoreLib/src/Microsoft/Win32/SafeHandles/SafeFileHandle.Unix.cs
Outdated
Show resolved
Hide resolved
cbf483b
to
2254d98
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Great catch, thanks!
Filed #120741 for followup on better CanSeek detection. |
/backport to release/9.0-staging |
Started backporting to release/9.0-staging: https://github.com/dotnet/runtime/actions/runs/18584668226 |
/backport to release/10.0 |
Started backporting to release/10.0: https://github.com/dotnet/runtime/actions/runs/18584672863 |
Fixes #120577.
It seems that special files like /proc/net/route are not seekable on AzureLinux 3, which trips an assert in SafeFileHandle.Init.
This PR excludes 0-length files (including all pseudofiles) from the assert to unblock CI.
Original outdated description
Since these files report 0 length in
fstat
, we can relax the seekabilit assumption to only files with Size > 0.In practice, most file reads are done via
RandomAccess.ReadAtOffset
and we keep our own seek offset in the SafeFileHandle internally, so seekability does not matter much (the affected unit tests don't fail in release builds), but it breaks in e.g. following scenario where access to SafeFileHandle property would attempt to synchronize the position with the underlying OS handle:This would produce: