From 49d785c9343d64b93fadfb7eea36c358825cccb0 Mon Sep 17 00:00:00 2001 From: Ben Adams Date: Fri, 16 Nov 2018 06:34:44 +0000 Subject: [PATCH] Use object indirection in HttpContextAccessor (#1066) --- .../HttpContextAccessor.cs | 26 ++++++++++++++----- .../HttpContextFactory.cs | 4 --- .../HttpContextAccessorTests.cs | 11 +------- .../HttpContextFactoryTests.cs | 2 -- 4 files changed, 21 insertions(+), 22 deletions(-) diff --git a/src/Microsoft.AspNetCore.Http/HttpContextAccessor.cs b/src/Microsoft.AspNetCore.Http/HttpContextAccessor.cs index 897c27f734..286151029c 100644 --- a/src/Microsoft.AspNetCore.Http/HttpContextAccessor.cs +++ b/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 _httpContextCurrent = new AsyncLocal(); 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; + } } } diff --git a/src/Microsoft.AspNetCore.Http/HttpContextFactory.cs b/src/Microsoft.AspNetCore.Http/HttpContextFactory.cs index f293ef4782..8236a388a5 100644 --- a/src/Microsoft.AspNetCore.Http/HttpContextFactory.cs +++ b/src/Microsoft.AspNetCore.Http/HttpContextFactory.cs @@ -53,10 +53,6 @@ namespace Microsoft.AspNetCore.Http { _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; } } } \ No newline at end of file diff --git a/test/Microsoft.AspNetCore.Http.Tests/HttpContextAccessorTests.cs b/test/Microsoft.AspNetCore.Http.Tests/HttpContextAccessorTests.cs index c1521b1bc3..c224a66a7d 100644 --- a/test/Microsoft.AspNetCore.Http.Tests/HttpContextAccessorTests.cs +++ b/test/Microsoft.AspNetCore.Http.Tests/HttpContextAccessorTests.cs @@ -44,7 +44,6 @@ namespace Microsoft.AspNetCore.Http var accessor = new HttpContextAccessor(); var context = new DefaultHttpContext(); - context.TraceIdentifier = "1"; accessor.HttpContext = context; var checkAsyncFlowTcs = new TaskCompletionSource(TaskCreationOptions.RunContinuationsAsynchronously); @@ -76,7 +75,6 @@ namespace Microsoft.AspNetCore.Http // Null out the accessor accessor.HttpContext = null; - context.TraceIdentifier = null; waitForNullTcs.SetResult(null); @@ -86,12 +84,11 @@ namespace Microsoft.AspNetCore.Http } [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(TaskCreationOptions.RunContinuationsAsynchronously); @@ -121,12 +118,8 @@ namespace Microsoft.AspNetCore.Http 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 @@ namespace Microsoft.AspNetCore.Http var accessor = new HttpContextAccessor(); var context = new DefaultHttpContext(); - context.TraceIdentifier = "1"; accessor.HttpContext = context; var checkAsyncFlowTcs = new TaskCompletionSource(TaskCreationOptions.RunContinuationsAsynchronously); @@ -172,7 +164,6 @@ namespace Microsoft.AspNetCore.Http var accessor = new HttpContextAccessor(); var context = new DefaultHttpContext(); - context.TraceIdentifier = "1"; accessor.HttpContext = context; var checkAsyncFlowTcs = new TaskCompletionSource(TaskCreationOptions.RunContinuationsAsynchronously); diff --git a/test/Microsoft.AspNetCore.Http.Tests/HttpContextFactoryTests.cs b/test/Microsoft.AspNetCore.Http.Tests/HttpContextFactoryTests.cs index 80e421273a..56b996f5be 100644 --- a/test/Microsoft.AspNetCore.Http.Tests/HttpContextFactoryTests.cs +++ b/test/Microsoft.AspNetCore.Http.Tests/HttpContextFactoryTests.cs @@ -34,7 +34,6 @@ namespace Microsoft.AspNetCore.Http // Act var context = contextFactory.Create(new FeatureCollection()); - var traceIdentifier = context.TraceIdentifier; // Assert Assert.Same(context, accessor.HttpContext); @@ -42,7 +41,6 @@ namespace Microsoft.AspNetCore.Http contextFactory.Dispose(context); Assert.Null(accessor.HttpContext); - Assert.NotEqual(traceIdentifier, context.TraceIdentifier); } [Fact]