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

[API Proposal]: Add CancellationToken to TextWriter.FlushAsync #64340

Closed
Shane32 opened this issue Jan 26, 2022 · 2 comments · Fixed by #84325
Closed

[API Proposal]: Add CancellationToken to TextWriter.FlushAsync #64340

Shane32 opened this issue Jan 26, 2022 · 2 comments · Fixed by #84325
Assignees
Labels
api-approved API was approved in API review, it can be implemented area-System.IO
Milestone

Comments

@Shane32
Copy link
Contributor

Shane32 commented Jan 26, 2022

Background and motivation

Some TextWriter implementations such as StreamWriter serve to encode data in a particular format and then write that data to an underlying data stream. The underlying data stream could easily be a file stream or network stream, where it is desired to be able to cancel any pending operation upon either an upstream cancellation (e.g. termination of the HTTP request that initiated such as data transfer) or user request (e.g. user clicks Cancel button).

Some implementations also buffer data before reaching the underlying stream, and without a call to Flush (or AutoFlush = true), do not flush those buffers to the underlying stream. So it is sometimes necessary in these instances to call Flush to ensure that the TextWriter implementation has flushed its buffers to the underlying stream, even if it is not necessary to flush the underlying stream. Typical implementations of Flush however will typically flush their own buffers and then call the Flush method of the underlying stream.

Calling FlushAsync on an underlying data stream can often result in an asynchronous delay while the data is written to disk or sent over the network. However, the method does not provide any means to cancel the underlying write/flush operation.

With the merging of #61898, both TextReader and TextWriter supports cancellation tokens for each of their methods (in some form) except this one.

Note that the Stream.FlushAsync method has an overload with a cancellation token since .NET Standard 1.0, so there is underlying support for the means to asynchronously flush buffers to disk/network as described in the example above.

API Proposal

public abstract class TextWriter : MarshalByRefObject, IAsyncDisposable, IDisposable
{
    public virtual System.Threading.Tasks.Task FlushAsync (CancellationToken cancellationToken);
}

API Usage

async Task PrintReportAsync(TextWriter writer, CancellationToken cancellationToken = default)
{
    await writer.WriteLineAsync("Some data".AsMemory(), cancellationToken);
    await writer.FlushAsync(cancellationToken);
}

Alternative Designs

Alternatively, ValueTask could be used, but based on the other APIs that do not return a value, it would seem that Task is preferred here.

public abstract class TextWriter : MarshalByRefObject, IAsyncDisposable, IDisposable
{
    public virtual System.Threading.Tasks.ValueTask FlushAsync (CancellationToken cancellationToken);
}

Risks

No response

References

@Shane32 Shane32 added the api-suggestion Early API idea and discussion, it is NOT ready for implementation label Jan 26, 2022
@dotnet-issue-labeler dotnet-issue-labeler bot added area-System.IO untriaged New issue has not been triaged by the area owner labels Jan 26, 2022
@ghost
Copy link

ghost commented Jan 26, 2022

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

Issue Details

Background and motivation

Some TextWriter implementations such as StreamWriter serve to encode data in a particular format and then write that data to an underlying data stream. The underlying data stream could easily be a file stream or network stream, where it is desired to be able to cancel any pending operation upon either an upstream cancellation (e.g. termination of the HTTP request that initiated such as data transfer) or user request (e.g. user clicks Cancel button).

Some implementations also buffer data before reaching the underlying stream, and without a call to Flush (or AutoFlush = true), do not flush those buffers to the underlying stream. So it is sometimes necessary in these instances to call Flush to ensure that the TextWriter implementation has flushed its buffers to the underlying stream, even if it is not necessary to flush the underlying stream. Typical implementations of Flush however will typically flush their own buffers and then call the Flush method of the underlying stream.

Calling FlushAsync on an underlying data stream can often result in an asynchronous delay while the data is written to disk or sent over the network. However, the method does not provide any means to cancel the underlying write/flush operation.

With the merging of #61898, both TextReader and TextWriter supports cancellation tokens for each of their methods (in some form) except this one.

Note that the Stream.FlushAsync method has an overload with a cancellation token since .NET Standard 1.0, so there is underlying support for the means to asynchronously flush buffers to disk/network as described in the example above.

References:

API Proposal

public abstract class TextWriter : MarshalByRefObject, IAsyncDisposable, IDisposable
{
    public virtual System.Threading.Tasks.Task FlushAsync (CancellationToken cancellationToken);
}

API Usage

async Task PrintReportAsync(TextWriter writer, CancellationToken cancellationToken = default)
{
    await writer.WriteLineAsync("Some data".AsMemory(), cancellationToken);
    await writer.FlushAsync(cancellationToken);
}

Alternative Designs

Alternatively, ValueTask could be used, but based on the other APIs that do not return a value, it would seem that Task is preferred here.

public abstract class TextWriter : MarshalByRefObject, IAsyncDisposable, IDisposable
{
    public virtual System.Threading.Tasks.ValueTask FlushAsync (CancellationToken cancellationToken);
}

Risks

No response

Author: Shane32
Assignees: -
Labels:

api-suggestion, area-System.IO, untriaged

Milestone: -

@adamsitnik adamsitnik removed the untriaged New issue has not been triaged by the area owner label May 16, 2022
@adamsitnik adamsitnik added this to the Future milestone May 16, 2022
@stephentoub stephentoub added api-ready-for-review API is ready for review, it is NOT ready for implementation and removed api-suggestion Early API idea and discussion, it is NOT ready for implementation labels Sep 2, 2022
@stephentoub stephentoub modified the milestones: Future, 8.0.0 Apr 1, 2023
@terrajobst
Copy link
Member

terrajobst commented Apr 4, 2023

Video

  • Looks good as proposed
namespace System.IO;

public partial class TextWriter
{
    // Existing:
    // public virtual Task FlushAsync();
    public virtual Task FlushAsync(CancellationToken cancellationToken);
}

@terrajobst terrajobst added api-approved API was approved in API review, it can be implemented and removed api-ready-for-review API is ready for review, it is NOT ready for implementation labels Apr 4, 2023
@stephentoub stephentoub self-assigned this Apr 4, 2023
@ghost ghost added the in-pr There is an active PR which will close this issue when it is merged label Apr 4, 2023
@ghost ghost removed the in-pr There is an active PR which will close this issue when it is merged label Apr 25, 2023
@ghost ghost locked as resolved and limited conversation to collaborators May 25, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
api-approved API was approved in API review, it can be implemented area-System.IO
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants