Skip to content

[ci-fix] De-flake DispatcherTests by serializing DispatcherProvider global-state mutators - #36391

Closed
github-actions[bot] wants to merge 1 commit into
mainfrom
ci-fix/issue-35910-attempt-1-03dfac0e535d4131-21ad9a228d5cb1e8
Closed

[ci-fix] De-flake DispatcherTests by serializing DispatcherProvider global-state mutators#36391
github-actions[bot] wants to merge 1 commit into
mainfrom
ci-fix/issue-35910-attempt-1-03dfac0e535d4131-21ad9a228d5cb1e8

Conversation

@github-actions

@github-actions github-actions Bot commented Jul 5, 2026

Copy link
Copy Markdown
Contributor

Note

Are you waiting for the changes in this PR to be merged?
It would be very helpful if you could test the resulting artifacts from this PR and let us know in a comment if this change resolves your issue. Thank you!

Target branch: main
Refs: #35910
Attempt: 1/10
Outcome: de-flake (test-quality flake — genuine state-leakage/ordering defect in the test, not a product bug and not infra)

Summary

Microsoft.Maui.UnitTests.Dispatching.DispatcherTests.CreateTimerNonRepeatingDoesNotRepeat intermittently fails on the main maui-pr pipeline with a NullReferenceException (seen in build 1459938, Helix queue osx.15.arm64.maui.open, work item Microsoft.Maui.UnitTests.dll). It is a test-quality flake caused by two test classes racing on a process-global static — not a product defect — so it is de-flaked rather than muted.

Root cause

The reported stack trace points at DispatcherTests.cs line 140, which is var timer = dispatcher.CreateTimer(); — i.e. dispatcher is null, so Dispatcher.GetForCurrentThread() returned null a few lines earlier:

DispatcherTest.Run(async () =>
{
    var dispatcher = Dispatcher.GetForCurrentThread();   // returns null under the race
    var ticks = 0;
    var timer = dispatcher.CreateTimer();                // line 140 -> NullReferenceException
    ...
    await Task.Delay(TimeSpan.FromSeconds(1.1));          // the FIRST await is here, AFTER the NRE
});

DispatcherTest.Run executes the test body on a fresh dedicated new Thread(...), and the failing line runs synchronously, before the first await. On a brand-new thread every [ThreadStatic] starts at its default, so a leaked DispatcherProviderStubOptions.SkipDispatcherCreation flag cannot be the cause of this failure.

The real cause is a data race on the process-global provider in src/Core/src/Dispatching/DispatcherProvider.cs:

static IDispatcherProvider? s_currentProvider;                    // process-global, not thread-static
public static IDispatcherProvider Current => s_currentProvider ??= new DispatcherProvider();

DispatcherTests (ctor SetCurrent(stub), Dispose SetCurrent(null)) and MainThreadBridgeTests (repeatedly SetCurrent(stub) / SetCurrent(null)) are the only two classes in this assembly that mutate that global, and by default xUnit runs different test classes in parallel collections. When MainThreadBridgeTests calls DispatcherProvider.SetCurrent(null) at the moment DispatcherTests' dedicated thread reads DispatcherProvider.Current, the getter lazily constructs the real DispatcherProvider, whose GetForCurrentThreadImplementation() returns null on a non-UI thread → null dispatcher → NullReferenceException. Whether the two interleave is timing-dependent, which is why it reproduces ~1 in 10 builds and passes on retry.

Fix

Introduce a shared, non-parallel xUnit collection and apply it to the only two classes that touch the global, so they can never run concurrently:

[CollectionDefinition(DispatcherProviderGlobalStateCollection.Name, DisableParallelization = true)]
public class DispatcherProviderGlobalStateCollection
{
    public const string Name = "DispatcherProviderGlobalState";
}
[Category(TestCategory.Core, TestCategory.Dispatching)]
[Collection(DispatcherProviderGlobalStateCollection.Name)]
public class DispatcherTests : IDisposable { ... }

[Category(TestCategory.Core, TestCategory.Hosting)]
[Collection(DispatcherProviderGlobalStateCollection.Name)]
public class MainThreadBridgeTests : IDisposable { ... }

This mirrors the existing in-repo pattern CultureTestsCollection (src/Controls/tests/SourceGen.UnitTests). No assertion is removed or weakened, no test is disabled or skipped, no [Retry]/[Repeat] is added, and no timeout is changed — the two racing classes are simply serialized. This is a deterministic state-leakage de-flake, not a mute.

Relationship to net11.0 PR #36221

