diff --git a/src/ReactiveUI.Shared/Interactions/Interaction.cs b/src/ReactiveUI.Shared/Interactions/Interaction.cs index 9fe50f276e..990eee8a20 100644 --- a/src/ReactiveUI.Shared/Interactions/Interaction.cs +++ b/src/ReactiveUI.Shared/Interactions/Interaction.cs @@ -118,8 +118,6 @@ public IDisposable RegisterHandler(Func, Ta return RegisterHandlerCore(ContentHandler); - // Yield before invoking the async handler so it is not run inside the current scheduler - // trampoline (see #4351). IObservable ContentHandler(IInteractionContext interaction) => new TaskUnitObservable(InvokeAsync(interaction, handler)); @@ -127,7 +125,10 @@ static async Task InvokeAsync( IInteractionContext interaction, Func, Task> asyncHandler) { - await YieldToCurrentContext().ConfigureAwait(false); + // Yield so the handler is not invoked inside the current scheduler trampoline (see #4351). Task.Yield + // resumes on the captured SynchronizationContext, so a handler registered from a UI thread runs back on + // that context instead of a thread-pool thread (see #4393); ConfigureAwait(false) here would discard it. + await Task.Yield(); await asyncHandler(interaction).ConfigureAwait(false); } } @@ -169,10 +170,6 @@ protected Func, IObservable>[] GetH protected virtual IOutputContext GenerateContext(TInput input) => new InteractionContext(input); - /// Yields once so asynchronous handlers are not invoked inside the current scheduler trampoline. - /// A task that completes after the current context has yielded. - private static async Task YieldToCurrentContext() => await Task.Yield(); - /// Registers a normalized interaction handler that produces a stream. /// The normalized handler. /// A disposable which unregisters the handler. diff --git a/src/tests/ReactiveUI.Tests/InteractionsTest.cs b/src/tests/ReactiveUI.Tests/InteractionsTest.cs index 666f8e7327..0b2bbfefbe 100644 --- a/src/tests/ReactiveUI.Tests/InteractionsTest.cs +++ b/src/tests/ReactiveUI.Tests/InteractionsTest.cs @@ -3,6 +3,7 @@ // The .NET Foundation licenses this file to you under the MIT license. // See the LICENSE file in the project root for full license information. +using System.Collections.Concurrent; using ReactiveUI.Tests.Utilities.Schedulers; using TUnit.Core.Executors; @@ -270,4 +271,117 @@ public async Task UnhandledInteractionsShouldCauseException() await Assert.That(ex.Input).IsEqualTo("bar"); } } + + /// + /// A task-based handler registered while a is installed resumes on that + /// captured context rather than on a thread-pool thread, so UI handlers run on the UI thread (see issue #4393). + /// + /// A representing the asynchronous operation. + [Test] + public async Task TaskHandlerResumesOnCapturedSynchronizationContext() + { + using var uiContext = new SingleThreadedSynchronizationContext(); + var observedContext = new TaskCompletionSource(TaskCreationOptions.RunContinuationsAsynchronously); + var completed = new TaskCompletionSource(TaskCreationOptions.RunContinuationsAsynchronously); + + uiContext.Post( + static state => + { + var (observed, done) = ((TaskCompletionSource, TaskCompletionSource))state!; + var interaction = new Interaction(); + _ = interaction.RegisterHandler(async context => + { + _ = observed.TrySetResult(SynchronizationContext.Current); + context.SetOutput(RxVoid.Default); + await Task.CompletedTask; + }); + + _ = interaction.Handle(RxVoid.Default).Subscribe( + _ => done.TrySetResult(RxVoid.Default), + done.SetException); + }, + (observedContext, completed)); + + var handlerContext = await observedContext.Task; + _ = await completed.Task; + + await Assert.That(handlerContext).IsSameReferenceAs(uiContext); + } + + /// + /// A task-based handler yields before running, so it is not invoked inside the subscription that triggers the + /// interaction (see issue #4351); the handler only runs once the triggering call has unwound. + /// + /// A representing the asynchronous operation. + [Test] + public async Task TaskHandlerDoesNotRunInsideTriggeringSubscription() + { + const string afterSubscribeMarker = "after-subscribe"; + const string handlerMarker = "handler"; + const int expectedStepCount = 2; + + using var uiContext = new SingleThreadedSynchronizationContext(); + var order = new List(); + var completed = new TaskCompletionSource>(TaskCreationOptions.RunContinuationsAsynchronously); + + uiContext.Post( + static state => + { + var (steps, done) = ((List, TaskCompletionSource>))state!; + var interaction = new Interaction(); + _ = interaction.RegisterHandler(async context => + { + steps.Add(handlerMarker); + context.SetOutput(ResultOutput); + await Task.CompletedTask; + }); + + _ = interaction.Handle(RxVoid.Default).Subscribe( + _ => done.TrySetResult(steps), + done.SetException); + + // Recorded before the yielded handler continuation is pumped, so it must precede the handler marker. + steps.Add(afterSubscribeMarker); + }, + (order, completed)); + + var sequence = await completed.Task; + + using (Assert.Multiple()) + { + await Assert.That(sequence).Count().IsEqualTo(expectedStepCount); + await Assert.That(sequence[0]).IsEqualTo(afterSubscribeMarker); + await Assert.That(sequence[1]).IsEqualTo(handlerMarker); + } + } + + /// A minimal single-threaded backed by one pumped worker thread. + private sealed class SingleThreadedSynchronizationContext : SynchronizationContext, IDisposable + { + /// The queue of work items pumped on the worker thread in order. + private readonly BlockingCollection _queue = new(); + + /// Initializes a new instance of the class. + public SingleThreadedSynchronizationContext() + { + var thread = new Thread(Run) { IsBackground = true, Name = "interaction-ui" }; + thread.Start(); + } + + /// + public override void Post(SendOrPostCallback d, object? state) => _queue.Add(() => d(state)); + + /// + public void Dispose() => _queue.CompleteAdding(); + + /// Pumps queued work on the worker thread with this context installed as current. + private void Run() + { + SynchronizationContext.SetSynchronizationContext(this); + foreach (var work in _queue.GetConsumingEnumerable()) + { + work(); + } + } + } }