From 8785b654aded77962a407a111338215083ccefa1 Mon Sep 17 00:00:00 2001 From: Glenn Watson <5834289+glennawatson@users.noreply.github.com> Date: Tue, 21 Jul 2026 09:18:39 +1000 Subject: [PATCH] fix: resume interaction task handlers on the captured context - RegisterHandler(Func<..., Task>) yielded off the scheduler trampoline but discarded the captured SynchronizationContext with ConfigureAwait(false), so UI handlers resumed on a thread-pool thread (#4393) - Await Task.Yield() directly: it yields off the trampoline (#4351) while resuming on the captured context; drop the now-dead YieldToCurrentContext helper - Add tests covering resumption on the installed SynchronizationContext and the handler still running only after the triggering subscription unwinds --- .../Interactions/Interaction.cs | 11 +- .../ReactiveUI.Tests/InteractionsTest.cs | 112 ++++++++++++++++++ 2 files changed, 116 insertions(+), 7 deletions(-) diff --git a/src/ReactiveUI.Shared/Interactions/Interaction.cs b/src/ReactiveUI.Shared/Interactions/Interaction.cs index 2e5c3e6677..2bb97b538b 100644 --- a/src/ReactiveUI.Shared/Interactions/Interaction.cs +++ b/src/ReactiveUI.Shared/Interactions/Interaction.cs @@ -118,14 +118,15 @@ 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)); async Task InvokeAsync(IInteractionContext interaction) { - 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 handler(interaction).ConfigureAwait(false); } } @@ -167,10 +168,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 81036ed95b..518999d84f 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; @@ -267,4 +268,115 @@ 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(); + var completed = new TaskCompletionSource(); + + uiContext.Post(() => + { + var interaction = new Interaction(); + _ = interaction.RegisterHandler(async context => + { + _ = observedContext.TrySetResult(SynchronizationContext.Current); + context.SetOutput(RxVoid.Default); + await Task.CompletedTask; + }); + + _ = interaction.Handle(RxVoid.Default).Subscribe( + _ => completed.TrySetResult(RxVoid.Default), + completed.SetException); + }); + + 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>(); + + uiContext.Post(() => + { + var interaction = new Interaction(); + _ = interaction.RegisterHandler(async context => + { + order.Add(handlerMarker); + context.SetOutput(ResultOutput); + await Task.CompletedTask; + }); + + _ = interaction.Handle(RxVoid.Default).Subscribe( + _ => completed.TrySetResult(order), + completed.SetException); + + // Recorded before the yielded handler continuation is pumped, so it must precede the handler marker. + order.Add(afterSubscribeMarker); + }); + + 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)); + + /// Queues an action to run on the worker thread. + /// The action to run. + public void Post(Action action) => _queue.Add(action); + + /// + 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(); + } + } + } }