From 1d842a17f4735f17c6b30955924f4b45bf8ba84f Mon Sep 17 00:00:00 2001 From: David Fowler Date: Tue, 17 Jan 2023 15:05:01 -0800 Subject: [PATCH 1/3] Suppress the execution context --- .../src/Internal/HttpConnectionContext.cs | 22 +++++++++++++++---- 1 file changed, 18 insertions(+), 4 deletions(-) diff --git a/src/SignalR/common/Http.Connections/src/Internal/HttpConnectionContext.cs b/src/SignalR/common/Http.Connections/src/Internal/HttpConnectionContext.cs index 5af5f316def0..948a572bffdf 100644 --- a/src/SignalR/common/Http.Connections/src/Internal/HttpConnectionContext.cs +++ b/src/SignalR/common/Http.Connections/src/Internal/HttpConnectionContext.cs @@ -42,6 +42,7 @@ internal sealed partial class HttpConnectionContext : ConnectionContext, private IDictionary? _items; private readonly CancellationTokenSource _connectionClosedTokenSource; private readonly CancellationTokenSource _connectionCloseRequested; + private ConnectionDelegate? _connectionDelegate; private CancellationTokenSource? _sendCts; private bool _activeSend; @@ -546,17 +547,30 @@ public void MarkInactive() } } + private Task ExecuteApplication() => _connectionDelegate!(this); + private async Task ExecuteApplication(ConnectionDelegate connectionDelegate) { // Verify some initialization invariants Debug.Assert(TransportType != HttpTransportType.None, "Transport has not been initialized yet"); - // Jump onto the thread pool thread so blocking user code doesn't block the setup of the + // - Jump onto the thread pool thread so blocking user code doesn't block the setup of the // connection and transport - await Task.Yield(); + // - Discard the current ExecutionContext and SynchronizationContext if any + + // Set the connection delegate here so we can avoid the closure allocation in Task.Run. + // We're still paying for the delegate allocation but this happens once per connection so there's no point caching. + _connectionDelegate = connectionDelegate; + + if (!ExecutionContext.IsFlowSuppressed()) + { + ExecutionContext.SuppressFlow(); + } + + await Task.Run(ExecuteApplication); - // Running this in an async method turns sync exceptions into async ones - await connectionDelegate(this); + // No need to undo the flow suppression since we're in an async method. + // The suppression won't be observed outside of this call. } internal void StartSendCancellation() From 90d2579e910f28054030c5b5324f46a828eadf87 Mon Sep 17 00:00:00 2001 From: David Fowler Date: Wed, 18 Jan 2023 22:07:50 -0800 Subject: [PATCH 2/3] Preserve the connection id in the new execution context --- .../src/Internal/HttpConnectionContext.cs | 30 +++++++++++++++++-- 1 file changed, 28 insertions(+), 2 deletions(-) diff --git a/src/SignalR/common/Http.Connections/src/Internal/HttpConnectionContext.cs b/src/SignalR/common/Http.Connections/src/Internal/HttpConnectionContext.cs index 948a572bffdf..0c8845bb5bfa 100644 --- a/src/SignalR/common/Http.Connections/src/Internal/HttpConnectionContext.cs +++ b/src/SignalR/common/Http.Connections/src/Internal/HttpConnectionContext.cs @@ -547,8 +547,6 @@ public void MarkInactive() } } - private Task ExecuteApplication() => _connectionDelegate!(this); - private async Task ExecuteApplication(ConnectionDelegate connectionDelegate) { // Verify some initialization invariants @@ -573,6 +571,34 @@ private async Task ExecuteApplication(ConnectionDelegate connectionDelegate) // The suppression won't be observed outside of this call. } + private async Task ExecuteApplication() + { + var connectionDelegate = _connectionDelegate!; + + Debug.Assert(connectionDelegate is not null, "Connection delegate was not set!"); + + // Clear the field + _connectionDelegate = null; + + if (_logger.IsEnabled(LogLevel.Critical)) + { + // Preserve the connection id logging scope when we execute the connection delegate + var logScope = new ConnectionLogScope(ConnectionId); + + // REVIEW: Should we create also create an activity here + + using (_logger.BeginScope(logScope)) + { + await connectionDelegate(this); + } + } + else + { + await connectionDelegate(this); + } + } + + internal void StartSendCancellation() { if (!_options.TransportSendTimeoutEnabled) From 12a2403cea1dfc820aaaa2d3df9affad40581b0a Mon Sep 17 00:00:00 2001 From: David Fowler Date: Wed, 18 Jan 2023 23:37:24 -0800 Subject: [PATCH 3/3] Fix whitespace --- .../Http.Connections/src/Internal/HttpConnectionContext.cs | 1 - 1 file changed, 1 deletion(-) diff --git a/src/SignalR/common/Http.Connections/src/Internal/HttpConnectionContext.cs b/src/SignalR/common/Http.Connections/src/Internal/HttpConnectionContext.cs index 0c8845bb5bfa..7ab982f5cabd 100644 --- a/src/SignalR/common/Http.Connections/src/Internal/HttpConnectionContext.cs +++ b/src/SignalR/common/Http.Connections/src/Internal/HttpConnectionContext.cs @@ -598,7 +598,6 @@ private async Task ExecuteApplication() } } - internal void StartSendCancellation() { if (!_options.TransportSendTimeoutEnabled)