Skip to content

fix(selftest): drain dispatcher after UpdateLayout to fix local TabView race#442

Merged
codemonkeychris merged 1 commit into
mainfrom
fix/selftest-harness-render-tabview-race
May 29, 2026
Merged

fix(selftest): drain dispatcher after UpdateLayout to fix local TabView race#442
codemonkeychris merged 1 commit into
mainfrom
fix/selftest-harness-render-tabview-race

Conversation

@codemonkeychris
Copy link
Copy Markdown
Collaborator

Summary

  • Local 10x sweeps of Reactor.SelfTests flaked at ~60% (top offender 3/10), all clustered in NativeDocking_* fixtures whose pane bodies render through a WinUI TabView. CI was stable.
  • Root cause: Harness.Render() did WaitForIdleAsync() → UpdateLayout() → Task.Delay(16). WaitForIdleAsync short-circuits when Reactor reports idle (a known footgun — flagged in the comment at ReactorHost.cs:910), leaving 16 ms wall-clock as the only safety net for WinUI work that UpdateLayout() itself schedules (notably TabView's lazy ContentPresenter realization for the selected tab). CI dispatchers had steady enough load that 16 ms always covered it; local boxes (IDE, file watchers, AV) lost the race ~30–60% of the time.
  • Fix: yield once at Low priority between two UpdateLayout() calls in Harness.Render. The first layout schedules content-realization onto the dispatcher; the Low yield drains it (guaranteed, not timing-dependent); the second layout arranges the just-realized content. The 16 ms Task.Delay is now just compositor breathing room rather than the entire safety margin.

This is the same pattern WaitForIdleAsync uses internally and what the else (no-host) fallback already did — pulled up so the host-present path benefits too.

Affected flakes observed in the original 10x sweep (all eliminated):

  • NativeDocking_TabGroupRendersToTabView (3/10)
  • NativeDocking_Composition_ContentMutationFlowsToActivePane (2/10)
  • NativeDocking_DockContextHooksResolveOnRealMount (1/10)
  • NativeDocking_Reliability_UseEffectCleanup_BodyRemovedOnPaneClose (1/10)
  • NativeDocking_DynamicallyDockedComponentPage_WithOuterShellState (1/10)

Verification

Three 10x sweeps on local (tools/flake-loop.ps1 against Reactor.SelfTests, x64 Release):

Variant Clean runs Distinct flaky fixtures
Before 4 / 10 5
Low yield before UpdateLayout (wrong order) 7 / 10 5
This PR (yield after UpdateLayout, second layout to arrange) 10 / 10 0

Test plan

  • CI green on this branch (the existing CI signal is the strongest validation that nothing regresses elsewhere — the 815 non-flaky fixtures all still pass each local run)
  • One more local 10x or 20x sweep on a reviewer's box if desired, but 10/10 is consistent with the explanation

🤖 Generated with Claude Code

Local 10x sweeps of the SelfTests suite flake at ~60% (6/10 runs failed),
clustered in NativeDocking_* fixtures whose pane bodies are hosted in a
TabView. The same suite is stable on CI.

Root cause: Harness.Render() did `WaitForIdleAsync() → UpdateLayout() →
Task.Delay(16)`. When Reactor reports idle, WaitForIdleAsync short-circuits
without pumping the dispatcher (the comment at ReactorHost.cs:910 already
flags this: "Returning early here is the classic flake source"). That left
16ms wall-clock as the only safety net for WinUI work that UpdateLayout()
itself scheduled — notably TabView's lazy ContentPresenter realization for
the selected tab, posted at Normal priority.

CI dispatchers run with steady load → 16ms always covered it. Local boxes
(IDE, file watchers, AV) introduced enough jitter that the post-Render
visual-tree probe occasionally lost the race. Failure signature was the
selected pane body / Memo subtree returning null from FindText, which then
cascaded across every assertion in the affected fixture.

Fix: yield once at Low priority between two UpdateLayout() calls. The
first layout schedules content-realization onto the dispatcher; the Low
yield drains it (guaranteed, not timing-dependent); the second layout
arranges the just-realized content. The trailing 16ms Task.Delay is now
just compositor breathing room rather than the entire safety margin.

Same pattern WaitForIdleAsync uses internally and what the no-host
fallback already did — pulled up so the host-present path benefits too.

Verified with 3 local 10x sweeps:
  before:  4/10 clean, 5 distinct flaky fixtures, top offender 3/10
  v1 fix:  7/10 clean (yield before UpdateLayout — wrong order, didn't
           drain layout-spawned work)
  final:  10/10 clean, 0 flakes

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Copy link
Copy Markdown

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

This PR fixes a local-only flake in NativeDocking_* selftest fixtures by restructuring Harness.Render's settle sequence. Previously, after WaitForIdleAsync() (which short-circuits when Reactor reports idle), a single UpdateLayout() plus Task.Delay(16) was the only safety net for WinUI work scheduled by layout itself (notably TabView's lazy content-presenter realization). The new sequence runs UpdateLayout → Low-priority dispatcher yield → UpdateLayout again, so dispatcher-scheduled realization work is deterministically drained before assertions probe the visual tree.

Changes:

  • Replaced the no-host else fallback with an unconditional Low-priority yield placed between two UpdateLayout() calls.
  • Added the second UpdateLayout() to arrange content realized during the yield.
  • Added an explanatory comment block documenting the race and the ordering rationale.
Show a summary per file
File Description
tests/Reactor.AppTests.Host/SelfTest/Harness.cs Restructures Render() to drain dispatcher work scheduled by UpdateLayout via a Low-priority yield, then re-runs layout.

Copilot's findings

Tip

Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

  • Files reviewed: 1/1 changed files
  • Comments generated: 0

@codemonkeychris codemonkeychris merged commit 6d2b1db into main May 29, 2026
16 checks passed
@codemonkeychris codemonkeychris deleted the fix/selftest-harness-render-tabview-race branch May 29, 2026 00:45
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants