Skip to content

Commit c4ab2bb

Browse files
fix: address three remaining flakes from CI Stress run #25511463854 (#196)
After PR #193 dropped the per-shard failure rate from 32% to 7.5%, three flakes remained. Investigation produced one production fix, one fixture-side race fix, and a more honest test of the render coalescing invariant. Validation regex timeout — production fix ───────────────────────────────────────── ValidationIntegrationTests.Full_Pipeline_Valid_State failed because Validate.Email's MatchValidator capped regex execution at 200ms wall clock. Under threadpool starvation on a CI runner, the timer that RegexMatchTimeout uses can fire mid-match even though CPU time was microseconds, causing IsMatch to throw RegexMatchTimeoutException and the validator to mark the legitimate "user@example.com" as INVALID. Bump validator regex timeout to 1s. Diagnostic-path timeouts (DevtoolsPropertyTools, LogCaptureBuffer, DevtoolsUiaTools) keep their 200ms caps — they're admin/dev-only, not on the keystroke path. False-invalid validation on legitimate input is worse UX than slower tarpit detection on adversarial input, since the former affects every user under any CI-loaded environment. AsyncResource.Framerate.DepsThrashing — fixture race fix ───────────────────────────────────────────────────────── DepsThrashing_AtMostOneCompleted asserted completed<=1; observed completed=2. The fetcher's race window: `await Task.Delay(200, ct)` followed unconditionally by `Interlocked.Increment(ref completed)`. When deps change exactly as the 200ms timer fires, Task.Delay's internal atomic result-set can be won by the timer before cancellation lands — the await resumes normally and the increment runs for a fetch the production hook will correctly discard. Add `ct.ThrowIfCancellationRequested()` after the await so the fixture's counter accurately reflects fetches that ran to completion versus those that completed-then-got-discarded. ThreadSafe_RenderCoalescing — split into honest + strict variants ────────────────────────────────────────────────────────────────── The threshold "1000 setStates → fewer than 200 renders" was too aggressive for the algorithm's actual semantics. Trace: the CAS gate in RequestRender promises "at most one RenderLoop pending" — not "many calls → ~1 render" — when the producer (Task.Run) runs in parallel with an idle UI thread that drains RenderLoop between each setState. Observed 436/1000 and 456/1000 = ~45% coalescing, right in the alternation-dominant regime. Two changes: 1) Existing background-thread fixture: relax threshold from `renders<200` to `renders<writes` (i.e., <1000). This proves *some* coalescing happened and catches a totally-broken-gate regression (which would give renders==writes), but does not over-claim a coalescing ratio the algorithm doesn't promise. Comment updated to reference the strict-coalescing sibling. 2) New ThreadSafe_RenderCoalescingDispatcherBatch fixture: fires 1000 setStates from inside a single DispatcherQueue.TryEnqueue block on the UI thread. Because the UI thread is busy executing the loop, RenderLoop cannot drain between calls — every setState after the first finds the CAS gate held and is coalesced. This is the actual invariant the production gate is designed for, and should hold deterministically: renders<=5 (expected ~2).
1 parent 9096ae2 commit c4ab2bb

4 files changed

Lines changed: 98 additions & 15 deletions

File tree

src/Reactor/Controls/Validation/Validators/BuiltInValidators.cs

Lines changed: 9 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -157,11 +157,15 @@ public MatchValidator(string pattern, string message, string code = "MATCH")
157157
if (pattern is null) throw new ArgumentNullException(nameof(pattern));
158158
if (pattern.Length > MaxPatternLength)
159159
throw new ArgumentException($"Match pattern too long (>{MaxPatternLength} chars).", nameof(pattern));
160-
// SECURITY (TASK-094): cap regex execution at 200ms so a pathological
161-
// pattern can't tarpit the UI thread on every keystroke. 200ms matches
162-
// the cap used elsewhere (DevtoolsPropertyTools, LogCaptureBuffer,
163-
// WaitForPredicate) and tolerates cold-JIT under CI load.
164-
_regex = new Regex(pattern, RegexOptions.Compiled, TimeSpan.FromMilliseconds(200));
160+
// SECURITY (TASK-094): cap regex execution so a pathological pattern
161+
// can't tarpit the UI thread on every keystroke. 1s rather than the
162+
// 200ms used by diagnostic paths (DevtoolsPropertyTools, LogCaptureBuffer):
163+
// RegexMatchTimeout is wall-clock, and under threadpool starvation a
164+
// non-pathological match against a short input can blow past 200ms even
165+
// though CPU time is microseconds. False-invalid validation on legitimate
166+
// input is worse UX than slower tarpit detection on adversarial input,
167+
// since the former affects every user on every CI-loaded environment.
168+
_regex = new Regex(pattern, RegexOptions.Compiled, TimeSpan.FromSeconds(1));
165169
_message = message;
166170
_code = code;
167171
}

