Call OnStarting and OnCompleted callbacks in LIFO order (#1042).

aspnet · 08a91f17bfd593fcaaac486846b5bdb1da06e86c · cesarbs committed Aug 10, 2016 · Aug 10, 2016 · 08a91f1
1 parent 5181e41
commit 08a91f17bfd593fcaaac486846b5bdb1da06e86c
Unified Split

Showing with 103 additions and 12 deletions.

+8 −9 src/Microsoft.AspNetCore.Server.Kestrel/Internal/Http/Frame.cs

+95 −3 test/Microsoft.AspNetCore.Server.KestrelTests/EngineTests.cs
@@ -47,9 +47,8 @@ public abstract partial class Frame : ConnectionContext, IFrameControl
         protected bool _requestRejected;
         private Streams _frameStreams;
 
-        protected List<KeyValuePair<Func<object, Task>, object>> _onStarting;
-
-        protected List<KeyValuePair<Func<object, Task>, object>> _onCompleted;
+        protected Stack<KeyValuePair<Func<object, Task>, object>> _onStarting;
+        protected Stack<KeyValuePair<Func<object, Task>, object>> _onCompleted;
 
         private Task _requestProcessingTask;
         protected volatile bool _requestProcessingStopping; // volatile, see: https://msdn.microsoft.com/en-us/library/x13ttww7.aspx
@@ -395,9 +394,9 @@ public void OnStarting(Func<object, Task> callback, object state)
 
                 if (_onStarting == null)
                 {
-                    _onStarting = new List<KeyValuePair<Func<object, Task>, object>>();
+                    _onStarting = new Stack<KeyValuePair<Func<object, Task>, object>>();
                 }
-                _onStarting.Add(new KeyValuePair<Func<object, Task>, object>(callback, state));
+                _onStarting.Push(new KeyValuePair<Func<object, Task>, object>(callback, state));
             }
         }
 
@@ -407,15 +406,15 @@ public void OnCompleted(Func<object, Task> callback, object state)
             {
                 if (_onCompleted == null)
                 {
-                    _onCompleted = new List<KeyValuePair<Func<object, Task>, object>>();
+                    _onCompleted = new Stack<KeyValuePair<Func<object, Task>, object>>();
                 }
-                _onCompleted.Add(new KeyValuePair<Func<object, Task>, object>(callback, state));
+                _onCompleted.Push(new KeyValuePair<Func<object, Task>, object>(callback, state));
             }
         }
 
         protected async Task FireOnStarting()
         {
-            List<KeyValuePair<Func<object, Task>, object>> onStarting = null;
+            Stack<KeyValuePair<Func<object, Task>, object>> onStarting = null;
             lock (_onStartingSync)
             {
                 onStarting = _onStarting;
@@ -439,7 +438,7 @@ public void OnCompleted(Func<object, Task> callback, object state)
 
         protected async Task FireOnCompleted()
         {
-            List<KeyValuePair<Func<object, Task>, object>> onCompleted = null;
+            Stack<KeyValuePair<Func<object, Task>, object>> onCompleted = null;
             lock (_onCompletedSync)
             {
                 onCompleted = _onCompleted;
@@ -2,6 +2,7 @@
 // Licensed under the Apache License, Version 2.0. See License.txt in the project root for license information.
 
 using System;
+using System.Collections.Generic;
 using System.IO;
 using System.Linq;
 using System.Net.Sockets;
@@ -833,9 +834,12 @@ public void ConnectionCanReadAndWrite(TestServiceContext testContext)
                         "",
                         "");
 
-                    Assert.Equal(2, onStartingCallCount1);
-                    // The second OnStarting callback should not be called since the first failed.
-                    Assert.Equal(0, onStartingCallCount2);
+                    Assert.Equal(2, onStartingCallCount2);
+
+                    // The first registered OnStarting callback should not be called,
+                    // since they are called LIFO and the other one failed.
+                    Assert.Equal(0, onStartingCallCount1);
+
                     Assert.Equal(2, testLogger.ApplicationErrorsLogged);
                 }
             }
@@ -1199,5 +1203,93 @@ public void ConnectionCanReadAndWrite(TestServiceContext testContext)
                 }
             }
         }
+
+        [Theory]
+        [MemberData(nameof(ConnectionFilterData))]
+        public async Task OnStartingCallbacksAreCalledInLastInFirstOutOrder(TestServiceContext testContext)
+        {
+            const string response = "hello, world";
+
+            var callOrder = new Stack<int>();
+
+            using (var server = new TestServer(async context =>
+            {
+                context.Response.OnStarting(_ =>
+                {
+                    callOrder.Push(1);
+                    return TaskUtilities.CompletedTask;
+                }, null);
+                context.Response.OnStarting(_ =>
+                {
+                    callOrder.Push(2);
+                    return TaskUtilities.CompletedTask;
+                }, null);
+                
+                context.Response.ContentLength = response.Length;
+                await context.Response.WriteAsync(response);
+            }, testContext))
+            {
+                using (var connection = server.CreateConnection())
+                {
+                    await connection.SendEnd(
+                        "GET / HTTP/1.1",
+                        "",
+                        "");
+                    await connection.ReceiveEnd(
+                        "HTTP/1.1 200 OK",
+                        $"Date: {testContext.DateHeaderValue}",
+                        $"Content-Length: {response.Length}",
+                        "",
+                        "hello, world");
+
+                    Assert.Equal(1, callOrder.Pop());
+                    Assert.Equal(2, callOrder.Pop());
+                }
+            }
+        }
+
+        [Theory]
+        [MemberData(nameof(ConnectionFilterData))]
+        public async Task OnCompletedCallbacksAreCalledInLastInFirstOutOrder(TestServiceContext testContext)
+        {
+            const string response = "hello, world";
+
+            var callOrder = new Stack<int>();
+
+            using (var server = new TestServer(async context =>
+            {
+                context.Response.OnCompleted(_ =>
+                {
+                    callOrder.Push(1);
+                    return TaskUtilities.CompletedTask;
+                }, null);
+                context.Response.OnCompleted(_ =>
+                {
+                    callOrder.Push(2);
+                    return TaskUtilities.CompletedTask;
+                }, null);
+
+                context.Response.ContentLength = response.Length;
+                await context.Response.WriteAsync(response);
+            }, testContext))
+            {
+                using (var connection = server.CreateConnection())
+                {
+                    await connection.SendEnd(
+                        "GET / HTTP/1.1",
+                        "",
+                        "");
+                    await connection.ReceiveEnd(
+                        "HTTP/1.1 200 OK",
+                        $"Date: {testContext.DateHeaderValue}",
+                        $"Content-Length: {response.Length}",
+                        "",
+                        "hello, world");
+
+                    Assert.Equal(1, callOrder.Pop());
+                    Assert.Equal(2, callOrder.Pop());
+                }
+            }
+        }
     }
 }