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
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
20 changes: 18 additions & 2 deletions src/libraries/System.Private.CoreLib/src/System/IO/File.cs
Original file line number Diff line number Diff line change
Expand Up @@ -379,7 +379,15 @@ public static void WriteAllBytes(string path, byte[] bytes)
fs.Write(bytes, 0, bytes.Length);
#else
using SafeFileHandle sfh = OpenHandle(path, FileMode.Create, FileAccess.Write, FileShare.Read);
RandomAccess.WriteAtOffset(sfh, bytes, 0);

long offset = 0;
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).

offset += bytesWritten;
buffer = buffer.Slice(bytesWritten);
} while (!buffer.IsEmpty);
#endif
}
public static string[] ReadAllLines(string path)
Expand Down Expand Up @@ -840,7 +848,15 @@ static async Task Core(string path, byte[] bytes, CancellationToken cancellation
await fs.WriteAsync(bytes, 0, bytes.Length, cancellationToken).ConfigureAwait(false);
#else
using SafeFileHandle sfh = OpenHandle(path, FileMode.Create, FileAccess.Write, FileShare.Read, FileOptions.Asynchronous | FileOptions.SequentialScan);
await RandomAccess.WriteAtOffsetAsync(sfh, bytes, 0, cancellationToken).ConfigureAwait(false);

long offset = 0;
Memory<byte> buffer = bytes;
do
{
int bytesWritten = await RandomAccess.WriteAtOffsetAsync(sfh, buffer, offset, cancellationToken).ConfigureAwait(false);
offset += bytesWritten;
buffer = buffer.Slice(bytesWritten);
} while (!buffer.IsEmpty);
#endif
}
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -64,27 +64,22 @@ public override Task WriteAsync(byte[] buffer, int offset, int count, Cancellati
public override ValueTask WriteAsync(ReadOnlyMemory<byte> buffer, CancellationToken cancellationToken = default)
=> WriteAsyncInternal(buffer, cancellationToken);

private unsafe ValueTask WriteAsyncInternal(ReadOnlyMemory<byte> source, CancellationToken cancellationToken)
private async ValueTask WriteAsyncInternal(ReadOnlyMemory<byte> source, CancellationToken cancellationToken)
{
if (!CanWrite)
{
ThrowHelper.ThrowNotSupportedException_UnwritableStream();
}

long positionBefore = _filePosition;
if (CanSeek)
do
{
// When using overlapped IO, the OS is not supposed to
// touch the file pointer location at all. We will adjust it
// ourselves, but only in memory. This isn't threadsafe.
_filePosition += source.Length;
UpdateLengthOnChangePosition();
}
int bytesWritten = await RandomAccess.WriteAtOffsetAsync(_fileHandle, source, _filePosition, cancellationToken).ConfigureAwait(false);
_filePosition += bytesWritten;
source = source.Slice(bytesWritten);

(SafeFileHandle.OverlappedValueTaskSource? vts, int errorCode) = RandomAccess.QueueAsyncWriteFile(_fileHandle, source, positionBefore, cancellationToken);
return vts != null
? new ValueTask(vts, vts.Version)
: (errorCode == 0) ? ValueTask.CompletedTask : ValueTask.FromException(HandleIOError(positionBefore, errorCode));
// we update it now, because if the write was incomplete, the next try might fail with an exception
UpdateLengthOnChangePosition();
} while (!source.IsEmpty);
}

private Exception HandleIOError(long positionBefore, int errorCode)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -290,11 +290,18 @@ public sealed override void Write(ReadOnlySpan<byte> buffer)
ThrowHelper.ThrowNotSupportedException_UnwritableStream();
}

int r = RandomAccess.WriteAtOffset(_fileHandle, buffer, _filePosition);
Debug.Assert(r >= 0, $"RandomAccess.WriteAtOffset returned {r}.");
_filePosition += r;
do
{
int bytesWritten = RandomAccess.WriteAtOffset(_fileHandle, buffer, _filePosition);
Debug.Assert(bytesWritten >= 0, $"RandomAccess.WriteAtOffset returned {bytesWritten}.");

_filePosition += bytesWritten;
buffer = buffer.Slice(bytesWritten);

// we update it now, because if the write was incomplete, the next try might fail with an exception
UpdateLengthOnChangePosition();
} while (!buffer.IsEmpty);

UpdateLengthOnChangePosition();
}
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -58,28 +58,21 @@ public override void EndWrite(IAsyncResult asyncResult) =>
TaskToApm.End(asyncResult);

public override Task WriteAsync(byte[] buffer, int offset, int count, CancellationToken cancellationToken) =>
WriteAsyncCore(new ReadOnlyMemory<byte>(buffer, offset, count), cancellationToken).AsTask();
WriteAsync(new ReadOnlyMemory<byte>(buffer, offset, count), cancellationToken).AsTask();

public override ValueTask WriteAsync(ReadOnlyMemory<byte> source, CancellationToken cancellationToken) =>
#pragma warning disable CA2012 // The analyzer doesn't know the internal AsValueTask is safe.
WriteAsyncCore(source, cancellationToken).AsValueTask();
#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)

{
if (!CanWrite)
{
ThrowHelper.ThrowNotSupportedException_UnwritableStream();
}

long filePositionBefore = -1;
if (CanSeek)
do
{
filePositionBefore = _filePosition;
_filePosition += source.Length;
}

return RandomAccess.WriteAtOffsetAsync(_fileHandle, source, filePositionBefore, cancellationToken);
int bytesWritten = await RandomAccess.WriteAtOffsetAsync(_fileHandle, source, _filePosition, cancellationToken).ConfigureAwait(false);
_filePosition += bytesWritten;
source = source.Slice(bytesWritten);
} while (!source.IsEmpty);
}

/// <summary>Provides a reusable ValueTask-backing object for implementing ReadAsync.</summary>
Expand Down