tests/Reactor.AppTests.Host/SelfTest/Fixtures/AsyncResourceFramerateFixtures.cs

Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -72,6 +72,15 @@ public override async Task RunAsync()
7272
{
7373
ct.Register(() => Interlocked.Increment(ref cancelled));
7474
await Task.Delay(200, ct);
75+
// Close the race where Task.Delay's 200ms timer
76+
// fires almost simultaneously with cancellation:
77+
// Task.Delay can complete normally (timer wins
78+
// the atomic result-set) even though Cancel()
79+
// was called, leaving the await to resume and
80+
// increment `completed` for a fetch the hook
81+
// will correctly discard. Re-checking the CT
82+
// after the await observes the cancellation.
83+
ct.ThrowIfCancellationRequested();
7584
Interlocked.Increment(ref completed);
7685
return $"dep={dep}";
7786
}

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

Lines changed: 78 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -1,3 +1,4 @@
1+
using Microsoft.UI.Dispatching;
12
using Microsoft.UI.Reactor;
23
using Microsoft.UI.Reactor.Core;
34
using Microsoft.UI.Reactor.AppTests.Host.SelfTest;
@@ -260,9 +261,14 @@ public override async Task RunAsync()
260261
// ════════════════════════════════════════════════════════════════════
261262

262263
/// <summary>
263-
/// Verifies that rapid setState calls coalesce into far fewer actual renders.
264-
/// A component tracks its render count via UseRef. After 1000 rapid setState
265-
/// calls, the render count should be much less than 1000.
264+
/// Verifies that rapid background-thread setState calls coalesce into fewer
265+
/// renders than calls. The CAS gate in RequestRender promises "at most one
266+
/// RenderLoop pending at a time" — it does not promise N calls → ~1 render
267+
/// when the producer runs in parallel with an idle UI thread, because the
268+
/// UI thread can drain RenderLoop between each setState. Under that worst
269+
/// case (multi-core CI runner, no real WinUI work to keep the UI thread
270+
/// busy), coalescing ratio drops to ~50%. The strict-coalescing invariant
271+
/// is tested by the sibling RenderCoalescingDispatcherBatch fixture.
266272
/// </summary>
267273
internal class RenderCoalescing(Harness h) : SelfTestFixtureBase(h)
268274
{
@@ -287,27 +293,89 @@ public override async Task RunAsync()
287293
H.Check("Coalesce_Initial", H.FindText("Value: 0") is not null);
288294

289295
// Fire 1000 setState calls as fast as possible from one thread
296+
const int writes = 1000;
290297
await Task.Run(() =>
291298
{
292-
for (int i = 1; i <= 1000; i++)
299+
for (int i = 1; i <= writes; i++)
293300
setValue!(i);
294301
});
295302

296303
await Harness.Render();
297304

298305
H.Check("Coalesce_FinalValue", H.FindText("Value: 1000") is not null);
299306

300-
// Render count should be far less than 1000 due to coalescing.
301-
// Threshold is loose (1/5 of writes) to tolerate scheduler interleaving
302-
// on CI runners where the UI thread can sneak in renders between
303-
// background-thread setState calls; full coalescing on a quiet machine
304-
// produces ~2 renders.
307+
// Honest threshold: renders < writes proves *some* coalescing happened
308+
// (a totally broken gate would give renders == writes). The "strict"
309+
// invariant — many calls collapse to ~1 render — only holds when the
310+
// producer cannot be interleaved with RenderLoop, which is what the
311+
// sibling RenderCoalescingDispatcherBatch fixture verifies.
305312
var renderText = H.FindControl<TextBlock>(tb =>
306313
tb.Text?.StartsWith("Renders:") == true);
307314
if (renderText is not null)
308315
{
309316
var renders = int.Parse(renderText.Text.Replace("Renders: ", ""));
310-
H.Check($"Coalesce_FarFewerRenders (renders={renders})", renders < 200);
317+
H.Check($"Coalesce_FewerRendersThanWrites (renders={renders}, writes={writes})", renders < writes);
318+
}
319+
}
320+
}
321+
322+
// ════════════════════════════════════════════════════════════════════
323+
// Strict render coalescing: setStates inside one dispatcher block
324+
// ════════════════════════════════════════════════════════════════════
325+
326+
/// <summary>
327+
/// Strict-coalescing complement to RenderCoalescing. Fires 1000 setState
328+
/// calls from inside a single DispatcherQueue.TryEnqueue block on the UI
329+
/// thread. Because the UI thread is busy executing the loop, RenderLoop
330+
/// cannot drain between calls — every setState after the first finds the
331+
/// CAS gate held and is coalesced. Verifies the actual invariant the
332+
/// production gate is designed for: a burst of setStates that arrive
333+
/// faster than RenderLoop drains collapses to ~1 follow-up render.
334+
/// </summary>
335+
internal class RenderCoalescingDispatcherBatch(Harness h) : SelfTestFixtureBase(h)
336+
{
337+
public override async Task RunAsync()
338+
{
339+
Action<int>? setValue = null;
340+
341+
var host = H.CreateHost();
342+
host.Mount(ctx =>
343+
{
344+
var (value, set) = ctx.UseState(0, threadSafe: true);
345+
var renderCount = ctx.UseRef(0);
346+
renderCount.Current++;
347+
setValue = set;
348+
return VStack(
349+
TextBlock($"Value: {value}"),
350+
TextBlock($"Renders: {renderCount.Current}")
351+
);
352+
});
353+
354+
await Harness.Render();
355+
H.Check("CoalesceBatch_Initial", H.FindText("Value: 0") is not null);
356+
357+
const int writes = 1000;
358+
var dq = DispatcherQueue.GetForCurrentThread();
359+
var done = new TaskCompletionSource();
360+
dq.TryEnqueue(() =>
361+
{
362+
for (int i = 1; i <= writes; i++) setValue!(i);
363+
done.SetResult();
364+
});
365+
await done.Task;
366+
await Harness.Render();
367+
368+
H.Check("CoalesceBatch_FinalValue", H.FindText("Value: 1000") is not null);
369+
370+
// Strict bound: 1000 writes inside one dispatcher block cannot
371+
// schedule more than a handful of renders. Allow a small margin
372+
// for any _needsRerender → re-enqueue paths after Render() drains.
373+
var renderText = H.FindControl<TextBlock>(tb =>
374+
tb.Text?.StartsWith("Renders:") == true);
375+
if (renderText is not null)
376+
{
377+
var renders = int.Parse(renderText.Text.Replace("Renders: ", ""));
378+
H.Check($"CoalesceBatch_StrictCoalescing (renders={renders}, writes={writes})", renders <= 5);
311379
}
312380
}
313381
}

