From adb9d6a05307219955a7bcdc0d1e39f318fe91d8 Mon Sep 17 00:00:00 2001 From: Ian Cooper Date: Tue, 31 Dec 2024 15:23:14 +0000 Subject: [PATCH] fix: try executing on the thread pool --- .../Tasks/BrighterSynchronizationContext.cs | 42 ++++++--------- src/Paramore.Brighter/Tasks/ContextMessage.cs | 4 +- .../BrighterSynchronizationContextsTests.cs | 53 ++++++++++++++----- 3 files changed, 57 insertions(+), 42 deletions(-) diff --git a/src/Paramore.Brighter/Tasks/BrighterSynchronizationContext.cs b/src/Paramore.Brighter/Tasks/BrighterSynchronizationContext.cs index 502c1dedfd..02d2a410f1 100644 --- a/src/Paramore.Brighter/Tasks/BrighterSynchronizationContext.cs +++ b/src/Paramore.Brighter/Tasks/BrighterSynchronizationContext.cs @@ -33,10 +33,8 @@ namespace Paramore.Brighter.Tasks /// Only uses one thread, so predictable performance, but may have many messages queued. Once queue length exceeds /// buffer size, we will stop reading new work. /// - internal class BrighterSynchronizationContext : SynchronizationContext + public class BrighterSynchronizationContext : SynchronizationContext { - private readonly ExecutionContext? _executionContext; - /// /// Gets the synchronization helper. /// @@ -62,7 +60,6 @@ internal class BrighterSynchronizationContext : SynchronizationContext public BrighterSynchronizationContext(BrighterSynchronizationHelper synchronizationHelper) { SynchronizationHelper = synchronizationHelper; - _executionContext = ExecutionContext.Capture(); } /// @@ -132,8 +129,7 @@ public override void Post(SendOrPostCallback callback, object? state) Debug.IndentLevel = 0; if (callback == null) throw new ArgumentNullException(nameof(callback)); - var ctxt = ExecutionContext.Capture(); - bool queued = SynchronizationHelper.Enqueue(new ContextMessage(callback, state, ctxt), true); + bool queued = SynchronizationHelper.Enqueue(new ContextMessage(callback, state), true); if (queued) return; @@ -143,26 +139,19 @@ public override void Post(SendOrPostCallback callback, object? state) // If the execution context can help, we might be able to redirect; if not just run immediately on this thread var contextCallback = new ContextCallback(callback); - if (ctxt != null && ctxt != _executionContext) - { - Debug.WriteLine(string.Empty); - Debug.IndentLevel = 1; - Debug.WriteLine($"BrighterSynchronizationContext: Post Failed to queue {callback.Method.Name} on thread {Thread.CurrentThread.ManagedThreadId}"); - Debug.WriteLine($"BrighterSynchronizationContext: Parent Task {ParentTaskId}"); - Debug.IndentLevel = 0; - SynchronizationHelper.ExecuteOnContext(ctxt, contextCallback, state); - } - else - { - Debug.WriteLine(string.Empty); - Debug.IndentLevel = 1; - Debug.WriteLine($"BrighterSynchronizationContext: Post Failed to queue {callback.Method.Name} on thread {Thread.CurrentThread.ManagedThreadId}"); - Debug.WriteLine($"BrighterSynchronizationContext: Parent Task {ParentTaskId}"); - Debug.IndentLevel = 0; - //just execute inline - SynchronizationHelper.ExecuteImmediately(contextCallback, state); - } + Debug.WriteLine(string.Empty); + Debug.IndentLevel = 1; + Debug.WriteLine($"BrighterSynchronizationContext: Post Failed to queue {callback.Method.Name} on thread {Thread.CurrentThread.ManagedThreadId}"); + Debug.WriteLine($"BrighterSynchronizationContext: Parent Task {ParentTaskId}"); + Debug.IndentLevel = 0; + + //just execute inline + // current thread already owns the context, so just execute inline to prevent deadlocks + //if (BrighterSynchronizationHelper.Current == SynchronizationHelper) + //SynchronizationHelper.ExecuteImmediately(contextCallback, state); + //else + base.Post(callback, state); } @@ -186,8 +175,7 @@ public override void Send(SendOrPostCallback callback, object? state) } else { - var ctxt = ExecutionContext.Capture(); - var task = SynchronizationHelper.MakeTask(new ContextMessage(callback, state, ctxt)); + var task = SynchronizationHelper.MakeTask(new ContextMessage(callback, state)); if (!task.Wait(Timeout)) // Timeout mechanism throw new TimeoutException("BrighterSynchronizationContext: Send operation timed out."); } diff --git a/src/Paramore.Brighter/Tasks/ContextMessage.cs b/src/Paramore.Brighter/Tasks/ContextMessage.cs index 038648bdff..0804f431fa 100644 --- a/src/Paramore.Brighter/Tasks/ContextMessage.cs +++ b/src/Paramore.Brighter/Tasks/ContextMessage.cs @@ -16,11 +16,9 @@ public struct ContextMessage /// /// The callback to execute. /// The state to pass to the callback. - /// The execution context, mainly intended for debugging purposes - public ContextMessage(SendOrPostCallback callback, object? state, ExecutionContext? ctxt) + public ContextMessage(SendOrPostCallback callback, object? state) { Callback = callback; State = state; - Context = ctxt; } } diff --git a/tests/Paramore.Brighter.Core.Tests/Tasks/BrighterSynchronizationContextsTests.cs b/tests/Paramore.Brighter.Core.Tests/Tasks/BrighterSynchronizationContextsTests.cs index c102564b61..20efe5a9c5 100644 --- a/tests/Paramore.Brighter.Core.Tests/Tasks/BrighterSynchronizationContextsTests.cs +++ b/tests/Paramore.Brighter.Core.Tests/Tasks/BrighterSynchronizationContextsTests.cs @@ -151,26 +151,26 @@ public void Current_WithoutAsyncContext_IsNull() } [Fact] - public void Current_FromAsyncContext_IsAsyncContext() + public void Current_FromBrighterSynchronizationHelper_IsBrighterSynchronizationHelper() { - BrighterSynchronizationHelper observedContext = null; - var context = new BrighterSynchronizationHelper(); + BrighterSynchronizationHelper observedHelper = null; + var helper = new BrighterSynchronizationHelper(); - var task = context.Factory.StartNew( - () => { observedContext = BrighterSynchronizationHelper.Current; }, - context.Factory.CancellationToken, - context.Factory.CreationOptions | TaskCreationOptions.DenyChildAttach, - context.TaskScheduler); + var task = helper.Factory.StartNew( + () => { observedHelper = BrighterSynchronizationHelper.Current; }, + helper.Factory.CancellationToken, + helper.Factory.CreationOptions | TaskCreationOptions.DenyChildAttach, + helper.TaskScheduler); - context.Execute(task); + helper.Execute(task); - observedContext.Should().Be(context); + observedHelper.Should().Be(helper); } [Fact] - public void SynchronizationContextCurrent_FromAsyncContext_IsAsyncContextSynchronizationContext() + public void SynchronizationContextCurrent_FromBrighterSynchronizationHelper_IsBrighterSynchronizationHelperSynchronizationContext() { - System.Threading.SynchronizationContext observedContext = null; + System.Threading.SynchronizationContext? observedContext = null; var context = new BrighterSynchronizationHelper(); var task = context.Factory.StartNew( @@ -360,6 +360,35 @@ public async Task Task_AfterExecute_Runs_On_ThreadPool() threadPoolExceptionRan.Should().BeTrue(); } + + [Fact] + public async Task SynchronizationContextCurrent_FromAsyncContext_PostFromAnotherThread() + { + System.Threading.SynchronizationContext? observedContext = null; + var helper = new BrighterSynchronizationHelper(); + + var task = helper.Factory.StartNew( + () => { observedContext =BrighterSynchronizationContext.Current; }, + helper.Factory.CancellationToken, + helper.Factory.CreationOptions | TaskCreationOptions.DenyChildAttach, + helper.TaskScheduler); + + //this should complete the task + helper.Execute(task); + + //but this simulates us being disposed + observedContext.OperationCompleted(); + + //we may be called on a different thread + int value = 1; + await Task.Run(() => + { + observedContext .Post(_ => value = 2, null); + }); + + value.Should().Be(2); + + } [Fact] public void SynchronizationContext_IsEqualToCopyOfItself()