Skip to content

Commit fed5931

Browse files
ChrisPulmanCopilot
andauthored
fix: WaitForDispatcherScheduler marshals to UI thread from non-UI threads (fixes #4354) (#4361)
## Summary Fixes #4354 — recurrence of #3886 and #4180. `RxSchedulers.MainThreadScheduler` (registered as `WaitForDispatcherScheduler` after `WithWpf()` bootstrap) was silently falling back to `CurrentThreadScheduler.Instance` when `Schedule()` was called from a background thread. This made `ObserveOn(RxSchedulers.MainThreadScheduler)` a no-op from background threads, causing cross-thread `InvalidOperationException` crashes on WPF controls (e.g. `MenuItem.UpdateCanExecute`). ## Root Cause `WpfMainThreadScheduler` used `DispatcherScheduler.Current` as its factory: ```csharp // Before public static IScheduler WpfMainThreadScheduler { get; } = new WaitForDispatcherScheduler(static () => DispatcherScheduler.Current); ``` `DispatcherScheduler.Current` internally calls `Dispatcher.FromThread(Thread.CurrentThread)`. This **always returns null on background threads** (pool threads have no WPF Dispatcher), causing an `InvalidOperationException`. `WaitForDispatcherScheduler.AttemptToCreateScheduler()` catches this, returns `CurrentThreadScheduler.Instance` as a temporary fallback, and never caches the real scheduler — so every background-thread call runs on the caller's thread. ## Fix Change the factory to use `Application.Current?.Dispatcher`, which returns the **WPF Application's UI dispatcher** (not the calling thread's dispatcher): ```csharp // After public static IScheduler WpfMainThreadScheduler { get; } = new WaitForDispatcherScheduler( static () => { var dispatcher = Application.Current?.Dispatcher ?? throw new InvalidOperationException("WPF Application has not been initialized yet."); return new DispatcherScheduler(dispatcher); }); ``` **Behaviour:** - Before `Application` is created (very early in startup): factory throws `InvalidOperationException` → caught → retried on next `Schedule()` call (same safe behaviour as before) - Once `Application.Current` is set (from **any** thread, including background): factory succeeds → `_scheduler` cached as `DispatcherScheduler` bound to the UI dispatcher - All subsequent `Schedule()` calls → `DispatcherScheduler.BeginInvoke()` → work correctly marshalled to UI thread ## Changes - **`src/ReactiveUI.Wpf/Builder/WpfReactiveUIBuilderExtensions.cs`** — fix the factory lambda - **`src/tests/ReactiveUI.Tests/WaitForDispatcherSchedulerTests.cs`** — add regression test `FactoryThrowsThenSucceeds_CachesSuccessfulScheduler` covering the retry-then-cache path All 18 `WaitForDispatcherSchedulerTests` pass on net8.0, net9.0, and net10.0. --------- Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
1 parent 1f35503 commit fed5931

3 files changed

Lines changed: 66 additions & 8 deletions

File tree

src/ReactiveUI.Wpf/Builder/WpfReactiveUIBuilderExtensions.cs

Lines changed: 9 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -3,6 +3,8 @@
33
// The .NET Foundation licenses this file to you under the MIT license.
44
// See the LICENSE file in the project root for full license information.
55

6+
using System.Windows;
7+
68
using Splat.Builder;
79

810
namespace ReactiveUI.Builder;
@@ -18,7 +20,13 @@ public static class WpfReactiveUIBuilderExtensions
1820
/// <value>
1921
/// The WPF main thread scheduler.
2022
/// </value>
21-
public static IScheduler WpfMainThreadScheduler { get; } = new WaitForDispatcherScheduler(static () => DispatcherScheduler.Current);
23+
public static IScheduler WpfMainThreadScheduler { get; } = new WaitForDispatcherScheduler(
24+
static () =>
25+
{
26+
var dispatcher = Application.Current?.Dispatcher
27+
?? throw new InvalidOperationException("WPF Application has not been initialized yet.");
28+
return new DispatcherScheduler(dispatcher);
29+
});
2230

2331
/// <summary>
2432
/// Configures ReactiveUI for WPF platform with appropriate schedulers.

src/ReactiveUI/Interactions/Interaction.cs

Lines changed: 1 addition & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -118,12 +118,7 @@ public IDisposable RegisterHandler<TDontCare>(Func<IInteractionContext<TInput, T
118118
ArgumentExceptionHelper.ThrowIfNull(handler);
119119

120120
IObservable<Unit> ContentHandler(IInteractionContext<TInput, TOutput> context) =>
121-
Observable.FromAsync(
122-
async () =>
123-
{
124-
await YieldToCurrentContext();
125-
return handler(context);
126-
})
121+
Observable.FromAsync(() => Task.Run(() => handler(context)))
127122
.SelectMany(result => result.Select(_ => Unit.Default));
128123

129124
return RegisterHandlerCore(ContentHandler);

src/tests/ReactiveUI.Tests/WaitForDispatcherSchedulerTests.cs

Lines changed: 56 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -48,7 +48,7 @@ public async Task FactoryThrowsArgumentNullException_FallsBackToCurrentThread()
4848
}
4949

5050
/// <summary>
51-
/// Tests that factories throws exception re calls on schedule.
51+
/// Tests that factory throws exception re calls on schedule.
5252
/// </summary>
5353
/// <returns>A <see cref="Task" /> representing the asynchronous operation.</returns>
5454
[Test]
@@ -67,6 +67,61 @@ public async Task FactoryThrowsException_ReCallsOnSchedule()
6767
await Assert.That(schedulerFactoryCalls).IsEqualTo(2);
6868
}
6969

70+
/// <summary>
71+
/// Tests that when the factory first throws but later succeeds, the scheduler is cached and reused.
72+
/// This covers the regression where WpfMainThreadScheduler falls back to CurrentThreadScheduler when
73+
/// Schedule is called from a non-UI thread before the WPF Application Dispatcher is available.
74+
/// </summary>
75+
/// <returns>A <see cref="Task" /> representing the asynchronous operation.</returns>
76+
[Test]
77+
public async Task FactoryThrowsThenSucceeds_CachesSuccessfulScheduler()
78+
{
79+
var schedulerFactoryCalls = 0;
80+
var successScheduler = CurrentThreadScheduler.Instance;
81+
var schedulerFactory = new Func<IScheduler>(() =>
82+
{
83+
schedulerFactoryCalls++;
84+
if (schedulerFactoryCalls == 1)
85+
{
86+
throw new InvalidOperationException("Dispatcher not ready yet.");
87+
}
88+
89+
return successScheduler;
90+
});
91+
92+
var sut = new WaitForDispatcherScheduler(schedulerFactory);
93+
94+
// First Schedule call — factory throws, falls back to CurrentThreadScheduler (not cached)
95+
IScheduler? firstCallScheduler = null;
96+
sut.Schedule<object>(
97+
null!,
98+
(scheduler, state) =>
99+
{
100+
firstCallScheduler = scheduler;
101+
return Disposable.Empty;
102+
});
103+
104+
// Second Schedule call — factory succeeds, result is cached
105+
IScheduler? secondCallScheduler = null;
106+
sut.Schedule<object>(
107+
null!,
108+
(scheduler, state) =>
109+
{
110+
secondCallScheduler = scheduler;
111+
return Disposable.Empty;
112+
});
113+
114+
// Third Schedule call — uses cached scheduler, factory not called again
115+
var callsBeforeThird = schedulerFactoryCalls;
116+
sut.Schedule<object>(
117+
null!,
118+
(scheduler, state) => Disposable.Empty);
119+
120+
await Assert.That(firstCallScheduler).IsEqualTo(CurrentThreadScheduler.Instance);
121+
await Assert.That(secondCallScheduler).IsEqualTo(successScheduler);
122+
await Assert.That(schedulerFactoryCalls).IsEqualTo(callsBeforeThird);
123+
}
124+
70125
/// <summary>
71126
/// Tests that factories throws invalid operation exception falls back to current thread.
72127
/// </summary>

0 commit comments

Comments
 (0)