tests/Reactor.AppTests.Host/SelfTest/SelfTestFixtureRegistry.cs

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -272,6 +272,7 @@ internal static class SelfTestFixtureRegistry
272272
"ThreadSafe_ReducerSerializationStress",
273273
"ThreadSafe_HighFrequencyLargeTree",
274274
"ThreadSafe_RenderCoalescing",
275+
"ThreadSafe_RenderCoalescingDispatcherBatch",
275276
// Animation system — Curve & Easing
276277
"Curve_RecordEquality",
277278
"Curve_PresetValues",
@@ -1038,6 +1039,7 @@ internal static class SelfTestFixtureRegistry
10381039
"ThreadSafe_ReducerSerializationStress" => new ThreadSafeHookFixtures.ReducerSerializationStress(harness),
10391040
"ThreadSafe_HighFrequencyLargeTree" => new ThreadSafeHookFixtures.HighFrequencyLargeTree(harness),
10401041
"ThreadSafe_RenderCoalescing" => new ThreadSafeHookFixtures.RenderCoalescing(harness),
1042+
"ThreadSafe_RenderCoalescingDispatcherBatch" => new ThreadSafeHookFixtures.RenderCoalescingDispatcherBatch(harness),
10411043
// Animation system — Curve & Easing
10421044
"Curve_RecordEquality" => new CurveTests.RecordEquality(harness),
10431045
"Curve_PresetValues" => new CurveTests.PresetValues(harness),

0 commit comments

Comments
 (0)