FazBrowse GitHub Viewer | Trending |
URL:
| Home
Tools: [Download Repo ZIP]   [Original HTTPS Page]

[release/9.0-staging] Fix few RandomAccess.Write edge case bugs by adamsitnik · Pull Request #109646 · dotnet/runtime · GitHub

  1. dotnet
  2. runtime

Repository navigation

[release/9.0-staging] Fix few RandomAccess.Write edge case bugs - #109646

Merged
adamsitnik merged 4 commits into
dotnet:release/9.0-stagingfrom
adamsitnik:backportWriteGather9
Jul 11, 2025
ConversationCommits (4)ChecksFiles changed
Merged

[release/9.0-staging] Fix few RandomAccess.Write edge case bugs#109646
adamsitnik merged 4 commits into
dotnet:release/9.0-stagingfrom
adamsitnik:backportWriteGather9

Conversation

adamsitnik commented Nov 8, 2024 •
edited
Loading

Copy link
Copy Markdown
Member

Backport of #108380 and #109340 and #109826 to release/9.0-staging

Customer Impact

  • Customer reported
  • Found internally

Customers using RandomAccess.Write overload that accepts multiple buffers have observed IOException being thrown in two scenarios:

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

  • Yes
  • No

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.

NicoAvanzDev and others added 2 commits November 8, 2024 17:06
RandomAccess.Write throws System.IO.IOException Fix (dotnet#108380)
f3a867b
Handle IOV_MAX limit in RandomAccess, add missing test (dotnet#109340)
861b80d 
* 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 <3840695+am11@users.noreply.github.com>
Co-authored-by: Stephen Toub <stoub@microsoft.com>
adamsitnik self-assigned this Nov 8, 2024

dotnet-policy-service Bot commented Nov 8, 2024

Copy link
Copy Markdown
Contributor

Tagging subscribers to this area: @dotnet/area-system-io
See info in area-owners.md if you want to be subscribed.

adamsitnik added this to the 9.0.x milestone Nov 8, 2024
adamsitnik added the Servicing-consider Issue for next servicing release review label Nov 8, 2024

adamsitnik commented Nov 8, 2024

Copy link
Copy Markdown
Member Author

/azp list

azure-pipelines Bot commented Nov 8, 2024

Copy link
Copy Markdown
CI/CD Pipelines for this repository:

adamsitnik commented Nov 8, 2024

Copy link
Copy Markdown
Member Author

/azp run runtime-libraries-coreclr outerloop

azure-pipelines Bot commented Nov 8, 2024

Copy link
Copy Markdown
Azure Pipelines successfully started running 1 pipeline(s).

This was referenced Nov 9, 2024
Closed
Open
Closed
Closed
Closed

ericstj commented Nov 19, 2024

Copy link
Copy Markdown
Member

@adamsitnik did you want to port the same test fixes to this PR that you mentioned in #109648?

adamsitnik commented Nov 20, 2024

Copy link
Copy Markdown
Member Author

@adamsitnik did you want to port the same test fixes to this PR that you mentioned in #109648?

@ericstj yes, and to be exact the fixes are #109826

adamsitnik added the NO-MERGE The PR is not ready for merge yet (see discussion for detailed reasons) label Nov 20, 2024
ericstj removed the Servicing-consider Issue for next servicing release review label Nov 20, 2024

ericstj commented Nov 20, 2024

Copy link
Copy Markdown
Member

Ok, please add back servicing-consider when it's ready.

More WriteGather fixes (dotnet#109826)
f62051c 
* 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 <35800402+MichalPetryka@users.noreply.github.com>

adamsitnik commented Dec 12, 2024

Copy link
Copy Markdown
Member Author

/azp run runtime-libraries-coreclr outerloop

azure-pipelines Bot commented Dec 12, 2024

Copy link
Copy Markdown
Azure Pipelines successfully started running 1 pipeline(s).

build-analysis Bot mentioned this pull request Dec 13, 2024
Open
3 tasks
carlossanlop added NO-MERGE The PR is not ready for merge yet (see discussion for detailed reasons) and removed NO-MERGE The PR is not ready for merge yet (see discussion for detailed reasons) labels Jan 29, 2025

carlossanlop commented Apr 2, 2025 •
edited
Loading

Copy link
Copy Markdown
Contributor

@adamsitnik Friendly reminder that code complete is on April 14th for the May Release. If you'd like to get this change included in that release, please get a Tactics approval and merge this PR before that date.

jozkee commented May 7, 2025

Copy link
Copy Markdown
Member

@adamsitnik friendly reminder that code complete is on Monday May 12th (2:00 PM Pacific) for the June Release. If you'd like to get this change included in that release, please get a Tactics approval and merge this PR before the deadline.

This was referenced Aug 7, 2025
Closed
Merged
Closed
Merged
Merged
Merged
Merged
Closed
Closed
Merged
Merged
Closed
Closed
Closed
Closed
Closed
Closed
Closed
Closed
Closed
Merged
Closed
Merged
Merged
Merged
Merged
Merged
Merged
Closed
github-actions Bot locked and limited conversation to collaborators Aug 11, 2025
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
Show hidden characters
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Reviewers

stephentoub stephentoub approved these changes

Assignees

adamsitnik

Labels

area-System.IO Servicing-approved Approved for servicing release

Projects

None yet

Milestone

9.0.8

Development

Successfully merging this pull request may close these issues.

8 participants

Back | FazBrowse Home | New Git URL