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

Enable CA2012 (Use ValueTask Correctly) #31221

Merged
merged 2 commits into from
Mar 26, 2021
Merged
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
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
stephentoub marked this conversation as resolved.
Show resolved Hide resolved
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();
stephentoub marked this conversation as resolved.
Show resolved Hide resolved
}
}

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();
stephentoub marked this conversation as resolved.
Show resolved Hide resolved
}

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;
stephentoub marked this conversation as resolved.
Show resolved Hide resolved

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;
halter73 marked this conversation as resolved.
Show resolved Hide resolved
}

_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();
stephentoub marked this conversation as resolved.
Show resolved Hide resolved
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();
stephentoub marked this conversation as resolved.
Show resolved Hide resolved
}

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)
stephentoub marked this conversation as resolved.
Show resolved Hide resolved
{
_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