fix(hooks): auto-marshal UseState/UseReducer setters to UI thread (#212)

codemonkeychris · claude · codemonkeychris · commit b5352d3e04af · 2026-05-10T22:13:06.000-07:00
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) &lt;noreply@anthropic.com&gt;
diff --git a/docs/_pipeline/templates/effects.md.dt b/docs/_pipeline/templates/effects.md.dt
@@ -55,6 +55,15 @@ The `Func<Action>` overload of `UseEffect` returns a cleanup function. Here
 the cleanup disposes the timer, preventing leaks when the component unmounts
 or when `isRunning` changes.
 
+The timer body runs on a background thread (it's inside `Task.Run`) and calls
+the `updateSeconds` reducer from there. That works out of the box — every
+`UseState` / `UseReducer` setter automatically marshals onto the UI dispatcher
+when called from a non-UI thread, so timers, `PeriodicTimer`, network
+callbacks, and code after `await ... ConfigureAwait(false)` can all call the
+returned setter without any extra opt-in. Pass `threadSafe: true` to the hook
+only when you need many concurrent setters to apply in-place (locked) instead
+of being queued one-by-one onto the UI thread.
+
 ## Async Data Loading
 
 Use `UseEffect` with [`UseState`](hooks.md) to load data asynchronously:
diff --git a/docs/_pipeline/templates/hooks.md.dt b/docs/_pipeline/templates/hooks.md.dt
@@ -120,6 +120,57 @@ object every render. `UseCallback` returns the same delegate instance as long
 as the dependencies haven't changed. This matters when passing callbacks to
 memoized [child components](components.md).
 
+## Updating State From Background Work
+
+`UseState` and `UseReducer` setters are safe to call from any thread. When you
+invoke a setter from a background task — inside `Task.Run`, from a
+`PeriodicTimer` loop, from a network callback, or after
+`await ... ConfigureAwait(false)` — the setter automatically marshals the write
+and the resulting re-render onto the UI dispatcher. You write the same code
+you'd write on the UI thread:
+
+<!-- ai:lock -->
+```csharp
+public override Element Render()
+{
+    var (seconds, setSeconds) = UseState(0);
+
+    UseEffect(() =>
+    {
+        var cts = new CancellationTokenSource();
+        _ = Task.Run(async () =>
+        {
+            using var timer = new PeriodicTimer(TimeSpan.FromSeconds(1));
+            while (await timer.WaitForNextTickAsync(cts.Token))
+                setSeconds(s => s + 1);   // auto-marshals to the UI thread
+        });
+        return () => cts.Cancel();
+    });
+
+    return TextBlock($"Elapsed: {seconds}s");
+}
+```
+<!-- /ai:lock -->
+
+Each cross-thread setter call costs one `DispatcherQueue.TryEnqueue` —
+microseconds, not free, but vastly cheaper than the bugs you'd hit writing the
+field directly from a worker. If you need many concurrent setters to apply
+in-place rather than serialize through the UI thread (typical for ingest loops
+that hammer the same hook from multiple producers), pass `threadSafe: true` to
+the hook:
+
+<!-- ai:lock -->
+```csharp
+var (count, setCount) = UseState(0, threadSafe: true);
+var (sum, addToSum) = UseReducer(0, threadSafe: true);
+```
+<!-- /ai:lock -->
+
+`threadSafe: true` switches the hook to a per-cell lock: concurrent writers
+serialize on the lock instead of queuing through the UI dispatcher, and reads
+inside the setter (the `prev` argument of a reducer) see the latest committed
+write rather than a snapshot from the last UI tick.
+
 ## Hook Rules
 
 Hooks must be called **in the same order** every render. Reactor tracks hooks by
diff --git a/docs/guide/effects.md b/docs/guide/effects.md
@@ -113,6 +113,15 @@ The `Func<Action>` overload of `UseEffect` returns a cleanup function. Here
 the cleanup disposes the timer, preventing leaks when the component unmounts
 or when `isRunning` changes.
 
+The timer body runs on a background thread (it's inside `Task.Run`) and calls
+the `updateSeconds` reducer from there. That works out of the box — every
+`UseState` / `UseReducer` setter automatically marshals onto the UI dispatcher
+when called from a non-UI thread, so timers, `PeriodicTimer`, network
+callbacks, and code after `await ... ConfigureAwait(false)` can all call the
+returned setter without any extra opt-in. Pass `threadSafe: true` to the hook
+only when you need many concurrent setters to apply in-place (locked) instead
+of being queued one-by-one onto the UI thread.
+
 ## Async Data Loading
 
 Use `UseEffect` with [`UseState`](hooks.md) to load data asynchronously:
diff --git a/docs/guide/hooks.md b/docs/guide/hooks.md
@@ -273,6 +273,57 @@ object every render. `UseCallback` returns the same delegate instance as long
 as the dependencies haven't changed. This matters when passing callbacks to
 memoized [child components](components.md).
 
+## Updating State From Background Work
+
+`UseState` and `UseReducer` setters are safe to call from any thread. When you
+invoke a setter from a background task — inside `Task.Run`, from a
+`PeriodicTimer` loop, from a network callback, or after
+`await ... ConfigureAwait(false)` — the setter automatically marshals the write
+and the resulting re-render onto the UI dispatcher. You write the same code
+you'd write on the UI thread:
+
+<!-- ai:lock -->
+```csharp
+public override Element Render()
+{
+    var (seconds, setSeconds) = UseState(0);
+
+    UseEffect(() =>
+    {
+        var cts = new CancellationTokenSource();
+        _ = Task.Run(async () =>
+        {
+            using var timer = new PeriodicTimer(TimeSpan.FromSeconds(1));
+            while (await timer.WaitForNextTickAsync(cts.Token))
+                setSeconds(s => s + 1);   // auto-marshals to the UI thread
+        });
+        return () => cts.Cancel();
+    });
+
+    return TextBlock($"Elapsed: {seconds}s");
+}
+```
+<!-- /ai:lock -->
+
+Each cross-thread setter call costs one `DispatcherQueue.TryEnqueue` —
+microseconds, not free, but vastly cheaper than the bugs you'd hit writing the
+field directly from a worker. If you need many concurrent setters to apply
+in-place rather than serialize through the UI thread (typical for ingest loops
+that hammer the same hook from multiple producers), pass `threadSafe: true` to
+the hook:
+
+<!-- ai:lock -->
+```csharp
+var (count, setCount) = UseState(0, threadSafe: true);
+var (sum, addToSum) = UseReducer(0, threadSafe: true);
+```
+<!-- /ai:lock -->
+
+`threadSafe: true` switches the hook to a per-cell lock: concurrent writers
+serialize on the lock instead of queuing through the UI dispatcher, and reads
+inside the setter (the `prev` argument of a reducer) see the latest committed
+write rather than a snapshot from the last UI tick.
+
 ## Hook Rules
 
 Hooks must be called **in the same order** every render. Reactor tracks hooks by
diff --git a/src/Reactor/Core/RenderContext.cs b/src/Reactor/Core/RenderContext.cs
@@ -30,13 +30,37 @@ internal void BeginRender(Action requestRerender, ContextScope contextScope)
         _uiThreadId = Environment.CurrentManagedThreadId;
     }
 
-    [global::System.Diagnostics.Conditional("DEBUG")]
-    private void AssertUIThread(string hookName)
+    /// <summary>
+    /// Auto-marshals <paramref name="work"/> to the captured UI dispatcher when the
+    /// caller is not on the UI thread. Returns <c>true</c> if the work was
+    /// scheduled cross-thread (caller should return without executing inline).
+    /// Returns <c>false</c> when the caller is already on the UI thread (hot path).
+    /// <para>
+    /// When no <see cref="Microsoft.UI.Reactor.ReactorApp.UIDispatcher"/> has been
+    /// captured (unit-test / headless render contexts), a cross-thread call throws
+    /// instead of silently racing: there is no UI dispatcher to marshal onto.
+    /// Production hosts always capture a dispatcher in <c>OnLaunched</c>.
+    /// </para>
+    /// </summary>
+    private bool MarshalIfOffUIThread(string hookName, Action work)
     {
-        if (Environment.CurrentManagedThreadId != _uiThreadId)
+        // Hot path — same thread that ran BeginRender. ~1ns TLS read + cmp + branch.
+        if (Environment.CurrentManagedThreadId == _uiThreadId) return false;
+
+        var dq = Microsoft.UI.Reactor.ReactorApp.UIDispatcher;
+        if (dq is null)
+        {
+            // Test/headless context with no captured UI dispatcher AND off-thread.
+            // The legacy [Conditional("DEBUG")] assert at this spot swallowed silently
+            // in RELEASE; surface loudly in both flavors so the call is visible.
             throw new InvalidOperationException(
                 $"{hookName} setter was called from thread {Environment.CurrentManagedThreadId}, " +
-                $"but the UI thread is {_uiThreadId}. Use threadSafe: true to allow cross-thread calls.");
+                $"but the captured UI thread is {_uiThreadId}, and no UI dispatcher is " +
+                $"available to marshal the call. Run the setter on the UI thread, " +
+                $"or pass threadSafe: true to the hook.");
+        }
+        dq.TryEnqueue(() => work());
+        return true;
     }
 
     /// <summary>
@@ -55,9 +79,18 @@ internal void UseStateSetterByIndex<T>(int index, T newValue)
     /// <summary>
     /// Declares a piece of state. Returns (currentValue, setter).
     /// Must be called in the same order every render (just like React hooks).
-    /// When <paramref name="threadSafe"/> is true, the setter can be called from any thread
-    /// and reads/writes are synchronized. When false (default), the setter must be called
-    /// from the UI thread — in DEBUG builds, a cross-thread call throws.
+    /// <para>
+    /// When <paramref name="threadSafe"/> is true, reads and writes are synchronized
+    /// with a per-hook lock so concurrent setter calls from many threads serialize.
+    /// When false (default), cross-thread setter calls are auto-marshaled onto the
+    /// captured UI dispatcher — the write and the rerender both run on the UI thread,
+    /// so background-thread setters from <c>Task.Run</c>, <c>PeriodicTimer</c>, or
+    /// callbacks after <c>await … ConfigureAwait(false)</c> work correctly without
+    /// any extra opt-in. Use <paramref name="threadSafe"/>: <c>true</c> when you need
+    /// many concurrent setters to apply in-place (i.e., without an intervening UI
+    /// thread hop) or when the setter result must be visible to its caller before
+    /// the next UI tick.
+    /// </para>
     /// </summary>
     public (T Value, Action<T> Set) UseState<T>(T initialValue, bool threadSafe = false)
     {
@@ -99,7 +132,7 @@ void Setter(T newValue)
             }
             else
             {
-                AssertUIThread("UseState");
+                if (MarshalIfOffUIThread("UseState", () => Setter(newValue))) return;
                 changed = !EqualityComparer<T>.Default.Equals(h.Value, newValue);
                 if (changed) h.Value = newValue;
                 if (Diagnostics.ReactorEventSource.Log.IsEnabled(
@@ -116,7 +149,10 @@ void Setter(T newValue)
     /// <summary>
     /// Declares a piece of state with a functional updater variant.
     /// The updater receives the previous value and returns the next.
-    /// When <paramref name="threadSafe"/> is true, the updater can be called from any thread.
+    /// Cross-thread updater calls are auto-marshaled onto the captured UI dispatcher
+    /// (same semantics as <see cref="UseState{T}(T, bool)"/>); pass
+    /// <paramref name="threadSafe"/>: <c>true</c> for locked in-place updates that
+    /// serialize many concurrent writers without an intervening UI tick.
     /// </summary>
     public (T Value, Action<Func<T, T>> Update) UseReducer<T>(T initialValue, bool threadSafe = false)
     {
@@ -160,7 +196,7 @@ void Updater(Func<T, T> reducer)
             }
             else
             {
-                AssertUIThread("UseReducer");
+                if (MarshalIfOffUIThread("UseReducer", () => Updater(reducer))) return;
                 var prev = h.Value;
                 var next = reducer(prev);
                 changed = !EqualityComparer<T>.Default.Equals(prev, next);
@@ -180,7 +216,10 @@ void Updater(Func<T, T> reducer)
     /// Declares a piece of state managed by a reducer function (like Redux).
     /// The reducer takes (currentState, action) and returns the next state.
     /// Returns (currentState, dispatch) where dispatch sends an action through the reducer.
-    /// When <paramref name="threadSafe"/> is true, dispatch can be called from any thread.
+    /// Cross-thread dispatch calls are auto-marshaled onto the captured UI dispatcher
+    /// (same semantics as <see cref="UseState{T}(T, bool)"/>); pass
+    /// <paramref name="threadSafe"/>: <c>true</c> for locked in-place dispatch that
+    /// serializes concurrent writers without an intervening UI tick.
     /// </summary>
     public (TState Value, Action<TAction> Dispatch) UseReducer<TState, TAction>(
         Func<TState, TAction, TState> reducer, TState initialValue, bool threadSafe = false)
@@ -221,7 +260,7 @@ void Dispatch(TAction action)
             }
             else
             {
-                AssertUIThread("UseReducer");
+                if (MarshalIfOffUIThread("UseReducer", () => Dispatch(action))) return;
                 var prev = h.Value;
                 var next = reducer(prev, action);
                 if (!EqualityComparer<TState>.Default.Equals(prev, next))
@@ -399,6 +438,7 @@ public Ref<T> UseRef<T>(T initialValue = default!)
 
         void Setter(T newValue)
         {
+            if (MarshalIfOffUIThread("UsePersisted", () => Setter(newValue))) return;
             var h = (PersistedHookState<T>)_hooks[currentIndex];
             if (!EqualityComparer<T>.Default.Equals(h.Value, newValue))
             {
diff --git a/tests/Reactor.AppTests.Host/SelfTest/Fixtures/ThreadSafeHookFixtures.cs b/tests/Reactor.AppTests.Host/SelfTest/Fixtures/ThreadSafeHookFixtures.cs
@@ -319,6 +319,69 @@ await Task.Run(() =>
         }
     }
 
+    // ════════════════════════════════════════════════════════════════════
+    //  Issue #212 — non-thread-safe setter from a background task auto-marshals
+    // ════════════════════════════════════════════════════════════════════
+
+    /// <summary>
+    /// Reproduces the Pomodoro / PeriodicTimer scenario from microsoft-ui-reactor#212:
+    /// a default <c>UseState</c> + <c>UseReducer</c> hook (no <c>threadSafe: true</c>)
+    /// whose setters are invoked from inside <c>Task.Run</c>. Before the fix this
+    /// either silently failed (RELEASE) or threw an unobserved
+    /// <see cref="InvalidOperationException"/> inside <c>_ = Task.Run(...)</c> (DEBUG).
+    /// After the fix the setter auto-marshals onto the UI dispatcher and the
+    /// rendered text reflects the writes.
+    /// </summary>
+    internal class NonThreadSafeAutoMarshal(Harness h) : SelfTestFixtureBase(h)
+    {
+        public override async Task RunAsync()
+        {
+            Action<int>? setValue = null;
+            Action<Func<int, int>>? bumpValue = null;
+
+            var host = H.CreateHost();
+            host.Mount(ctx =>
+            {
+                var (value, set) = ctx.UseState(0); // threadSafe: false (default)
+                var (sum, bump) = ctx.UseReducer(0); // threadSafe: false (default)
+                setValue = set;
+                bumpValue = bump;
+                return VStack(
+                    TextBlock($"Value: {value}"),
+                    TextBlock($"Sum: {sum}")
+                );
+            });
+
+            await Harness.Render();
+            H.Check("AutoMarshal_Initial",
+                H.FindText("Value: 0") is not null &&
+                H.FindText("Sum: 0") is not null);
+
+            // Fire from background tasks — the failure mode the issue documents.
+            // The discard `_ = Task.Run(...)` is intentional: it's the exact shape
+            // that swallows the legacy AssertUIThread throw.
+            const int iterations = 25;
+            var done = new TaskCompletionSource();
+            _ = Task.Run(async () =>
+            {
+                for (int i = 1; i <= iterations; i++)
+                {
+                    setValue!(i);
+                    bumpValue!(prev => prev + 1);
+                    await Task.Delay(1);
+                }
+                done.SetResult();
+            });
+
+            await done.Task;
+            // Let the marshaled writes drain and render
+            for (int i = 0; i < 4; i++) await Harness.Render();
+
+            H.Check("AutoMarshal_ValueFinal", H.FindText($"Value: {iterations}") is not null);
+            H.Check("AutoMarshal_SumFinal", H.FindText($"Sum: {iterations}") is not null);
+        }
+    }
+
     // ════════════════════════════════════════════════════════════════════
     //  Strict render coalescing: setStates inside one dispatcher block
     // ════════════════════════════════════════════════════════════════════
diff --git a/tests/Reactor.AppTests.Host/SelfTest/SelfTestFixtureRegistry.cs b/tests/Reactor.AppTests.Host/SelfTest/SelfTestFixtureRegistry.cs
@@ -274,6 +274,7 @@ internal static class SelfTestFixtureRegistry
         "ThreadSafe_HighFrequencyLargeTree",
         "ThreadSafe_RenderCoalescing",
         "ThreadSafe_RenderCoalescingDispatcherBatch",
+        "ThreadSafe_NonThreadSafeAutoMarshal",
         // Animation system — Curve & Easing
         "Curve_RecordEquality",
         "Curve_PresetValues",
@@ -1055,6 +1056,7 @@ internal static class SelfTestFixtureRegistry
         "ThreadSafe_HighFrequencyLargeTree" => new ThreadSafeHookFixtures.HighFrequencyLargeTree(harness),
         "ThreadSafe_RenderCoalescing" => new ThreadSafeHookFixtures.RenderCoalescing(harness),
         "ThreadSafe_RenderCoalescingDispatcherBatch" => new ThreadSafeHookFixtures.RenderCoalescingDispatcherBatch(harness),
+        "ThreadSafe_NonThreadSafeAutoMarshal" => new ThreadSafeHookFixtures.NonThreadSafeAutoMarshal(harness),
         // Animation system — Curve & Easing
         "Curve_RecordEquality" => new CurveTests.RecordEquality(harness),
         "Curve_PresetValues" => new CurveTests.PresetValues(harness),
diff --git a/tests/Reactor.Tests/ThreadSafeHookTests.cs b/tests/Reactor.Tests/ThreadSafeHookTests.cs
