[release/8.0-staging] Fix few RandomAccess.Write edge case bugs #109648
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
* add test for Int32 overflow for WriteGather in RandomAccess * add failing test fore more than IOV_MAX buffers * fix both the native and managed parts --------- Co-authored-by: Adeel Mujahid <[email protected]> Co-authored-by: Stephen Toub <[email protected]>
adamsitnik
added
Servicing-consider
|
/azp run runtime-libraries-coreclr outerloop |
|
Azure Pipelines successfully started running 1 pipeline(s). |
This was referenced Nov 8, 2024
Open
|
/azp run runtime-libraries-coreclr outerloop |
|
Azure Pipelines successfully started running 1 pipeline(s). |
stephentoub
approved these changes
Nov 13, 2024
adamsitnik
added
the
NO-MERGE
|
The failures are related, I am going to send a follow up PR to main and sync this branch soon |
jeffhandley
removed
the
Servicing-consider
|
@adamsitnik I've removed the https://github.com/dotnet/runtime/labels/servicing-consider since you're still investigating the CI failures. Once it's ready for tactics review, please reapply the label. Does this also need to be backported to release/9.0-staging? |
4 tasks
* don't run these tests in parallel, as each test cases uses more than 4 GB ram and disk! * fix the test: handle incomplete reads that should happen when we hit the max buffer limit * incomplete write fix: - pin the buffers only once - when re-trying, do that only for the actual reminder * Use native memory to get OOM a soon as we run out of memory (hoping to avoid the process getting killed on Linux when OOM happens) * For macOS preadv and pwritev can fail with EINVAL when the total length of all vectors overflows a 32-bit integer. * add an assert that is going to warn us if vector.Count is ever more than Int32.MaxValue --------- Co-authored-by: Michał Petryka <[email protected]> # Conflicts: # src/libraries/System.IO.FileSystem/tests/RandomAccess/WriteGatherAsync.cs
|
/azp run runtime-libraries-coreclr outerloop |
|
Azure Pipelines successfully started running 1 pipeline(s). |
This was referenced Aug 7, 2025
Closed
Closed
Closed
Closed
Merged
Closed
Closed
Sign up for free
to subscribe to this conversation on GitHub.
Already have an account?
Sign in.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Backport of #108380 and #109340 and #109826 to release/8.0-staging
Customer Impact
Customers using
RandomAccess.Writeoverload that accepts multiple buffers have observedIOExceptionbeing thrown in two scenarios:Int32) overflows and the method tries to perform a write with negative file offset. Reported by the user in RandomAccess.Write throws System.IO.IOException: The parameter is incorrect. #108322.IOV_MAX(usually 1024). Reported by the user in RandomAccess.Write throws System.IO.IOException: Invalid argument when buffers count is greater than 1024 #108383.When I was fixing #108383 I've realized that the logic for handling incomplete writes has a bug.
For incomplete multi-buffer writes, the API was not reporting any exceptions. It returns void, so it should write everything or throw an exception.
Regression
We had all 3 bugs since the API was introduced (.NET 6).
Testing
New unit tests were added. They were failing before the fix were applied. They are passing now.
Risk
Low, the fix is simple and covered with tests.