Skip to content

Bug: winformspumphost-tests-load-flaky-visible-window #511

Description

@drmoisan
  • Work Mode: full-bug

Summary

Unit tests built on QuickFiler.Test/TestSupport/WinFormsPumpHost.cs start a real WinForms message pump (Application.Run) on a dedicated STA thread and construct real WinForms controls. They fail nondeterministically when the machine is under heavy CPU load, and they display a visible window during an otherwise headless test run.

Environment

  • OS/version: Windows 11 Pro 10.0.26200
  • Runtime: .NET Framework 4.8.1, MSTest via vstest.console.exe (VS18 test platform)
  • Command/flags used: ./scripts/vscode/Invoke-MSTestWithCoverage.ps1 -SearchRoot . (full-suite run with coverage)
  • Data source or fixture: QuickFiler.Test/TestSupport/WinFormsPumpHost.cs; affected suites include QfcItemController_InitializationTests (the *ThroughThePumpHost* cases) and WpfDispatcherYieldTests

Steps to Reproduce

  1. Drive the machine to sustained high CPU utilization (observed at approximately 96%).
  2. Run the full MSTest suite with coverage using the command above.
  3. Repeat several times and observe both the pass/fail outcome and the desktop.

Expected Behavior

Unit tests are deterministic and headless. Per .claude/rules/general-unit-test.md, tests must produce the same result given the same inputs and must not depend on real wall-clock timing. No unit test should create a visible window on the developer's desktop.

Actual Behavior

  • The affected tests failed nondeterministically under load. During baseline capture for issue Bug: quickfiler-search-keystroke-focus-steal #438 on 2026-08-08, six attempts were required to obtain a clean full-suite baseline; the failures were confined to these suites and did not reproduce once the machine was idle.
  • A visible window appeared during the run, because the host constructs a real WinForms control and pumps a real message loop.

Logs / Screenshots

  • Attached minimal logs or screenshot
  • Snippet: no captured failure log is retained; the observation is recorded in the issue Bug: quickfiler-search-keystroke-focus-steal #438 execution report and in .claude/agent-memory/atomic-executor/project_winformspumphost_tests_load_flaky.md. A fresh capture under induced load should accompany the fix.

Impact / Severity

  • Blocker
  • High
  • Medium
  • Low

Nondeterministic tests are corrosive to an autonomous development workflow: they force repeated full-suite reruns (six were needed for one baseline), and they train reviewers and agents to retry a red suite rather than investigate it, which is exactly the habit that lets a real regression through. The visible window also makes the suite unsuitable for unattended or headless CI execution.

Source

From: docs/features/potential/2026-08-08-winformspumphost-tests-load-flaky-visible-window.md

