Skip to content

Commit

Permalink
Enable CA2012 (Use ValueTask Correctly) (#31221)
Browse files Browse the repository at this point in the history
  • Loading branch information
stephentoub authored Mar 26, 2021
1 parent f5feab7 commit d86be87
Show file tree
Hide file tree
Showing 17 changed files with 61 additions and 65 deletions.
6 changes: 6 additions & 0 deletions .editorconfig
Original file line number Diff line number Diff line change
Expand Up @@ -72,3 +72,9 @@ charset = utf-8-bom
[*.{cs,vb}]
# CA1305: Specify IFormatProvider
dotnet_diagnostic.CA1305.severity = error

[*.{cs,vb}]
# CA2012: Use ValueTask correctly
dotnet_diagnostic.CA2012.severity = warning
[**/{test,perf}/**.{cs,vb}]
dotnet_diagnostic.CA2012.severity = suggestion
2 changes: 1 addition & 1 deletion src/Components/Components/src/Routing/Router.cs
Original file line number Diff line number Diff line change
Expand Up @@ -267,7 +267,7 @@ private void OnLocationChanged(object sender, LocationChangedEventArgs args)
_locationAbsolute = args.Location;
if (_renderHandle.IsInitialized && Routes != null)
{
_ = RunOnNavigateAsync(NavigationManager.ToBaseRelativePath(_locationAbsolute), args.IsNavigationIntercepted);
_ = RunOnNavigateAsync(NavigationManager.ToBaseRelativePath(_locationAbsolute), args.IsNavigationIntercepted).Preserve();
}
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -75,7 +75,7 @@ protected override void NavigateToCore(string uri, bool forceLoad)
throw new NavigationException(absoluteUriString);
}

_jsRuntime.InvokeAsync<object>(Interop.NavigateTo, uri, forceLoad);
_jsRuntime.InvokeAsync<object>(Interop.NavigateTo, uri, forceLoad).Preserve();
}

private static class Log
Expand Down
2 changes: 1 addition & 1 deletion src/Hosting/Hosting/src/Internal/WebHost.cs
Original file line number Diff line number Diff line change
Expand Up @@ -334,7 +334,7 @@ public async Task StopAsync(CancellationToken cancellationToken = default)

public void Dispose()
{
DisposeAsync().GetAwaiter().GetResult();
DisposeAsync().AsTask().GetAwaiter().GetResult();
}

public async ValueTask DisposeAsync()
Expand Down
2 changes: 1 addition & 1 deletion src/Http/Http/src/Features/RequestServicesFeature.cs
Original file line number Diff line number Diff line change
Expand Up @@ -87,7 +87,7 @@ static async ValueTask Awaited(RequestServicesFeature servicesFeature, ValueTask
/// <inheritdoc />
public void Dispose()
{
DisposeAsync().GetAwaiter().GetResult();
DisposeAsync().AsTask().GetAwaiter().GetResult();
}
}
}
37 changes: 15 additions & 22 deletions src/Servers/Kestrel/Core/src/Internal/Http/Http1OutputProducer.cs
Original file line number Diff line number Diff line change
Expand Up @@ -58,9 +58,7 @@ internal class Http1OutputProducer : IHttpOutputProducer, IDisposable

private bool _autoChunk;

// We rely on the TimingPipeFlusher to give us ValueTasks that can be safely awaited multiple times.
private bool _writeStreamSuffixCalled;
private ValueTask<FlushResult> _writeStreamSuffixValueTask;

private int _advancedBytesForChunk;
private Memory<byte> _currentChunkMemory;
Expand Down Expand Up @@ -118,31 +116,27 @@ public ValueTask<FlushResult> WriteDataToPipeAsync(ReadOnlySpan<byte> buffer, Ca

public ValueTask<FlushResult> WriteStreamSuffixAsync()
{
ValueTask<FlushResult> result = default;

lock (_contextLock)
{
if (_writeStreamSuffixCalled)
if (!_writeStreamSuffixCalled)
{
// If WriteStreamSuffixAsync has already been called, no-op and return the previously returned ValueTask.
return _writeStreamSuffixValueTask;
}
if (_autoChunk)
{
var writer = new BufferWriter<PipeWriter>(_pipeWriter);
result = WriteAsyncInternal(ref writer, EndChunkedResponseBytes);
}
else if (_unflushedBytes > 0)
{
result = FlushAsync();
}

if (_autoChunk)
{
var writer = new BufferWriter<PipeWriter>(_pipeWriter);
_writeStreamSuffixValueTask = WriteAsyncInternal(ref writer, EndChunkedResponseBytes);
}
else if (_unflushedBytes > 0)
{
_writeStreamSuffixValueTask = FlushAsync();
}
else
{
_writeStreamSuffixValueTask = default;
_writeStreamSuffixCalled = true;
}

_writeStreamSuffixCalled = true;
return _writeStreamSuffixValueTask;
}

return result;
}

public ValueTask<FlushResult> FlushAsync(CancellationToken cancellationToken = default)
Expand Down Expand Up @@ -533,7 +527,6 @@ public void Reset()
_currentMemoryPrefixBytes = 0;
_autoChunk = false;
_writeStreamSuffixCalled = false;
_writeStreamSuffixValueTask = default;
_currentChunkMemoryUpdated = false;
_startCalled = false;
}
Expand Down
12 changes: 11 additions & 1 deletion src/Servers/Kestrel/Core/src/Internal/Http/HttpProtocol.cs
Original file line number Diff line number Diff line change
Expand Up @@ -917,7 +917,15 @@ public void ProduceContinue()
((IHeaderDictionary)HttpRequestHeaders).TryGetValue(HeaderNames.Expect, out var expect) &&
(expect.FirstOrDefault() ?? "").Equals("100-continue", StringComparison.OrdinalIgnoreCase))
{
Output.Write100ContinueAsync().GetAwaiter().GetResult();
ValueTask<FlushResult> vt = Output.Write100ContinueAsync();
if (vt.IsCompleted)
{
vt.GetAwaiter().GetResult();
}
else
{
vt.AsTask().GetAwaiter().GetResult();
}
}
}

Expand Down Expand Up @@ -1050,6 +1058,8 @@ private Task WriteSuffix()
return WriteSuffixAwaited(writeTask);
}

writeTask.GetAwaiter().GetResult();

_requestProcessingStatus = RequestProcessingStatus.ResponseCompleted;

if (_keepAlive)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -57,7 +57,7 @@ public void UpdateWindows(int bytes)
if (streamWindowUpdateSize > 0)
{
// Writing with the FrameWriter should only fail if given a canceled token, so just fire and forget.
_ = _frameWriter.WriteWindowUpdateAsync(StreamId, streamWindowUpdateSize);
_ = _frameWriter.WriteWindowUpdateAsync(StreamId, streamWindowUpdateSize).Preserve();
}

UpdateConnectionWindow(bytes);
Expand Down Expand Up @@ -90,7 +90,7 @@ private void UpdateConnectionWindow(int bytes)
if (connectionWindowUpdateSize > 0)
{
// Writing with the FrameWriter should only fail if given a canceled token, so just fire and forget.
_ = _frameWriter.WriteWindowUpdateAsync(0, connectionWindowUpdateSize);
_ = _frameWriter.WriteWindowUpdateAsync(0, connectionWindowUpdateSize).Preserve();
}
}
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -157,7 +157,7 @@ public void Abort(ConnectionAbortedException ex)
{
if (TryClose())
{
_frameWriter.WriteGoAwayAsync(int.MaxValue, Http2ErrorCode.INTERNAL_ERROR);
_frameWriter.WriteGoAwayAsync(int.MaxValue, Http2ErrorCode.INTERNAL_ERROR).Preserve();
}

_frameWriter.Abort(ex);
Expand Down Expand Up @@ -1214,7 +1214,7 @@ private void UpdateConnectionState()

if (_gracefulCloseInitiator == GracefulCloseInitiator.Server && _clientActiveStreamCount > 0)
{
_frameWriter.WriteGoAwayAsync(int.MaxValue, Http2ErrorCode.NO_ERROR);
_frameWriter.WriteGoAwayAsync(int.MaxValue, Http2ErrorCode.NO_ERROR).Preserve();
}
}

Expand All @@ -1224,7 +1224,7 @@ private void UpdateConnectionState()
{
if (TryClose())
{
_frameWriter.WriteGoAwayAsync(_highestOpenedStreamId, Http2ErrorCode.NO_ERROR);
_frameWriter.WriteGoAwayAsync(_highestOpenedStreamId, Http2ErrorCode.NO_ERROR).Preserve();
}
}
else
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -43,7 +43,7 @@ internal class Http2OutputProducer : IHttpOutputProducer, IHttpOutputAborter, IV
private bool _writerComplete;

// Internal for testing
internal ValueTask _dataWriteProcessingTask;
internal Task _dataWriteProcessingTask;
internal bool _disposed;

/// <summary>The core logic for the IValueTaskSource implementation.</summary>
Expand Down Expand Up @@ -394,7 +394,7 @@ public void Reset()
{
}

private async ValueTask ProcessDataWrites()
private async Task ProcessDataWrites()
{
// ProcessDataWrites runs for the lifetime of the Http2OutputProducer, and is designed to be reused by multiple streams.
// When Http2OutputProducer is no longer used (e.g. a stream is aborted and will no longer be used, or the connection is closed)
Expand Down
4 changes: 2 additions & 2 deletions src/Servers/Kestrel/Core/src/Internal/Http2/Http2Stream.cs
Original file line number Diff line number Diff line change
Expand Up @@ -145,7 +145,7 @@ public void CompleteStream(bool errored)
if (!errored)
{
// Don't block on IO. This never faults.
_ = _http2Output.WriteRstStreamAsync(Http2ErrorCode.NO_ERROR);
_ = _http2Output.WriteRstStreamAsync(Http2ErrorCode.NO_ERROR).Preserve();
}
RequestBodyPipe.Writer.Complete();
}
Expand Down Expand Up @@ -545,7 +545,7 @@ internal void ResetAndAbort(ConnectionAbortedException abortReason, Http2ErrorCo

DecrementActiveClientStreamCount();
// Don't block on IO. This never faults.
_ = _http2Output.WriteRstStreamAsync(error);
_ = _http2Output.WriteRstStreamAsync(error).Preserve();

AbortCore(abortReason);
}
Expand Down
10 changes: 5 additions & 5 deletions src/Servers/Kestrel/Core/src/Internal/Http3/Http3Connection.cs
Original file line number Diff line number Diff line change
Expand Up @@ -180,7 +180,7 @@ public void Abort(ConnectionAbortedException ex, Http3ErrorCode errorCode)
{
if (TryClose())
{
SendGoAway(_highestOpenedStreamId);
SendGoAway(_highestOpenedStreamId).Preserve();
}

_errorCodeFeature.Error = (long)errorCode;
Expand Down Expand Up @@ -213,10 +213,10 @@ public void OnTimeout(TimeoutReason reason)
switch (reason)
{
case TimeoutReason.KeepAlive:
SendGoAway(_highestOpenedStreamId);
SendGoAway(_highestOpenedStreamId).Preserve();
break;
case TimeoutReason.TimeoutFeature:
SendGoAway(_highestOpenedStreamId);
SendGoAway(_highestOpenedStreamId).Preserve();
break;
case TimeoutReason.RequestHeaders:
case TimeoutReason.ReadDataRate:
Expand Down Expand Up @@ -396,7 +396,7 @@ private void UpdateConnectionState()
if (_gracefulCloseInitiator == GracefulCloseInitiator.Server && activeRequestCount > 0)
{
// Go away with largest streamid to initiate graceful shutdown.
SendGoAway(VariableLengthIntegerHelper.EightByteLimit);
SendGoAway(VariableLengthIntegerHelper.EightByteLimit).Preserve();
}
}

Expand All @@ -406,7 +406,7 @@ private void UpdateConnectionState()
{
if (TryClose())
{
SendGoAway(_highestOpenedStreamId);
SendGoAway(_highestOpenedStreamId).Preserve();
}
}
else
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -51,7 +51,7 @@ public Http3OutputProducer(
_pipeReader = pipe.Reader;

_flusher = new TimingPipeFlusher(_pipeWriter, timeoutControl: null, log);
_dataWriteProcessingTask = ProcessDataWrites();
_dataWriteProcessingTask = ProcessDataWrites().Preserve();
}

public void Dispose()
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,7 @@ internal class QuicConnectionContext : TransportMultiplexedConnection, IProtocol
private readonly IQuicTrace _log;
private readonly CancellationTokenSource _connectionClosedTokenSource = new CancellationTokenSource();

private ValueTask _closeTask;
private Task? _closeTask;

public long Error { get; set; }

Expand Down Expand Up @@ -55,15 +55,8 @@ public override async ValueTask DisposeAsync()
{
try
{
if (_closeTask != default)
{
_closeTask = _connection.CloseAsync(errorCode: 0);
await _closeTask;
}
else
{
await _closeTask;
}
_closeTask ??= _connection.CloseAsync(errorCode: 0).AsTask();
await _closeTask;
}
catch (Exception ex)
{
Expand All @@ -78,7 +71,7 @@ public override async ValueTask DisposeAsync()
public override void Abort(ConnectionAbortedException abortReason)
{
// dedup calls to abort here.
_closeTask = _connection.CloseAsync(errorCode: Error);
_closeTask = _connection.CloseAsync(errorCode: Error).AsTask();
}

public override async ValueTask<ConnectionContext?> AcceptAsync(CancellationToken cancellationToken = default)
Expand Down
7 changes: 4 additions & 3 deletions src/Shared/ServerInfrastructure/DuplexPipeStream.cs
Original file line number Diff line number Diff line change
Expand Up @@ -71,9 +71,10 @@ public override void SetLength(long value)

public override int Read(byte[] buffer, int offset, int count)
{
// ValueTask uses .GetAwaiter().GetResult() if necessary
// https://github.com/dotnet/corefx/blob/f9da3b4af08214764a51b2331f3595ffaf162abe/src/System.Threading.Tasks.Extensions/src/System/Threading/Tasks/ValueTask.cs#L156
return ReadAsyncInternal(new Memory<byte>(buffer, offset, count), default).Result;
ValueTask<int> vt = ReadAsyncInternal(new Memory<byte>(buffer, offset, count), default);
return vt.IsCompleted ?
vt.Result :
vt.AsTask().GetAwaiter().GetResult();
}

public override Task<int> ReadAsync(byte[] buffer, int offset, int count, CancellationToken cancellationToken = default)
Expand Down
9 changes: 1 addition & 8 deletions src/SignalR/samples/ClientSample/Tcp/TcpConnection.cs
Original file line number Diff line number Diff line change
Expand Up @@ -168,14 +168,7 @@ private async Task ProcessReceives()

_application.Output.Advance(bytesReceived);

var flushTask = _application.Output.FlushAsync();

if (!flushTask.IsCompleted)
{
await flushTask;
}

var result = flushTask.GetAwaiter().GetResult();
var result = await _application.Output.FlushAsync();
if (result.IsCompleted)
{
// Pipe consumer is shut down, do we stop writing
Expand Down
2 changes: 1 addition & 1 deletion src/SignalR/server/Core/src/HubConnectionContext.cs
Original file line number Diff line number Diff line change
Expand Up @@ -625,7 +625,7 @@ private void KeepAliveTick()
// If the transport channel is full, this will fail, but that's OK because
// adding a Ping message when the transport is full is unnecessary since the
// transport is still in the process of sending frames.
_ = TryWritePingAsync();
_ = TryWritePingAsync().Preserve();

// We only update the timestamp here, because updating on each sent message is bad for performance
// There can be a lot of sent messages per 15 seconds
Expand Down

0 comments on commit d86be87

Please sign in to comment.