Skip to content
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

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

Open
wants to merge 2 commits into
base: release/9.0-staging
Choose a base branch
from

Conversation

adamsitnik
Copy link
Member

@adamsitnik adamsitnik commented Nov 8, 2024

Backport of #108380 and #109340 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
* 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 adamsitnik self-assigned this Nov 8, 2024
Copy link
Contributor

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

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

/azp list

Copy link

CI/CD Pipelines for this repository:

@adamsitnik
Copy link
Member Author

/azp run runtime-libraries-coreclr outerloop

Copy link

Azure Pipelines successfully started running 1 pipeline(s).

@ericstj
Copy link
Member

ericstj commented Nov 19, 2024

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

@adamsitnik
Copy link
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 adamsitnik added the NO-MERGE The PR is not ready for merge yet (see discussion for detailed reasons) label Nov 20, 2024
@ericstj ericstj removed the Servicing-consider Issue for next servicing release review label Nov 20, 2024
@ericstj
Copy link
Member

ericstj commented Nov 20, 2024

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

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area-System.IO NO-MERGE The PR is not ready for merge yet (see discussion for detailed reasons)
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants