Browse Source

Enable CA2012 (Use ValueTask Correctly) (#31221)

Stephen Toub 5 years ago
parent
commit
d86be87daa

+ 6 - 0
.editorconfig

@@ -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

+ 1 - 1
src/Components/Components/src/Routing/Router.cs

@@ -267,7 +267,7 @@ namespace Microsoft.AspNetCore.Components.Routing
             _locationAbsolute = args.Location;
             if (_renderHandle.IsInitialized && Routes != null)
             {
-                _ = RunOnNavigateAsync(NavigationManager.ToBaseRelativePath(_locationAbsolute), args.IsNavigationIntercepted);
+                _ = RunOnNavigateAsync(NavigationManager.ToBaseRelativePath(_locationAbsolute), args.IsNavigationIntercepted).Preserve();
             }
         }
 

+ 1 - 1
src/Components/Server/src/Circuits/RemoteNavigationManager.cs

@@ -75,7 +75,7 @@ namespace Microsoft.AspNetCore.Components.Server.Circuits
                 throw new NavigationException(absoluteUriString);
             }
 
-            _jsRuntime.InvokeAsync<object>(Interop.NavigateTo, uri, forceLoad);
+            _jsRuntime.InvokeAsync<object>(Interop.NavigateTo, uri, forceLoad).Preserve();
         }
 
         private static class Log

+ 1 - 1
src/Hosting/Hosting/src/Internal/WebHost.cs

@@ -334,7 +334,7 @@ namespace Microsoft.AspNetCore.Hosting
 
         public void Dispose()
         {
-            DisposeAsync().GetAwaiter().GetResult();
+            DisposeAsync().AsTask().GetAwaiter().GetResult();
         }
 
         public async ValueTask DisposeAsync()

+ 1 - 1
src/Http/Http/src/Features/RequestServicesFeature.cs

@@ -87,7 +87,7 @@ namespace Microsoft.AspNetCore.Http.Features
         /// <inheritdoc />
         public void Dispose()
         {
-            DisposeAsync().GetAwaiter().GetResult();
+            DisposeAsync().AsTask().GetAwaiter().GetResult();
         }
     }
 }

+ 15 - 22
src/Servers/Kestrel/Core/src/Internal/Http/Http1OutputProducer.cs

@@ -58,9 +58,7 @@ namespace Microsoft.AspNetCore.Server.Kestrel.Core.Internal.Http
 
         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;
@@ -118,31 +116,27 @@ namespace Microsoft.AspNetCore.Server.Kestrel.Core.Internal.Http
 
         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)
@@ -533,7 +527,6 @@ namespace Microsoft.AspNetCore.Server.Kestrel.Core.Internal.Http
             _currentMemoryPrefixBytes = 0;
             _autoChunk = false;
             _writeStreamSuffixCalled = false;
-            _writeStreamSuffixValueTask = default;
             _currentChunkMemoryUpdated = false;
             _startCalled = false;
         }

+ 11 - 1
src/Servers/Kestrel/Core/src/Internal/Http/HttpProtocol.cs

@@ -917,7 +917,15 @@ namespace Microsoft.AspNetCore.Server.Kestrel.Core.Internal.Http
                 ((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();
+                }
             }
         }
 
@@ -1050,6 +1058,8 @@ namespace Microsoft.AspNetCore.Server.Kestrel.Core.Internal.Http
                 return WriteSuffixAwaited(writeTask);
             }
 
+            writeTask.GetAwaiter().GetResult();
+
             _requestProcessingStatus = RequestProcessingStatus.ResponseCompleted;
 
             if (_keepAlive)

+ 2 - 2
src/Servers/Kestrel/Core/src/Internal/Http2/FlowControl/StreamInputFlowControl.cs

@@ -57,7 +57,7 @@ namespace Microsoft.AspNetCore.Server.Kestrel.Core.Internal.Http2.FlowControl
             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);
@@ -90,7 +90,7 @@ namespace Microsoft.AspNetCore.Server.Kestrel.Core.Internal.Http2.FlowControl
             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();
             }
         }
     }

+ 3 - 3
src/Servers/Kestrel/Core/src/Internal/Http2/Http2Connection.cs

@@ -157,7 +157,7 @@ namespace Microsoft.AspNetCore.Server.Kestrel.Core.Internal.Http2
         {
             if (TryClose())
             {
-                _frameWriter.WriteGoAwayAsync(int.MaxValue, Http2ErrorCode.INTERNAL_ERROR);
+                _frameWriter.WriteGoAwayAsync(int.MaxValue, Http2ErrorCode.INTERNAL_ERROR).Preserve();
             }
 
             _frameWriter.Abort(ex);
@@ -1214,7 +1214,7 @@ namespace Microsoft.AspNetCore.Server.Kestrel.Core.Internal.Http2
 
                 if (_gracefulCloseInitiator == GracefulCloseInitiator.Server && _clientActiveStreamCount > 0)
                 {
-                    _frameWriter.WriteGoAwayAsync(int.MaxValue, Http2ErrorCode.NO_ERROR);
+                    _frameWriter.WriteGoAwayAsync(int.MaxValue, Http2ErrorCode.NO_ERROR).Preserve();
                 }
             }
 
@@ -1224,7 +1224,7 @@ namespace Microsoft.AspNetCore.Server.Kestrel.Core.Internal.Http2
                 {
                     if (TryClose())
                     {
-                        _frameWriter.WriteGoAwayAsync(_highestOpenedStreamId, Http2ErrorCode.NO_ERROR);
+                        _frameWriter.WriteGoAwayAsync(_highestOpenedStreamId, Http2ErrorCode.NO_ERROR).Preserve();
                     }
                 }
                 else

+ 2 - 2
src/Servers/Kestrel/Core/src/Internal/Http2/Http2OutputProducer.cs

@@ -43,7 +43,7 @@ namespace Microsoft.AspNetCore.Server.Kestrel.Core.Internal.Http2
         private bool _writerComplete;
 
         // Internal for testing
-        internal ValueTask _dataWriteProcessingTask;
+        internal Task _dataWriteProcessingTask;
         internal bool _disposed;
 
         /// <summary>The core logic for the IValueTaskSource implementation.</summary>
@@ -394,7 +394,7 @@ namespace Microsoft.AspNetCore.Server.Kestrel.Core.Internal.Http2
         {
         }
 
-        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)

+ 2 - 2
src/Servers/Kestrel/Core/src/Internal/Http2/Http2Stream.cs

@@ -145,7 +145,7 @@ namespace Microsoft.AspNetCore.Server.Kestrel.Core.Internal.Http2
                         if (!errored)
                         {
                             // Don't block on IO. This never faults.
-                            _ = _http2Output.WriteRstStreamAsync(Http2ErrorCode.NO_ERROR);
+                            _ = _http2Output.WriteRstStreamAsync(Http2ErrorCode.NO_ERROR).Preserve();
                         }
                         RequestBodyPipe.Writer.Complete();
                     }
@@ -545,7 +545,7 @@ namespace Microsoft.AspNetCore.Server.Kestrel.Core.Internal.Http2
 
             DecrementActiveClientStreamCount();
             // Don't block on IO. This never faults.
-            _ = _http2Output.WriteRstStreamAsync(error);
+            _ = _http2Output.WriteRstStreamAsync(error).Preserve();
 
             AbortCore(abortReason);
         }

+ 5 - 5
src/Servers/Kestrel/Core/src/Internal/Http3/Http3Connection.cs

@@ -180,7 +180,7 @@ namespace Microsoft.AspNetCore.Server.Kestrel.Core.Internal.Http3
             {
                 if (TryClose())
                 {
-                    SendGoAway(_highestOpenedStreamId);
+                    SendGoAway(_highestOpenedStreamId).Preserve();
                 }
 
                 _errorCodeFeature.Error = (long)errorCode;
@@ -213,10 +213,10 @@ namespace Microsoft.AspNetCore.Server.Kestrel.Core.Internal.Http3
             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:
@@ -396,7 +396,7 @@ namespace Microsoft.AspNetCore.Server.Kestrel.Core.Internal.Http3
                 if (_gracefulCloseInitiator == GracefulCloseInitiator.Server && activeRequestCount > 0)
                 {
                     // Go away with largest streamid to initiate graceful shutdown.
-                    SendGoAway(VariableLengthIntegerHelper.EightByteLimit);
+                    SendGoAway(VariableLengthIntegerHelper.EightByteLimit).Preserve();
                 }
             }
 
@@ -406,7 +406,7 @@ namespace Microsoft.AspNetCore.Server.Kestrel.Core.Internal.Http3
                 {
                     if (TryClose())
                     {
-                        SendGoAway(_highestOpenedStreamId);
+                        SendGoAway(_highestOpenedStreamId).Preserve();
                     }
                 }
                 else

+ 1 - 1
src/Servers/Kestrel/Core/src/Internal/Http3/Http3OutputProducer.cs

@@ -51,7 +51,7 @@ namespace Microsoft.AspNetCore.Server.Kestrel.Core.Internal.Http3
             _pipeReader = pipe.Reader;
 
             _flusher = new TimingPipeFlusher(_pipeWriter, timeoutControl: null, log);
-            _dataWriteProcessingTask = ProcessDataWrites();
+            _dataWriteProcessingTask = ProcessDataWrites().Preserve();
         }
 
         public void Dispose()

+ 4 - 11
src/Servers/Kestrel/Transport.Quic/src/Internal/QuicConnectionContext.cs

@@ -19,7 +19,7 @@ namespace Microsoft.AspNetCore.Server.Kestrel.Transport.Experimental.Quic.Intern
         private readonly IQuicTrace _log;
         private readonly CancellationTokenSource _connectionClosedTokenSource = new CancellationTokenSource();
 
-        private ValueTask _closeTask;
+        private Task? _closeTask;
 
         public long Error { get; set; }
 
@@ -55,15 +55,8 @@ namespace Microsoft.AspNetCore.Server.Kestrel.Transport.Experimental.Quic.Intern
         {
             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)
             {
@@ -78,7 +71,7 @@ namespace Microsoft.AspNetCore.Server.Kestrel.Transport.Experimental.Quic.Intern
         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)

+ 4 - 3
src/Shared/ServerInfrastructure/DuplexPipeStream.cs

@@ -71,9 +71,10 @@ namespace Microsoft.AspNetCore.Server.Kestrel.Core.Internal
 
         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)

+ 1 - 8
src/SignalR/samples/ClientSample/Tcp/TcpConnection.cs

@@ -168,14 +168,7 @@ namespace ClientSample
 
                 _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

+ 1 - 1
src/SignalR/server/Core/src/HubConnectionContext.cs

@@ -625,7 +625,7 @@ namespace Microsoft.AspNetCore.SignalR
                 // 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