Activity

  1. drmoisan commented on Aug 8, 2026

    @drmoisan
    OwnerAuthor

    Additional symptom from the same root cause, observed 2026-08-08 during remediation of #438.

    Repo-wide coverage measurement is not reproducible across runs. Five full-suite coverage runs on the same commit produced line-rates clustered around 0.8585-0.8586 rather than a stable value. The variance was isolated to a small, rotating set of legacy classes — EfcHomeController.cs, PropertyStore.cs, SubjectMapSco.Orchestration.cs, and SegmentStopWatch.cs (a wall-clock timing helper).

    Concrete impact: a coverage floor recorded in one session (line 0.858665) could not be reproduced by a later run of the unmodified tree, which measured 0.858512. That caused a false coverage-regression failure on a change that had actually increased coverage. Any fixed cross-session coverage constant used as a gate will keep producing false failures until the underlying nondeterminism is fixed.

    Also seen in the same session: one hung testhost and an unrelated intermittent failure in WpfDispatcherYieldTests.YieldAsync_WithoutDispatcher_RemainsStrict, which is consistent with the timing-dependent-test hypothesis in this issue.

    Two implications worth folding into this issue's scope:

    1. Timing-dependent tests affect the coverage metric, not just pass/fail stability.
    2. Coverage gates should compare against a baseline captured in the same session and run rather than a constant carried across sessions. Full analysis: docs/features/active/2026-08-07-quickfiler-search-keystroke-focus-steal-438/evidence/other/timestamp-and-coverage-floor-correction.2026-08-08T19-25Z.md on branch bug/quickfiler-search-keystroke-focus-steal-438.
  2. drmoisan commented on Aug 8, 2026

    @drmoisan
    OwnerAuthor

    Fresh failure capture under load, with a controlled attribution experiment

    The issue body notes that "no captured failure log is retained; ... A fresh capture under induced load should accompany the fix." This comment supplies that capture. It was obtained incidentally while fixing #508 (a different order-dependent test), where these failures blocked the final toolchain gate and had to be attributed before that work could proceed.

    The two failing tests and the exact exception

    Both in QuickFiler.Test, class QuickFiler.Controllers.Tests.QfcItemController_InitializationTests:

    • InitializeBool_ThroughThePumpHost_CompletesAndInitializesState
    • InitializeNineArgOverload_ThroughThePumpHost_SavesParametersAndDelegates
    System.InvalidOperationException: Invoke or BeginInvoke cannot be called on a control
    until the window handle has been created.
       at System.Windows.Forms.Control.MarshaledInvoke(...)
       at QuickFiler.Controllers.QfcItemController.InvokeBeginInvoke(Boolean async, Action action)
            in QuickFiler\Controllers\QfcItemController.FocusAndTheme.cs:line 256
       at QuickFiler.Controllers.QfcItemController.ToggleTips(Boolean async, ToggleState desiredState)
            in QuickFiler\Controllers\QfcItemController.FocusAndTheme.cs:line 204
       at QuickFiler.Controllers.QfcItemController.Initialize(Boolean async)
            in QuickFiler\Controllers\QfcItemController.Initialization.cs:line 185
       at ... QuickFiler.Test\TestSupport\WinFormsPumpHost.cs:line 95
    

    This localizes the race precisely: WinFormsPumpHost hands control to QfcItemController.Initialize before the hosted control's window handle exists, and InvokeBeginInvoke at QfcItemController.FocusAndTheme.cs:256 marshals against that not-yet-created handle. It is a handle-creation ordering defect, not a general timing sensitivity.

    Controlled four-run experiment (isolates scope, not just symptom)

    Run at merge-base 003c5715, varying only the presence of an unrelated UtilitiesCS change:

    # Configuration Scope EXIT Total Passed Failed
    A change present QfcItemController_InitializationTests only, /InIsolation 0 9 9 0
    B change present full QuickFiler.Test assembly alone, /InIsolation 0 867 867 0
    C change present full 9-assembly instrumented suite 1 6295 6293 2
    C (repeat) change present full 9-assembly instrumented suite 1 6295 6293 2
    D change fully reverted full 9-assembly instrumented suite 1 6293 6291 2

    Two findings worth recording:

    1. The tests pass in class isolation (9/9) and in their own assembly (867/867), and fail only in the combined dotnet-coverage-instrumented 9-assembly run. Coverage instrumentation overhead is sufficient on its own to trigger the race — induced CPU load is not required to reproduce it. That is a cheaper and more reliable repro than the "drive the machine to ~96% CPU" steps currently in the issue body.
    2. Run D is the control: with the unrelated change reverted and git status --porcelain -- '*.cs' '*.csproj' '*.sln' empty, the same two tests fail with the same exception. The failure is intrinsic to merge-base.

    6293 / 6291 / 2 from run D is a byte-for-byte match with the "Run 1" baseline recorded in issue #508. So the historical Failed: 2 observation there was these two tests, and #508's own test accounted for the separate Failed: 1 observation in Run 2.

    Provenance

    The two tests were introduced by commit 8f98264c ("feat(quickfiler-test): add WinForms message-pump test seam (#230)", merged as PR #479), so the defect entered with the pump-host seam itself rather than with a later change.

    One scope correction to the issue body

    The issue currently lists WpfDispatcherYieldTests among the affected suites. That suite's nondeterminism had a different root cause — an unarranged ambient WPF Dispatcher precondition, not the WinForms pump host — and is fixed under #508 by making the dispatcher lookup injectable. After that fix, WpfDispatcherYieldTests passed 4667/4667 on three consecutive full parallel runs. It should be removed from the affected-suite list here so the remaining scope is unambiguously WinFormsPumpHost and its *ThroughThePumpHost* consumers.

    Note also that a WPF Dispatcher running on an owned STA thread does not create a visible window; the visible-window symptom in this issue is specific to WinFormsPumpHost constructing a real WinForms control and running a real Application.Run message loop.

    Full artifact

    The complete experiment record, including SHA-256 verification that the reverted change was restored byte-for-byte, is committed at:

    docs/features/active/2026-08-08-wpf-dispatcher-yield-test-order-dependent-508/evidence/regression-testing/preexisting-failure-attribution.2026-08-08T16-52.md

  3. 4 remaining items

  4. drmoisan commented on Aug 23, 2026

    @drmoisan
    OwnerAuthor

    Closing as superseded by #592 — the symptom is real and still tracked

    To be unambiguous: the flakiness described here is real and is not dismissed. It is measured at roughly a 1-in-21 (~4.8%) run-level failure rate, and reproduces under load at 8 of 10 green under induced 17-node MSBuild contention. What is wrong is the diagnosis, and that is why this issue is being closed rather than left open.

    Why closing rather than correcting in place

    Three reasons.

    1. The stated root cause is falsified. This issue attributes the failure to a missing window handle on ItemViewer, and its remedy forces handle creation. Verified against source:

    • ItemViewer() calls InitializeComponent() — QuickFiler/Viewers/ItemViewer.cs:25
    • InitializeComponent runs BeginInit() on both WebView2 children — QuickFiler/Viewers/ItemViewer.Designer.cs:89-90
    • and EndInit() on them — :6166-6167
    • EndInit creates the child handles, and WinForms creates a parent's handle when a child's handle is created

    The viewer's handle therefore already exists the instant construction returns. The remedy would force a handle that is already present — a no-op.

    2. The failure signature contradicts the diagnosis. PumpTimeoutMs = 60000 (QuickFiler.Test/Controllers/QfcItemController.InitializationTests.cs:38, applied as [Timeout(PumpTimeoutMs)]). The one genuine pre-fix failure was seven expiries at 60,000 ms, not an exception. A missing handle makes Control.Invoke throw immediately; it does not hang for sixty seconds.

    3. A premise correction in a comment is not enough. The issue body still reads authoritatively, and a body is what gets implemented. This has already happened once: the epic-planning pass was given the seam-dependency premise straight from this issue text and had to refute it against source. Leaving the issue open with a corrective comment preserves the trap for the next reader.

    #511 and #571 also contradict each other

    Independently of the false premise, these two cannot both be implemented as written. #511 proposes replacing the real message pump with an injectable context; executed literally that deletes the very tests #571 exists to stabilize. That conflict disappears once both are superseded by a single correctly-diagnosed issue.

    Where the work went

    Reopen this issue if the #592 investigation shows the handle attribution was correct after all.

  5. drmoisan commented on Sep 13, 2026

    @drmoisan
    OwnerAuthor

    Reconciliation from issue #743 (2026-09-13)

    This comment reconciles the closing state of #511 against the current tree and the resolution delivered under #743.

    (a) The premise correction stands

    The premise correction recorded above on 2026-08-22, which refutes the window-handle root cause, still holds against the current tree and was not in error. Quoting it: "EndInit creates the WebView2 child window handles, and WinForms creates a parent's handle when a child's handle is created. The ItemViewer's own handle therefore exists the instant construction returns." Re-verified on branch bug/quickfiler-itemviewer-ui-marshalling-seam-743: ItemViewer() calls InitializeComponent() at QuickFiler/Viewers/ItemViewer.cs:25; InitializeComponent runs BeginInit() on both WebView2 children at QuickFiler/Viewers/ItemViewer.Designer.cs:89-90 and EndInit() on them at QuickFiler/Viewers/ItemViewer.Designer.cs:6165 and :6166. One citation correction only: the two earlier comments cite the EndInit() pair as lines 6166-6167; in the current tree the pair sits at lines 6165 and 6166 (line 6169 is the _topicThread EndInit()). The window-handle root cause remains refuted, and forcing the handle remains a measured no-op.

    (b) Forward pointer

    The forward pointer to #592 is stale: #592 is closed and consolidated into #743 (2026-09-02-quickfiler-itemviewer-ui-marshalling-seam-743). #743 carries the resolution: QfcItemController.ResolveControlGroupsAsync is widened from the concrete ItemViewer to IItemViewer through two additive interface members (DescendantControls() and ItemNumberLabel), and the AssignControlsAsync marshal is routed through the injected IUiDispatcher seam with the same null tolerance the other seam sites carry. The member is therefore driven from a deterministic seam test class, QuickFiler.Test/Controllers/QfcItemController.SeamMarshallingTests.cs, with no pump host and no concrete viewer (fail-before three of three runs with InvalidCastException, pass-after three of three, then 62 consecutive targeted runs with zero failures). The retained pump-hosted test ResolveControlGroupsAsync_ThroughThePumpHost_PopulatesTipsAndControlGroups is unchanged. Evidence lives under docs/features/active/2026-09-02-quickfiler-itemviewer-ui-marshalling-seam-743/evidence/ on that branch.

    (c) The gate hypothesis is superseded

    The UiThreadDispatcherGate / SwapUiThreadDispatcher hypothesis restated in the closing comment above is superseded. Per correction C1 in the #743 spec, those two identifiers exist in zero .cs files in the current tree (they were removed under #493; the mechanism that exists today is UiThreadDispatcherFixture / UiThreadDispatcherTransaction). The mechanism was then identified by measurement rather than inference (AC1 verdict artifact evidence/baseline/ac1-mechanism-verdict.2026-09-12T17-00.md): three monotonic counters on the one-permit TransactionGate recorded, in a serial-regime run of the whole QuickFiler.Test assembly, acquisitions=11 releases=10 contended=0, with a balance test holding the permit and asserting acquisitions minus releases equals exactly 1. A serial run cannot queue a second live holder, so a contended count of zero rejects the gate-leak hypothesis (H-LEAK) by direct observation. The surviving mechanism is elapsed pump-hosted fixture cost under load (H-COST): the six ThroughThePumpHost tests measured 68-125 ms serially and up to 6,460 ms under class-level parallelism alone, and the recorded 6x-26x load multiplier applied to the latter exceeds the 60,000 ms PumpTimeoutMs bound. No expiry was reproduced during the instrumented runs; that is recorded as a negative result, and the identification rests on the counter observable rather than on an observed expiry.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

No one assigned

    Labels

    bugSomething isn't working

    Projects

    No projects

      Milestone

      No milestone

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions