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

Handle incomplete writes #55474

Closed
wants to merge 2 commits into from
Closed

Conversation

adamsitnik
Copy link
Member

This is a short term solution for #55473. It solves the problem, but only for FileStream and it's introducing a performance regression (allocations).

I can't provide a better solution right now because I am on vacations. I'll be back in a week, then I can provide a long-term fix.

@ghost
Copy link

ghost commented Jul 11, 2021

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

Issue Details

This is a short term solution for #55473. It solves the problem, but only for FileStream and it's introducing a performance regression (allocations).

I can't provide a better solution right now because I am on vacations. I'll be back in a week, then I can provide a long-term fix.

Author: adamsitnik
Assignees: -
Labels:

area-System.IO

Milestone: -

Span<byte> buffer = bytes;
do
{
int bytesWritten = RandomAccess.WriteAtOffset(sfh, buffer, offset);
Copy link
Member

@filipnavara filipnavara Jul 11, 2021

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Should this somehow handle !buffer.IsEmpty && bytesWritten == 0 to prevent infinite loop? (Logically, it would make more sense to me as a while loop instead of do-while loop).

#pragma warning restore CA2012

private ValueTask<int> WriteAsyncCore(ReadOnlyMemory<byte> source, CancellationToken cancellationToken)
public override ValueTask WriteAsync(ReadOnlyMemory<byte> source, CancellationToken cancellationToken)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
public override ValueTask WriteAsync(ReadOnlyMemory<byte> source, CancellationToken cancellationToken)
public override async ValueTask WriteAsync(ReadOnlyMemory<byte> source, CancellationToken cancellationToken)

@stephentoub
Copy link
Member

Thanks. #55490 provides the longer-term fix.

@adamsitnik adamsitnik deleted the incompleteWrites branch July 12, 2021 05:38
@ghost ghost locked as resolved and limited conversation to collaborators Aug 11, 2021
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants