Skip to content

Commit 597fe02

Browse files
codemonkeychrisclaude
codemonkeychris
and
claude
authored
fix(hooks): auto-marshal UseState/UseReducer setters to UI thread (#212) (#242)
* fix(hooks): auto-marshal UseState/UseReducer setters to UI thread (#212) UseState and UseReducer setters called from a background thread (Task.Run, PeriodicTimer, callbacks after ConfigureAwait(false)) previously hit a [Conditional("DEBUG")] AssertUIThread that either threw into a discarded Task (DEBUG) or compiled out entirely (RELEASE), producing silent UI update loss in both flavors. Replace the DEBUG-only assert with an always-on UI-thread check that auto-marshals the write and the resulting rerender onto the captured ReactorApp.UIDispatcher when called off-thread. The hot path adds one TLS read + int compare + branch (~1-2 ns); the marshal allocation is only paid on the off-thread path, where it's swamped by cross-thread dispatch cost. When no UI dispatcher has been captured (test/headless contexts), the off-thread call now throws a loud InvalidOperationException in both DEBUG and RELEASE instead of silently racing. threadSafe: true retains its existing locked-in-place semantics for callers that need many concurrent writers to serialize without an intervening UI tick. Adds a NonThreadSafeAutoMarshal selftest that mounts a real WinUI host and reproduces the issue's Task.Run + PeriodicTimer pattern with default hooks. Documents the auto-marshal contract in hooks.md and effects.md (first mention of `threadSafe` in the guide). Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * review: address PR #242 feedback (#212 follow-up) - MarshalIfOffUIThread now throws InvalidOperationException when DispatcherQueue.TryEnqueue returns false (dispatcher shutting down) instead of silently dropping the state update. - XML doc on MarshalIfOffUIThread reworded — dispatcher is captured during host bootstrap (ReactorApp / ReactorHost / ReactorHostControl), not specifically OnLaunched, so embedded and test-harness scenarios are accurately described. - NonThreadSafeAutoMarshal fixture now wraps the Task.Run loop in try/catch + TrySetException and awaits done.Task under a 10s Task.WhenAny timeout, so a regression that throws from the setter fails the fixture deterministically instead of hanging the selftest run. - hooks.md / effects.md add the "captured dispatcher required" caveat and document the shutdown / no-dispatcher InvalidOperationException paths so the prose matches what the code actually does. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> --------- Co-authored-by: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
1 parent 5eec60d commit 597fe02

8 files changed

Lines changed: 300 additions & 18 deletions

File tree

‎docs/_pipeline/templates/effects.md.dt‎

Lines changed: 13 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -55,6 +55,19 @@ The `Func<Action>` overload of `UseEffect` returns a cleanup function. Here
5555
the cleanup disposes the timer, preventing leaks when the component unmounts
5656
or when `isRunning` changes.
5757

58+
The timer body runs on a background thread (it's inside `Task.Run`) and calls
59+
the `updateSeconds` reducer from there. Once the host is bootstrapped, that
60+
works out of the box — every `UseState` / `UseReducer` setter automatically
61+
marshals onto the captured UI dispatcher when called from a non-UI thread, so
62+
timers, `PeriodicTimer`, network callbacks, and code after
63+
`await ... ConfigureAwait(false)` can all call the returned setter without any
64+
extra opt-in. Pass `threadSafe: true` to the hook only when you need many
65+
concurrent setters to apply in-place (locked) instead of being queued one-by-one
66+
onto the UI thread. The setter throws `InvalidOperationException` if it's
67+
called cross-thread before any host has bootstrapped (no UI dispatcher
68+
captured) or after the dispatcher has begun shutting down — make sure the
69+
effect cleanup cancels the background producer so it stops with the component.
70+
5871
## Async Data Loading
5972

6073
Use `UseEffect` with [`UseState`](hooks.md) to load data asynchronously:

‎docs/_pipeline/templates/hooks.md.dt‎

Lines changed: 59 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -120,6 +120,65 @@ object every render. `UseCallback` returns the same delegate instance as long
120120
as the dependencies haven't changed. This matters when passing callbacks to
121121
memoized [child components](components.md).
122122

123+
## Updating State From Background Work
124+
125+
Once the host is bootstrapped, `UseState` and `UseReducer` setters are safe to
126+
call from any thread. When you invoke a setter from a background task — inside
127+
`Task.Run`, from a `PeriodicTimer` loop, from a network callback, or after
128+
`await ... ConfigureAwait(false)` — the setter automatically marshals the write
129+
and the resulting re-render onto the UI dispatcher. You write the same code
130+
you'd write on the UI thread:
131+
132+
<!-- ai:lock -->
133+
```csharp
134+
public override Element Render()
135+
{
136+
var (seconds, setSeconds) = UseState(0);
137+
138+
UseEffect(() =>
139+
{
140+
var cts = new CancellationTokenSource();
141+
_ = Task.Run(async () =>
142+
{
143+
using var timer = new PeriodicTimer(TimeSpan.FromSeconds(1));
144+
while (await timer.WaitForNextTickAsync(cts.Token))
145+
setSeconds(s => s + 1); // auto-marshals to the UI thread
146+
});
147+
return () => cts.Cancel();
148+
});
149+
150+
return TextBlock($"Elapsed: {seconds}s");
151+
}
152+
```
153+
<!-- /ai:lock -->
154+
155+
Each cross-thread setter call costs one `DispatcherQueue.TryEnqueue` —
156+
microseconds, not free, but vastly cheaper than the bugs you'd hit writing the
157+
field directly from a worker. If you need many concurrent setters to apply
158+
in-place rather than serialize through the UI thread (typical for ingest loops
159+
that hammer the same hook from multiple producers), pass `threadSafe: true` to
160+
the hook:
161+
162+
<!-- ai:lock -->
163+
```csharp
164+
var (count, setCount) = UseState(0, threadSafe: true);
165+
var (sum, addToSum) = UseReducer(0, threadSafe: true);
166+
```
167+
<!-- /ai:lock -->
168+
169+
`threadSafe: true` switches the hook to a per-cell lock: concurrent writers
170+
serialize on the lock instead of queuing through the UI dispatcher, and reads
171+
inside the setter (the `prev` argument of a reducer) see the latest committed
172+
write rather than a snapshot from the last UI tick.
173+
174+
> **When auto-marshal can't help.** The setter needs a captured
175+
> `ReactorApp.UIDispatcher` to marshal onto. In unit-test / headless contexts
176+
> that drive `RenderContext` directly, or before the first host has been
177+
> bootstrapped, a cross-thread setter call throws `InvalidOperationException`
178+
> instead of silently racing. The setter also throws if the dispatcher refuses
179+
> the marshaled call (e.g., during shutdown). Cancel background producers in
180+
> your effect cleanup so they stop before the window closes.
181+
123182
## Hook Rules
124183

125184
Hooks must be called **in the same order** every render. Reactor tracks hooks by

‎docs/guide/effects.md‎

Lines changed: 13 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -113,6 +113,19 @@ The `Func<Action>` overload of `UseEffect` returns a cleanup function. Here
113113
the cleanup disposes the timer, preventing leaks when the component unmounts
114114
or when `isRunning` changes.
115115

116+
The timer body runs on a background thread (it's inside `Task.Run`) and calls
117+
the `updateSeconds` reducer from there. Once the host is bootstrapped, that
118+
works out of the box — every `UseState` / `UseReducer` setter automatically
119+
marshals onto the captured UI dispatcher when called from a non-UI thread, so
120+
timers, `PeriodicTimer`, network callbacks, and code after
121+
`await ... ConfigureAwait(false)` can all call the returned setter without any
122+
extra opt-in. Pass `threadSafe: true` to the hook only when you need many
123+
concurrent setters to apply in-place (locked) instead of being queued one-by-one
124+
onto the UI thread. The setter throws `InvalidOperationException` if it's
125+
called cross-thread before any host has bootstrapped (no UI dispatcher
126+
captured) or after the dispatcher has begun shutting down — make sure the
127+
effect cleanup cancels the background producer so it stops with the component.
128+
116129
## Async Data Loading
117130

118131
Use `UseEffect` with [`UseState`](hooks.md) to load data asynchronously:

‎docs/guide/hooks.md‎

Lines changed: 59 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -273,6 +273,65 @@ object every render. `UseCallback` returns the same delegate instance as long
273273
as the dependencies haven't changed. This matters when passing callbacks to
274274
memoized [child components](components.md).
275275

276+
## Updating State From Background Work
277+
278+
Once the host is bootstrapped, `UseState` and `UseReducer` setters are safe to
279+
call from any thread. When you invoke a setter from a background task — inside
280+
`Task.Run`, from a `PeriodicTimer` loop, from a network callback, or after
281+
`await ... ConfigureAwait(false)` — the setter automatically marshals the write
282+
and the resulting re-render onto the UI dispatcher. You write the same code
283+
you'd write on the UI thread:
284+
285+
<!-- ai:lock -->
286+
```csharp
287+
public override Element Render()
288+
{
289+
var (seconds, setSeconds) = UseState(0);
290+
291+
UseEffect(() =>
292+
{
293+
var cts = new CancellationTokenSource();
294+
_ = Task.Run(async () =>
295+
{
296+
using var timer = new PeriodicTimer(TimeSpan.FromSeconds(1));
297+
while (await timer.WaitForNextTickAsync(cts.Token))
298+
setSeconds(s => s + 1); // auto-marshals to the UI thread
299+
});
300+
return () => cts.Cancel();
301+
});
302+
303+
return TextBlock($"Elapsed: {seconds}s");
304+
}
305+
```
306+
<!-- /ai:lock -->
307+
308+
Each cross-thread setter call costs one `DispatcherQueue.TryEnqueue` —
309+
microseconds, not free, but vastly cheaper than the bugs you'd hit writing the
310+
field directly from a worker. If you need many concurrent setters to apply
311+
in-place rather than serialize through the UI thread (typical for ingest loops
312+
that hammer the same hook from multiple producers), pass `threadSafe: true` to
313+
the hook:
314+
315+
<!-- ai:lock -->
316+
```csharp
317+
var (count, setCount) = UseState(0, threadSafe: true);
318+
var (sum, addToSum) = UseReducer(0, threadSafe: true);
319+
```
320+
<!-- /ai:lock -->
321+
322+
`threadSafe: true` switches the hook to a per-cell lock: concurrent writers
323+
serialize on the lock instead of queuing through the UI dispatcher, and reads
324+
inside the setter (the `prev` argument of a reducer) see the latest committed
325+
write rather than a snapshot from the last UI tick.
326+
327+
> **When auto-marshal can't help.** The setter needs a captured
328+
> `ReactorApp.UIDispatcher` to marshal onto. In unit-test / headless contexts
329+
> that drive `RenderContext` directly, or before the first host has been
330+
> bootstrapped, a cross-thread setter call throws `InvalidOperationException`
331+
> instead of silently racing. The setter also throws if the dispatcher refuses
332+
> the marshaled call (e.g., during shutdown). Cancel background producers in
333+
> your effect cleanup so they stop before the window closes.
334+
276335
## Hook Rules
277336

278337
Hooks must be called **in the same order** every render. Reactor tracks hooks by

‎src/Reactor/Core/RenderContext.cs‎

Lines changed: 68 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -30,13 +30,53 @@ internal void BeginRender(Action requestRerender, ContextScope contextScope)
3030
_uiThreadId = Environment.CurrentManagedThreadId;
3131
}
3232

33-
[global::System.Diagnostics.Conditional("DEBUG")]
34-
private void AssertUIThread(string hookName)
33+
/// <summary>
34+
/// Auto-marshals <paramref name="work"/> to the captured UI dispatcher when the
35+
/// caller is not on the UI thread. Returns <c>true</c> if the work was
36+
/// scheduled cross-thread (caller should return without executing inline).
37+
/// Returns <c>false</c> when the caller is already on the UI thread (hot path).
38+
/// <para>
39+
/// When no <see cref="Microsoft.UI.Reactor.ReactorApp.UIDispatcher"/> has been
40+
/// captured (unit-test / headless render contexts) or when the dispatcher has
41+
/// already begun shutting down, a cross-thread call throws instead of silently
42+
/// racing or dropping the update. The dispatcher is captured during host
43+
/// bootstrap (<c>ReactorApp</c>'s <c>OnLaunched</c> for packaged apps, or
44+
/// <c>ReactorHost</c>/<c>ReactorHostControl</c> initialization for embedded /
45+
/// test scenarios), so production code reaches this method only after at
46+
/// least one render has happened.
47+
/// </para>
48+
/// </summary>
49+
private bool MarshalIfOffUIThread(string hookName, Action work)
3550
{
36-
if (Environment.CurrentManagedThreadId != _uiThreadId)
51+
// Hot path — same thread that ran BeginRender. ~1ns TLS read + cmp + branch.
52+
if (Environment.CurrentManagedThreadId == _uiThreadId) return false;
53+
54+
var dq = Microsoft.UI.Reactor.ReactorApp.UIDispatcher;
55+
if (dq is null)
56+
{
57+
// Test/headless context with no captured UI dispatcher AND off-thread.
58+
// The legacy [Conditional("DEBUG")] assert at this spot swallowed silently
59+
// in RELEASE; surface loudly in both flavors so the call is visible.
60+
throw new InvalidOperationException(
61+
$"{hookName} setter was called from thread {Environment.CurrentManagedThreadId}, " +
62+
$"but the captured UI thread is {_uiThreadId}, and no UI dispatcher is " +
63+
$"available to marshal the call. Run the setter on the UI thread, " +
64+
$"or pass threadSafe: true to the hook.");
65+
}
66+
// TryEnqueue returns false when the dispatcher has begun shutting down
67+
// (queue closed, owning thread exiting). Silently swallowing that case
68+
// would lose the state update with no diagnostic; throw so the caller
69+
// sees the same loud failure mode as the no-dispatcher path.
70+
if (!dq.TryEnqueue(() => work()))
71+
{
3772
throw new InvalidOperationException(
3873
$"{hookName} setter was called from thread {Environment.CurrentManagedThreadId}, " +
39-
$"but the UI thread is {_uiThreadId}. Use threadSafe: true to allow cross-thread calls.");
74+
$"but the UI dispatcher refused the marshaled call (TryEnqueue returned " +
75+
$"false — typically because the dispatcher is shutting down). The state " +
76+
$"update was dropped. Stop scheduling background setters past window/app " +
77+
$"shutdown (cancel the producing task in the effect cleanup).");
78+
}
79+
return true;
4080
}
4181

4282
/// <summary>
@@ -55,9 +95,18 @@ internal void UseStateSetterByIndex<T>(int index, T newValue)
5595
/// <summary>
5696
/// Declares a piece of state. Returns (currentValue, setter).
5797
/// Must be called in the same order every render (just like React hooks).
58-
/// When <paramref name="threadSafe"/> is true, the setter can be called from any thread
59-
/// and reads/writes are synchronized. When false (default), the setter must be called
60-
/// from the UI thread — in DEBUG builds, a cross-thread call throws.
98+
/// <para>
99+
/// When <paramref name="threadSafe"/> is true, reads and writes are synchronized
100+
/// with a per-hook lock so concurrent setter calls from many threads serialize.
101+
/// When false (default), cross-thread setter calls are auto-marshaled onto the
102+
/// captured UI dispatcher — the write and the rerender both run on the UI thread,
103+
/// so background-thread setters from <c>Task.Run</c>, <c>PeriodicTimer</c>, or
104+
/// callbacks after <c>await … ConfigureAwait(false)</c> work correctly without
105+
/// any extra opt-in. Use <paramref name="threadSafe"/>: <c>true</c> when you need
106+
/// many concurrent setters to apply in-place (i.e., without an intervening UI
107+
/// thread hop) or when the setter result must be visible to its caller before
108+
/// the next UI tick.
109+
/// </para>
61110
/// </summary>
62111
public (T Value, Action<T> Set) UseState<T>(T initialValue, bool threadSafe = false)
63112
{
@@ -99,7 +148,7 @@ void Setter(T newValue)
99148
}
100149
else
101150
{
102-
AssertUIThread("UseState");
151+
if (MarshalIfOffUIThread("UseState", () => Setter(newValue))) return;
103152
changed = !EqualityComparer<T>.Default.Equals(h.Value, newValue);
104153
if (changed) h.Value = newValue;
105154
if (Diagnostics.ReactorEventSource.Log.IsEnabled(
@@ -116,7 +165,10 @@ void Setter(T newValue)
116165
/// <summary>
117166
/// Declares a piece of state with a functional updater variant.
118167
/// The updater receives the previous value and returns the next.
119-
/// When <paramref name="threadSafe"/> is true, the updater can be called from any thread.
168+
/// Cross-thread updater calls are auto-marshaled onto the captured UI dispatcher
169+
/// (same semantics as <see cref="UseState{T}(T, bool)"/>); pass
170+
/// <paramref name="threadSafe"/>: <c>true</c> for locked in-place updates that
171+
/// serialize many concurrent writers without an intervening UI tick.
120172
/// </summary>
121173
public (T Value, Action<Func<T, T>> Update) UseReducer<T>(T initialValue, bool threadSafe = false)
122174
{
@@ -160,7 +212,7 @@ void Updater(Func<T, T> reducer)
160212
}
161213
else
162214
{
163-
AssertUIThread("UseReducer");
215+
if (MarshalIfOffUIThread("UseReducer", () => Updater(reducer))) return;
164216
var prev = h.Value;
165217
var next = reducer(prev);
166218
changed = !EqualityComparer<T>.Default.Equals(prev, next);
@@ -180,7 +232,10 @@ void Updater(Func<T, T> reducer)
180232
/// Declares a piece of state managed by a reducer function (like Redux).
181233
/// The reducer takes (currentState, action) and returns the next state.
182234
/// Returns (currentState, dispatch) where dispatch sends an action through the reducer.
183-
/// When <paramref name="threadSafe"/> is true, dispatch can be called from any thread.
235+
/// Cross-thread dispatch calls are auto-marshaled onto the captured UI dispatcher
236+
/// (same semantics as <see cref="UseState{T}(T, bool)"/>); pass
237+
/// <paramref name="threadSafe"/>: <c>true</c> for locked in-place dispatch that
238+
/// serializes concurrent writers without an intervening UI tick.
184239
/// </summary>
185240
public (TState Value, Action<TAction> Dispatch) UseReducer<TState, TAction>(
186241
Func<TState, TAction, TState> reducer, TState initialValue, bool threadSafe = false)
@@ -221,7 +276,7 @@ void Dispatch(TAction action)
221276
}
222277
else
223278
{
224-
AssertUIThread("UseReducer");
279+
if (MarshalIfOffUIThread("UseReducer", () => Dispatch(action))) return;
225280
var prev = h.Value;
226281
var next = reducer(prev, action);
227282
if (!EqualityComparer<TState>.Default.Equals(prev, next))
@@ -399,6 +454,7 @@ public Ref<T> UseRef<T>(T initialValue = default!)
399454

400455
void Setter(T newValue)
401456
{
457+
if (MarshalIfOffUIThread("UsePersisted", () => Setter(newValue))) return;
402458
var h = (PersistedHookState<T>)_hooks[currentIndex];
403459
if (!EqualityComparer<T>.Default.Equals(h.Value, newValue))
404460
{

‎tests/Reactor.AppTests.Host/SelfTest/Fixtures/ThreadSafeHookFixtures.cs‎

Lines changed: 77 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -319,6 +319,83 @@ await Task.Run(() =>
319319
}
320320
}
321321

322+
// ════════════════════════════════════════════════════════════════════
323+
// Issue #212 — non-thread-safe setter from a background task auto-marshals
324+
// ════════════════════════════════════════════════════════════════════
325+
326+
/// <summary>
327+
/// Reproduces the Pomodoro / PeriodicTimer scenario from microsoft-ui-reactor#212:
328+
/// a default <c>UseState</c> + <c>UseReducer</c> hook (no <c>threadSafe: true</c>)
329+
/// whose setters are invoked from inside <c>Task.Run</c>. Before the fix this
330+
/// either silently failed (RELEASE) or threw an unobserved
331+
/// <see cref="InvalidOperationException"/> inside <c>_ = Task.Run(...)</c> (DEBUG).
332+
/// After the fix the setter auto-marshals onto the UI dispatcher and the
333+
/// rendered text reflects the writes.
334+
/// </summary>
335+
internal class NonThreadSafeAutoMarshal(Harness h) : SelfTestFixtureBase(h)
336+
{
337+
public override async Task RunAsync()
338+
{
339+
Action<int>? setValue = null;
340+
Action<Func<int, int>>? bumpValue = null;
341+
342+
var host = H.CreateHost();
343+
host.Mount(ctx =>
344+
{
345+
var (value, set) = ctx.UseState(0); // threadSafe: false (default)
346+
var (sum, bump) = ctx.UseReducer(0); // threadSafe: false (default)
347+
setValue = set;
348+
bumpValue = bump;
349+
return VStack(
350+
TextBlock($"Value: {value}"),
351+
TextBlock($"Sum: {sum}")
352+
);
353+
});
354+
355+
await Harness.Render();
356+
H.Check("AutoMarshal_Initial",
357+
H.FindText("Value: 0") is not null &&
358+
H.FindText("Sum: 0") is not null);
359+
360+
// Fire from background tasks — the failure mode the issue documents.
361+
// The discard `_ = Task.Run(...)` is intentional: it's the exact shape
362+
// that swallows the legacy AssertUIThread throw. Wrap the body in
363+
// try/catch + TrySetException so a regression that throws from the
364+
// setter doesn't hang `await done.Task` forever; the outer
365+
// Task.WhenAny + Task.Delay is the belt-and-suspenders timeout in
366+
// case TrySetException is itself bypassed.
367+
const int iterations = 25;
368+
var done = new TaskCompletionSource();
369+
_ = Task.Run(async () =>
370+
{
371+
try
372+
{
373+
for (int i = 1; i <= iterations; i++)
374+
{
375+
setValue!(i);
376+
bumpValue!(prev => prev + 1);
377+
await Task.Delay(1);
378+
}
379+
done.TrySetResult();
380+
}
381+
catch (Exception ex)
382+
{
383+
done.TrySetException(ex);
384+
}
385+
});
386+
387+
var timeout = Task.Delay(TimeSpan.FromSeconds(10));
388+
var winner = await Task.WhenAny(done.Task, timeout);
389+
H.Check("AutoMarshal_LoopCompleted", winner == done.Task);
390+
if (winner == done.Task) await done.Task; // surface any captured exception
391+
// Let the marshaled writes drain and render
392+
for (int i = 0; i < 4; i++) await Harness.Render();
393+
394+
H.Check("AutoMarshal_ValueFinal", H.FindText($"Value: {iterations}") is not null);
395+
H.Check("AutoMarshal_SumFinal", H.FindText($"Sum: {iterations}") is not null);
396+
}
397+
}
398+
322399
// ════════════════════════════════════════════════════════════════════
323400
// Strict render coalescing: setStates inside one dispatcher block
324401
// ════════════════════════════════════════════════════════════════════

0 commit comments

Comments
 (0)