Use object indirection in HttpContextAccessor (#1066)

Author: benaadams

src/Microsoft.AspNetCore.Http/HttpContextAccessor.cs

@@ -7,21 +7,35 @@ namespace Microsoft.AspNetCore.Http
 {
     public class HttpContextAccessor : IHttpContextAccessor
     {
-        private static AsyncLocal<(string traceIdentifier, HttpContext context)> _httpContextCurrent = new AsyncLocal<(string traceIdentifier, HttpContext context)>();
+        private static AsyncLocal<HttpContextHolder> _httpContextCurrent = new AsyncLocal<HttpContextHolder>();
 
         public HttpContext HttpContext
         {
             get
             {
-                var value = _httpContextCurrent.Value;
-                // Only return the context if the stored request id matches the stored trace identifier
-                // context.TraceIdentifier is cleared by HttpContextFactory.Dispose.
-                return value.traceIdentifier == value.context?.TraceIdentifier ? value.context : null;
+                return  _httpContextCurrent.Value?.Context;
             }
             set
             {
-                _httpContextCurrent.Value = (value?.TraceIdentifier, value);
+                var holder = _httpContextCurrent.Value;
+                if (holder != null)
+                {
+                    // Clear current HttpContext trapped in the AsyncLocals, as its done.
+                    holder.Context = null;
+                }
+
+                if (value != null)
+                {
+                    // Use an object indirection to hold the HttpContext in the AsyncLocal,
+                    // so it can be cleared in all ExecutionContexts when its cleared.
+                    _httpContextCurrent.Value = new HttpContextHolder { Context = value };
+                }
             }
         }
+
+        private class HttpContextHolder
+        {
+            public HttpContext Context;
+        }
     }
 }

src/Microsoft.AspNetCore.Http/HttpContextFactory.cs

@@ -53,10 +53,6 @@ public void Dispose(HttpContext httpContext)
             {
                 _httpContextAccessor.HttpContext = null;
             }
-
-            // Null out the TraceIdentifier here as a sign that this request is done,
-            // the HttpContextAccessor implementation relies on this to detect that the request is over 
-            httpContext.TraceIdentifier = null;
         }
     }
 }

test/Microsoft.AspNetCore.Http.Tests/HttpContextAccessorTests.cs

@@ -44,7 +44,6 @@ public async Task HttpContextAccessor_GettingHttpContextReturnsNullHttpContextIf
             var accessor = new HttpContextAccessor();
 
             var context = new DefaultHttpContext();
-            context.TraceIdentifier = "1";
             accessor.HttpContext = context;
 
             var checkAsyncFlowTcs = new TaskCompletionSource<object>(TaskCreationOptions.RunContinuationsAsynchronously);
@@ -76,7 +75,6 @@ public async Task HttpContextAccessor_GettingHttpContextReturnsNullHttpContextIf
 
             // Null out the accessor
             accessor.HttpContext = null;
-            context.TraceIdentifier = null;
 
             waitForNullTcs.SetResult(null);
 
@@ -86,12 +84,11 @@ public async Task HttpContextAccessor_GettingHttpContextReturnsNullHttpContextIf
         }
 
         [Fact]
-        public async Task HttpContextAccessor_GettingHttpContextReturnsNullHttpContextIfDifferentTraceIdentifier()
+        public async Task HttpContextAccessor_GettingHttpContextReturnsNullHttpContextIfChanged()
         {
             var accessor = new HttpContextAccessor();
 
             var context = new DefaultHttpContext();
-            context.TraceIdentifier = "1";
             accessor.HttpContext = context;
 
             var checkAsyncFlowTcs = new TaskCompletionSource<object>(TaskCreationOptions.RunContinuationsAsynchronously);
@@ -121,12 +118,8 @@ public async Task HttpContextAccessor_GettingHttpContextReturnsNullHttpContextIf
 
             await checkAsyncFlowTcs.Task;
 
-            // Reset the trace identifier on the first request
-            context.TraceIdentifier = null;
-
             // Set a new http context
             var context2 = new DefaultHttpContext();
-            context2.TraceIdentifier = "2";
             accessor.HttpContext = context2;
 
             waitForNullTcs.SetResult(null);
@@ -142,7 +135,6 @@ public async Task HttpContextAccessor_GettingHttpContextDoesNotFlowIfAccessorSet
             var accessor = new HttpContextAccessor();
 
             var context = new DefaultHttpContext();
-            context.TraceIdentifier = "1";
             accessor.HttpContext = context;
 
             var checkAsyncFlowTcs = new TaskCompletionSource<object>(TaskCreationOptions.RunContinuationsAsynchronously);
@@ -172,7 +164,6 @@ public async Task HttpContextAccessor_GettingHttpContextDoesNotFlowIfExecutionCo
             var accessor = new HttpContextAccessor();
 
             var context = new DefaultHttpContext();
-            context.TraceIdentifier = "1";
             accessor.HttpContext = context;
 
             var checkAsyncFlowTcs = new TaskCompletionSource<object>(TaskCreationOptions.RunContinuationsAsynchronously);

test/Microsoft.AspNetCore.Http.Tests/HttpContextFactoryTests.cs

@@ -34,15 +34,13 @@ public void DisposeHttpContextSetsHttpContextAccessorToNull()
 
             // Act
             var context = contextFactory.Create(new FeatureCollection());
-            var traceIdentifier = context.TraceIdentifier;
 
             // Assert
             Assert.Same(context, accessor.HttpContext);
 
             contextFactory.Dispose(context);
 
             Assert.Null(accessor.HttpContext);
-            Assert.NotEqual(traceIdentifier, context.TraceIdentifier);
         }
 
         [Fact]