better

Author: BrennanConroy

src/Servers/Kestrel/Core/src/Internal/Http2/Http2FrameWriter.cs

@@ -370,17 +370,31 @@ public void UpdateMaxFrameSize(uint maxFrameSize)
         }
     }
 
+    /// <summary>
+    /// Call while in the <c>_writeLock</c>.
+    /// </summary>
+    /// <returns><c>true</c> if already completed.</returns>
+    private bool CompleteUnsynchronized()
+    {
+        if (_completed)
+        {
+            return true;
+        }
+
+        _completed = true;
+        _outputWriter.Abort();
+
+        return false;
+    }
+
     public void Complete()
     {
         lock (_writeLock)
         {
-            if (_completed)
+            if (CompleteUnsynchronized())
             {
                 return;
             }
-
-            _completed = true;
-            _outputWriter.Abort();
         }
 
         // Ok to call after aborting the Pipe because we've already set _completed to true which means any writes from the abort call
@@ -407,7 +421,10 @@ public void Abort(ConnectionAbortedException error)
             _aborted = true;
             _connectionContext.Abort(error);
 
-            Complete();
+            if (CompleteUnsynchronized())
+            {
+                return;
+            }
         }
     }
 

src/Servers/Kestrel/Core/src/Internal/Http2/Http2OutputProducer.cs

@@ -590,6 +590,7 @@ public void Reset()
 
     internal void OnRequestProcessingEnded()
     {
+        var shouldCompleteStream = false;
         lock (_dataWriterLock)
         {
             if (_requestProcessingComplete)
@@ -600,15 +601,22 @@ internal void OnRequestProcessingEnded()
 
             _requestProcessingComplete = true;
 
-            if (_completedResponse)
-            {
-                Stream.CompleteStream(errored: false);
-            }
+            shouldCompleteStream = _completedResponse;
+        }
+
+        // Complete outside of lock, anything this method does that needs a lock will acquire a lock itself.
+        if (shouldCompleteStream)
+        {
+            Stream.CompleteStream(errored: false);
         }
+
     }
 
     internal ValueTask<FlushResult> CompleteResponseAsync()
     {
+        var shouldCompleteStream = false;
+        ValueTask<FlushResult> task = default;
+
         lock (_dataWriterLock)
         {
             if (_completedResponse)
@@ -619,22 +627,23 @@ internal ValueTask<FlushResult> CompleteResponseAsync()
 
             _completedResponse = true;
 
-            ValueTask<FlushResult> task = default;
-
             if (_resetErrorCode is { } error)
             {
                 // If we have an error code to write, write it now that we're done with the response.
                 // Always send the reset even if the response body is completed. The request body may not have completed yet.
                 task = _frameWriter.WriteRstStreamAsync(StreamId, error);
             }
 
-            if (_requestProcessingComplete)
-            {
-                Stream.CompleteStream(errored: false);
-            }
+            shouldCompleteStream = _requestProcessingComplete;
+        }
 
-            return task;
+        // Complete outside of lock, anything this method does that needs a lock will acquire a lock itself.
+        if (shouldCompleteStream)
+        {
+            Stream.CompleteStream(errored: false);
         }
+
+        return task;
     }
 
     internal Memory<byte> GetFakeMemory(int minSize)