From 58d8c851c5fdeea8189529b4d23cff9869489580 Mon Sep 17 00:00:00 2001 From: Ben Adams Date: Thu, 19 Nov 2015 21:50:10 +0000 Subject: [PATCH 1/4] Remove CreateLinkedTokenSource Resolves #407 --- src/Microsoft.AspNet.Server.Kestrel/Http/Frame.cs | 12 ++++-------- 1 file changed, 4 insertions(+), 8 deletions(-) diff --git a/src/Microsoft.AspNet.Server.Kestrel/Http/Frame.cs b/src/Microsoft.AspNet.Server.Kestrel/Http/Frame.cs index 7bb0faa0e2..767ab0425f 100644 --- a/src/Microsoft.AspNet.Server.Kestrel/Http/Frame.cs +++ b/src/Microsoft.AspNet.Server.Kestrel/Http/Frame.cs @@ -43,8 +43,7 @@ namespace Microsoft.AspNet.Server.Kestrel.Http private Task _requestProcessingTask; private volatile bool _requestProcessingStopping; // volatile, see: https://msdn.microsoft.com/en-us/library/x13ttww7.aspx private volatile bool _requestAborted; - private CancellationTokenSource _disconnectCts = new CancellationTokenSource(); - private CancellationTokenSource _requestAbortCts; + private CancellationTokenSource _disconnectOrAbortedCts = new CancellationTokenSource(); private FrameRequestStream _requestBody; private FrameResponseStream _responseBody; @@ -145,8 +144,6 @@ namespace Microsoft.AspNet.Server.Kestrel.Http } _prepareRequest?.Invoke(this); - - _requestAbortCts?.Dispose(); } public void ResetResponseHeaders() @@ -199,7 +196,7 @@ namespace Microsoft.AspNet.Server.Kestrel.Http ConnectionControl.End(ProduceEndType.SocketDisconnect); SocketInput.AbortAwaiting(); - _disconnectCts.Cancel(); + _disconnectOrAbortedCts.Cancel(); } catch (Exception ex) { @@ -248,8 +245,7 @@ namespace Microsoft.AspNet.Server.Kestrel.Http ResponseBody = _responseBody; DuplexStream = new FrameDuplexStream(RequestBody, ResponseBody); - _requestAbortCts = CancellationTokenSource.CreateLinkedTokenSource(_disconnectCts.Token); - RequestAborted = _requestAbortCts.Token; + RequestAborted = _disconnectOrAbortedCts.Token; var httpContext = HttpContextFactory.Create(this); try @@ -302,7 +298,7 @@ namespace Microsoft.AspNet.Server.Kestrel.Http { try { - _disconnectCts.Dispose(); + _disconnectOrAbortedCts.Dispose(); // If _requestAborted is set, the connection has already been closed. if (!_requestAborted) From 3c7431aa354e8092571903cc3b96b0a069324a80 Mon Sep 17 00:00:00 2001 From: Ben Adams Date: Thu, 19 Nov 2015 23:25:31 +0000 Subject: [PATCH 2/4] Change disconnect to abort cts --- .../Http/Frame.cs | 18 ++++++++++++++---- 1 file changed, 14 insertions(+), 4 deletions(-) diff --git a/src/Microsoft.AspNet.Server.Kestrel/Http/Frame.cs b/src/Microsoft.AspNet.Server.Kestrel/Http/Frame.cs index 767ab0425f..8156706197 100644 --- a/src/Microsoft.AspNet.Server.Kestrel/Http/Frame.cs +++ b/src/Microsoft.AspNet.Server.Kestrel/Http/Frame.cs @@ -43,7 +43,7 @@ namespace Microsoft.AspNet.Server.Kestrel.Http private Task _requestProcessingTask; private volatile bool _requestProcessingStopping; // volatile, see: https://msdn.microsoft.com/en-us/library/x13ttww7.aspx private volatile bool _requestAborted; - private CancellationTokenSource _disconnectOrAbortedCts = new CancellationTokenSource(); + private CancellationTokenSource _abortedCts; private FrameRequestStream _requestBody; private FrameResponseStream _responseBody; @@ -144,6 +144,9 @@ namespace Microsoft.AspNet.Server.Kestrel.Http } _prepareRequest?.Invoke(this); + + _abortedCts?.Dispose(); + _abortedCts = null; } public void ResetResponseHeaders() @@ -196,12 +199,17 @@ namespace Microsoft.AspNet.Server.Kestrel.Http ConnectionControl.End(ProduceEndType.SocketDisconnect); SocketInput.AbortAwaiting(); - _disconnectOrAbortedCts.Cancel(); + _abortedCts?.Cancel(); } catch (Exception ex) { Log.LogError("Abort", ex); } + finally + { + _abortedCts?.Dispose(); + _abortedCts = null; + } } /// @@ -245,7 +253,8 @@ namespace Microsoft.AspNet.Server.Kestrel.Http ResponseBody = _responseBody; DuplexStream = new FrameDuplexStream(RequestBody, ResponseBody); - RequestAborted = _disconnectOrAbortedCts.Token; + _abortedCts = new CancellationTokenSource(); + RequestAborted = _abortedCts.Token; var httpContext = HttpContextFactory.Create(this); try @@ -298,7 +307,8 @@ namespace Microsoft.AspNet.Server.Kestrel.Http { try { - _disconnectOrAbortedCts.Dispose(); + _abortedCts?.Dispose(); + _abortedCts = null; // If _requestAborted is set, the connection has already been closed. if (!_requestAborted) From 174ec739bbe770b41b265f947abf29315f7a5921 Mon Sep 17 00:00:00 2001 From: Ben Adams Date: Fri, 20 Nov 2015 00:30:08 +0000 Subject: [PATCH 3/4] Don't log ODEs thrown from _abortedCts.Cancel --- src/Microsoft.AspNet.Server.Kestrel/Http/Frame.cs | 10 +++++++++- 1 file changed, 9 insertions(+), 1 deletion(-) diff --git a/src/Microsoft.AspNet.Server.Kestrel/Http/Frame.cs b/src/Microsoft.AspNet.Server.Kestrel/Http/Frame.cs index 8156706197..3922f4d9d7 100644 --- a/src/Microsoft.AspNet.Server.Kestrel/Http/Frame.cs +++ b/src/Microsoft.AspNet.Server.Kestrel/Http/Frame.cs @@ -199,7 +199,15 @@ namespace Microsoft.AspNet.Server.Kestrel.Http ConnectionControl.End(ProduceEndType.SocketDisconnect); SocketInput.AbortAwaiting(); - _abortedCts?.Cancel(); + try + { + _abortedCts?.Cancel(); + } + catch (ObjectDisposedException) + { + // Don't log ODEs thrown from _abortedCts.Cancel() + // If _abortedCts is disposed, the app has already completed. + } } catch (Exception ex) { From c7d7f0e57514b990e1dc04dd794912c2c79965dd Mon Sep 17 00:00:00 2001 From: stephentoub Date: Sat, 21 Nov 2015 08:38:48 -0500 Subject: [PATCH 4/4] Lazily allocate the RequestAborted CTS Avoid allocating the CancellationTokenSource unless it's actually requested. This makes it pay-for-play with regards to code that actually asks for the RequestAborted token and requests that are aborted. --- .../Http/Frame.cs | 61 ++++++++++++++----- 1 file changed, 45 insertions(+), 16 deletions(-) diff --git a/src/Microsoft.AspNet.Server.Kestrel/Http/Frame.cs b/src/Microsoft.AspNet.Server.Kestrel/Http/Frame.cs index 3922f4d9d7..4d0b19af45 100644 --- a/src/Microsoft.AspNet.Server.Kestrel/Http/Frame.cs +++ b/src/Microsoft.AspNet.Server.Kestrel/Http/Frame.cs @@ -44,6 +44,7 @@ namespace Microsoft.AspNet.Server.Kestrel.Http private volatile bool _requestProcessingStopping; // volatile, see: https://msdn.microsoft.com/en-us/library/x13ttww7.aspx private volatile bool _requestAborted; private CancellationTokenSource _abortedCts; + private CancellationToken? _manuallySetRequestAbortToken; private FrameRequestStream _requestBody; private FrameResponseStream _responseBody; @@ -92,8 +93,47 @@ namespace Microsoft.AspNet.Server.Kestrel.Http public Stream DuplexStream { get; set; } - public CancellationToken RequestAborted { get; set; } + public CancellationToken RequestAborted + { + get + { + // If a request abort token was previously explicitly set, return it. + if (_manuallySetRequestAbortToken.HasValue) + return _manuallySetRequestAbortToken.Value; + // Otherwise, get the abort CTS. If we have one, which would mean that someone previously + // asked for the RequestAborted token, simply return its token. If we don't, + // check to see whether we've already aborted, in which case just return an + // already canceled token. Finally, force a source into existence if we still + // don't have one, and return its token. + var cts = _abortedCts; + return + cts != null ? cts.Token : + _requestAborted ? new CancellationToken(true) : + RequestAbortedSource.Token; + } + set + { + // Set an abort token, overriding one we create internally. This setter and associated + // field exist purely to support IHttpRequestLifetimeFeature.set_RequestAborted. + _manuallySetRequestAbortToken = value; + } + } + + private CancellationTokenSource RequestAbortedSource + { + get + { + // Get the abort token, lazily-initializing it if necessary. + // Make sure it's canceled if an abort request already came in. + var cts = LazyInitializer.EnsureInitialized(ref _abortedCts, () => new CancellationTokenSource()); + if (_requestAborted) + { + cts.Cancel(); + } + return cts; + } + } public bool HasResponseStarted { get { return _responseStarted; } @@ -145,7 +185,7 @@ namespace Microsoft.AspNet.Server.Kestrel.Http _prepareRequest?.Invoke(this); - _abortedCts?.Dispose(); + _manuallySetRequestAbortToken = null; _abortedCts = null; } @@ -198,16 +238,7 @@ namespace Microsoft.AspNet.Server.Kestrel.Http { ConnectionControl.End(ProduceEndType.SocketDisconnect); SocketInput.AbortAwaiting(); - - try - { - _abortedCts?.Cancel(); - } - catch (ObjectDisposedException) - { - // Don't log ODEs thrown from _abortedCts.Cancel() - // If _abortedCts is disposed, the app has already completed. - } + RequestAbortedSource.Cancel(); } catch (Exception ex) { @@ -215,7 +246,6 @@ namespace Microsoft.AspNet.Server.Kestrel.Http } finally { - _abortedCts?.Dispose(); _abortedCts = null; } } @@ -261,8 +291,8 @@ namespace Microsoft.AspNet.Server.Kestrel.Http ResponseBody = _responseBody; DuplexStream = new FrameDuplexStream(RequestBody, ResponseBody); - _abortedCts = new CancellationTokenSource(); - RequestAborted = _abortedCts.Token; + _abortedCts = null; + _manuallySetRequestAbortToken = null; var httpContext = HttpContextFactory.Create(this); try @@ -315,7 +345,6 @@ namespace Microsoft.AspNet.Server.Kestrel.Http { try { - _abortedCts?.Dispose(); _abortedCts = null; // If _requestAborted is set, the connection has already been closed.