The parallel net11.0 workflow opened #36221 for the same test file, fixing a different latent flake: a [ThreadStatic] SkipDispatcherCreation leak from BackgroundThreadDoesNotGetDispatcherFromMainThread onto a pooled thread, observable only in an async continuation after an await. That mechanism cannot explain #35910's signature, which fails before any await on a fresh non-pooled thread. The two fixes are complementary and non-overlapping; this PR targets main and the SetCurrent race specifically.

Validation

  • Static root-cause: the null dispatcher can only originate from a concurrent SetCurrent(null) on the shared global; serializing the two — and only two — mutator classes in this assembly removes the concurrency window at its source.
  • Not runner-validated locally: the agent sandbox is network-restricted (the Arcade SDK feed returns proxy 403), so the repo cannot be restored/built here, and the failure is a low-frequency race that cannot be reproduced on demand. This PR's own maui-pr CI — specifically the Helix Unit Tests legs that execute Microsoft.Maui.UnitTests.dll — exercises the change.

Automated CI-fix candidate (main). Draft for human review — please verify before merging.

Warning

Firewall blocked 1 domain

The following domain was blocked by the firewall during workflow execution:

  • l49vsblobprodcus358.vsblob.vsassets.io

To allow these domains, add them to the network.allowed list in your workflow frontmatter:

network:
  allowed:
    - defaults
    - "l49vsblobprodcus358.vsblob.vsassets.io"

See Network Configuration for more information.

Generated by CI Failure Fixer (main) · 3.7K AIC · ⌖ 106.9 AIC · ⊞ 79.2K ·

…lobal-state mutators

DispatcherTests.CreateTimerNonRepeatingDoesNotRepeat intermittently threw a
NullReferenceException at 'dispatcher.CreateTimer()' because
Dispatcher.GetForCurrentThread() returned null. The dispatcher is obtained
synchronously on a fresh dedicated thread (DispatcherTest.Run) before any await,
so a leaked [ThreadStatic] flag cannot be the cause. The real cause is a race on
the process-global DispatcherProvider.s_currentProvider: MainThreadBridgeTests
runs in a parallel xUnit collection and calls DispatcherProvider.SetCurrent(null),
which nulls the shared provider while DispatcherTests is reading it. The Current
getter then lazily constructs the real DispatcherProvider, whose
GetForCurrentThreadImplementation() returns null on a non-UI thread -> NRE.

Serialize the only two classes in this assembly that mutate that global
(DispatcherTests and MainThreadBridgeTests) into a shared, non-parallel xUnit
collection so they can never run concurrently. No assertion is weakened and no
test is disabled or retried; this is a deterministic state-leakage de-flake.

Refs: #35910

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
@PureWeen

PureWeen commented Jul 5, 2026

Copy link
Copy Markdown
Member

Closing as a duplicate canary artifact. This draft was auto-generated by a live test dispatch of the CI-fix workflow (#36317) running from a feature branch, not a scheduled run. It is a byte-identical duplicate of its sibling (the two were created by a single agent cycle that incorrectly re-emitted create_pull_request to 'overwrite' an oversized patch — the safe-output handler mints a new branch + PR per emission, so it duplicated instead). It also overlaps the existing de-flake in #36221 (same DispatcherTests flake, issue #36192). Being cleaned up; the underlying prompt bug is being fixed in #36317.

@PureWeen PureWeen closed this Jul 5, 2026
@PureWeen
PureWeen deleted the ci-fix/issue-35910-attempt-1-03dfac0e535d4131-21ad9a228d5cb1e8 branch July 5, 2026 15:31
PureWeen added a commit that referenced this pull request Jul 6, 2026
The live canary (run 28724898702) surfaced a duplicate-PR bug: the agent
emitted create_pull_request for issue #35910, noticed the captured patch was
oversized (the fix branch had been cut from the non-main dispatch ref instead
of origin/main), and re-emitted create_pull_request believing 'the same branch
name' would overwrite the prior PR. It does not — the safe-output handler mints
a fresh branch (unique suffix) and opens a NEW PR per emission, so the recovery
produced two identical draft PRs (#36391, #36392) instead of one.

Add a single-shot idempotency callout to both twins' FRESH-mode section:
create_pull_request must be emitted at most once per issue per run; if the patch
is discovered wrong after emission, record a skip rather than re-emitting. Also
reinforces confirming the diff-stat before the single emission.

The oversized-patch trigger itself is test-harness-specific (a real scheduled
run from main has HEAD == origin/main, so no divergence); the re-emit
misconception is the generalizable bug this guards against.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
@github-actions github-actions Bot mentioned this pull request Jul 6, 2026
@github-actions github-actions Bot locked and limited conversation to collaborators Aug 5, 2026
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant