[ci-fix-net11] De-flake flaky DispatcherTests by resetting leaked ThreadStatic dispatcher flag

[github-actions]

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: net11.0
Refs: #36192
Attempt: 1/5
Outcome: de-flake (test-quality flake — Step 4.7 bucket b)

Summary

Microsoft.Maui.UnitTests.Dispatching.DispatcherTests is intermittently failing on the net11.0 maui-pr pipeline with a NullReferenceException that disappears on retry (failing in builds 1481555 / 1476157, green in 1484832 / 1486080). This is a test-quality flake caused by thread-state leakage between tests — not a product defect and not an infrastructure problem, so it is de-flaked rather than muted.

Root cause

BackgroundThreadDoesNotGetDispatcherFromMainThread sets the [ThreadStatic] flag DispatcherProviderStubOptions.SkipDispatcherCreation = true inside a Task.Run(...) delegate, which executes on a ThreadPool thread, and never resets it:

await Task.Run(() =>
{
    DispatcherProviderStubOptions.SkipDispatcherCreation = true;   // set on a pooled thread
    var dispatcher = Dispatcher.GetForCurrentThread();
    Assert.Null(dispatcher);
});                                                                // pooled thread returned DIRTY

Because the flag is [ThreadStatic], it stays true on that pooled thread after the task completes. DispatcherProviderStub.GetForCurrentThread() returns null whenever the flag is set. A later test whose async continuation happens to resume on that same pooled thread (e.g. after await Task.Delay(...)) then reads a null dispatcher and throws NullReferenceException. Whether a continuation lands on the contaminated thread is non-deterministic, which is why the failure is intermittent and passes on retry.

This is the only place in the file that writes the flag on a pooled thread:

• BackgroundThreadDoesNotHaveDispatcher sets it on a dedicated new Thread(...) that is discarded after the test, so it cannot leak into the ThreadPool.
• DifferentDispatcherForDifferentThread's Task.Run only reads the dispatcher; it never sets the flag.

Fix

Reset the flag in a finally so the pooled thread is always returned to the pool clean:

await Task.Run(() =>
{
    try
    {
        DispatcherProviderStubOptions.SkipDispatcherCreation = true;
        var dispatcher = Dispatcher.GetForCurrentThread();
        Assert.Null(dispatcher);
    }
    finally
    {
        DispatcherProviderStubOptions.SkipDispatcherCreation = false;
    }
});

The Assert.Null(dispatcher) assertion is preserved unchanged — the test still verifies that a background thread does not inherit the main-thread dispatcher. No assertion is weakened, no [Retry]/[Repeat]/[Ignore] is added, and no timeout is touched. This is a deterministic state-leakage de-flake (Step 4.7 bucket b), not a mute.

Validation

• Static root-cause: the leak is deterministic and the fix removes it at the source. After the change, no test leaves any pooled thread with SkipDispatcherCreation == true, so no later continuation can observe a null dispatcher.
• Not runner-validated locally: the repo's net11.0 SDK (11.0.100-preview.6) is not provisioned in this environment, and the failure is a low-frequency race that cannot be reproduced on demand. CI on this PR (which runs against net11.0) exercises the change.

_{Automated CI-fix candidate (net11.0). Draft for human review — please verify before merging.}

Generated by CI Failure Fixer (net11.0) · 4.7K AIC · ⌖ 152.1 AIC · ⊞ 64.5K · ◷

[MauiBot]

Expert Review — 1 findings

See inline comments for details.

[MauiBot]

🔍 AI-Generated Review (multi-model)

[major] Logic and Correctness — Resetting only the [ThreadStatic] flag does not undo the DispatcherProviderStub value created while skip mode was enabled. DispatcherProviderStub stores GetForCurrentThread() in a ThreadLocal<IDispatcher?>; when line 72 runs with SkipDispatcherCreation = true, that ThreadLocal caches null for this provider/thread, so a later Dispatcher.GetForCurrentThread() on the same pooled thread can still return the cached null after this finally block. Please avoid caching null in the provider when skip mode is active (or otherwise clear the per-thread cached value) so the dispatcher state is actually restored.

[MauiBot]

AI Review Summary

@github-actions[bot] — new AI review results are available based on this last commit: 7014480. To request a fresh review after new comments or commits, comment /review rerun.

---

🔍 Custom Prompt Analysis — maintainer-requested

● AI-generated — supplementary analysis from a maintainer-supplied custom prompt (GitHub Copilot CLI).

Requested: This resets leaked dispatcher state between tests. Could this mask a real Dispatcher leak, and are the added tests sufficient? Be concise.

PR #36221 Supplementary Review

Dispatcher Leak Risk

• Low risk of masking a real product Dispatcher leak. The reset targets DispatcherProviderStubOptions.SkipDispatcherCreation, a [ThreadStatic] test-only switch used by the unit-test dispatcher stub, not production dispatcher state.
• The leak being fixed is test harness state leaking through a reused thread-pool thread inside BackgroundThreadDoesNotGetDispatcherFromMainThread, which could make unrelated later test continuations see Dispatcher.GetForCurrentThread() as null.
• This does not reset DispatcherProvider's real per-thread dispatcher cache or dispose production dispatchers, so it should not hide a shipping-code dispatcher lifetime issue.

Test Sufficiency

• Adequate for the immediate flake. The changed test still verifies the intended scenario: a background thread with dispatcher creation disabled does not inherit the main-thread dispatcher.
• The finally cleanup is appropriate because it preserves the assertion path and guarantees the pooled thread is returned without poisoned [ThreadStatic] state.
• Gap: BackgroundThreadDoesNotHaveDispatcher also sets SkipDispatcherCreation = true without cleanup. It runs inside DispatcherTest.Run on a dedicated Thread, so it is less likely to leak into pooled continuations, but cleanup there would make the fixture more consistently isolated.

Bottom Line

The change looks like a targeted test-isolation fix rather than a mask for a real dispatcher leak. The added coverage is sufficient for the reported leaked-state path, with a small cleanup-consistency improvement available in the neighboring test.

🗂️ Review Sessions — click to expand

🚦 Gate — Test Before & After Fix

Gate Result: ⚠️ INCONCLUSIVE

Platform: ANDROID

⚠️ verify-tests-fail.ps1 exited before writing a verification report. Diagnostics below.

Exit code: 3

Likely cause:

• No fix files detected in the diff (PR may be test-only — should now run in failure-only mode).

Artifacts written before exit:

• test-failure-DispatcherTests.log (2.3 KB)
• verification-log.txt (0.2 KB)

Gate output log (last 60 lines)

📁 Output directory: CustomAgentLogsTmp/PRState/36221/PRAgent/gate/verify-tests-fail
🔍 Detecting base branch and merge point...
No PR detected, scanning remote branches for closest base...
✅ Base branch: net11.0 (via closest-merge-base)
✅ Merge base commit: c2cbef2e
   (1 commits ahead of net11.0)
╔═══════════════════════════════════════════════════════════╗
║         VERIFY FAILURE ONLY MODE                          ║
╠═══════════════════════════════════════════════════════════╣
║  No fix files detected - will only verify:                ║
║  1. Tests FAIL (proving they catch the bug)               ║
║                                                           ║
║  Use this mode when creating tests before writing a fix.  ║
╚═══════════════════════════════════════════════════════════╝
🔍 Auto-detecting test filter from changed test files...
✅ Auto-detected 1 test(s):
   🧪 [UnitTest] DispatcherTests (filter: DispatcherTests)
🧪 Running 1 test(s) (expecting them to FAIL)...
─────────────────────────────────────────────────
🧪 Test 1/1: [UnitTest] DispatcherTests
🧪 Running unit tests: /home/vsts/work/1/s/src/Core/tests/UnitTests/Core.UnitTests.csproj
   Filter: DispatcherTests
  📊 Parsed test results: Passed=9 Failed=0 Total=9 (from 1 result blocks)
==========================================
VERIFICATION RESULTS
==========================================
  🧪 [UnitTest] DispatcherTests: PASSED ❌ (should fail!)
╔═══════════════════════════════════════════════════════════╗
║              VERIFICATION FAILED ❌                       ║
╠═══════════════════════════════════════════════════════════╣
║  8/1 test(s) PASSED but should FAIL!                   ║
║  Those tests don't reproduce the bug. Revise them!        ║
╚═══════════════════════════════════════════════════════════╝

---

📋 Pre-Flight — Context & Validation

Issue: #unknown - DispatcherTests intermittent null dispatcher failure
PR: #36221 - Fix dispatcher test thread-static state leak
Platforms Affected: android test lane / Core unit tests
Files Changed: 0 implementation, 1 test

Key Findings

• GitHub CLI is unauthenticated in this environment, so PR issue/comments could not be fetched directly; analysis used the checked-out PR squashed commit and local diff.
• The PR changes only src/Core/tests/UnitTests/Dispatching/DispatcherTests.cs.
• The PR's current fix resets DispatcherProviderStubOptions.SkipDispatcherCreation in a finally after the ThreadPool-based negative dispatcher assertion.
• A stronger unresolved failure mode remains: DispatcherProviderStub can cache null in its per-thread ThreadLocal<IDispatcher?> while SkipDispatcherCreation is true, and resetting the flag does not clear that cached null.

Code Review Summary

Verdict: NEEDS_CHANGES
Confidence: low
Errors: 1 | Warnings: 0 | Suggestions: 0

Key code review findings:

• ✗ src/Core/tests/UnitTests/Dispatching/DispatcherTests.cs:77 — resetting the [ThreadStatic] flag does not clear a cached null dispatcher in DispatcherProviderStub.

Fix Candidates

#	Source	Approach	Test Result	Files Changed	Notes
PR	PR #36221	Reset DispatcherProviderStubOptions.SkipDispatcherCreation in finally inside the ThreadPool negative-dispatcher test.	⚠️ INCONCLUSIVE (Gate)	src/Core/tests/UnitTests/Dispatching/DispatcherTests.cs	Original PR; plausible cleanup but does not address cached null dispatcher state.

---

🔬 Code Review — Deep Analysis

Code Review — PR #36221

Independent Assessment

What this changes: Adds try/finally cleanup around DispatcherProviderStubOptions.SkipDispatcherCreation = true inside a Task.Run in BackgroundThreadDoesNotGetDispatcherFromMainThread.

Inferred motivation: Prevent [ThreadStatic] test state from leaking on a reusable ThreadPool thread and causing later dispatcher tests to receive null.

Reconciliation with PR Narrative

Author claims: The PR fixes intermittent DispatcherTests NullReferenceException failures by resetting a leaked [ThreadStatic] flag after the pooled-thread test path.

Agreement/disagreement: The leak is plausible and the cleanup is locally useful, but prior CI reportedly still fails in DispatcherTests with CreateTimerNonRepeatingDoesNotRepeat receiving a null dispatcher. That indicates this PR may not fully address the observed failure.

Prior Review Reconciliation

Prior ❌ Error Finding	Source	Status	Evidence
PR cleanup is plausible but not proven; may not cover null dispatcher in timer tests	MauiBot top-level review, per code-review sub-agent context	❌ Unresolved	Current PR diff still only adds try/finally; the reported failing test is still in the same class and can be explained by cached null dispatcher state, not only leaked [ThreadStatic] flag state.

Blast Radius Assessment

• Runs for all instances: No product impact; unit-test-only.
• Startup impact: No.
• Static/shared state: Yes — modifies cleanup around [ThreadStatic] dispatcher-test state and tests use static DispatcherProvider.Current.

CI Status

• Required-check result: undetermined locally; gh is unauthenticated in this environment.
• Classification: gate already reported inconclusive by caller; code-review sub-agent reported PR-target failure context from prior CI.
• Action taken: confidence capped low; no gate verification re-run per caller instruction.

Findings

❌ Error — Current fix does not address cached null dispatcher state

src/Core/tests/UnitTests/Dispatching/DispatcherTests.cs:77

The new finally cleans SkipDispatcherCreation after the inner Task.Run, but the test stub caches dispatcher lookup results per thread using ThreadLocal<IDispatcher?>. When SkipDispatcherCreation is true, the provider stores null for that pooled thread. Resetting the flag does not clear the provider's cached null, so any later continuation on the same pooled thread and same provider can still receive null.

Failure-Mode Probing

• If the assertion inside Task.Run throws: the new finally resets the flag.
• If a later continuation reuses the pooled thread after the flag reset: the thread-static flag is clean, but the provider can still return its previously cached null.
• If async tests resume on ThreadPool threads instead of the dedicated DispatcherTest.Run thread: calls after an await can observe ThreadPool-local state rather than the initial dedicated thread dispatcher.
• If another test mutates static DispatcherProvider.Current concurrently: this PR does not isolate that shared state.

Verdict: NEEDS_CHANGES

Confidence: low
Summary: The code change is plausible but insufficient for the stronger failure mode. Resetting the thread-static flag is not enough if the provider has already cached a null dispatcher for the same thread.

---

🛠️ Fix — Analysis & Comparison

Fix Candidates

#	Source	Approach	Test Result	Files Changed	Notes
1	try-fix-1	Change DispatcherProviderStub so skip mode returns null without caching it in ThreadLocal, and add a regression test for same-thread recovery after skip mode.	✅ PASS	2 files	Demonstrably better than PR fix: the new regression fails without the shared-provider fix but passes with it.
PR	PR #36221	Reset DispatcherProviderStubOptions.SkipDispatcherCreation in finally inside one ThreadPool negative-dispatcher test.	⚠️ INCONCLUSIVE (Gate)	1 file	Does not clear cached null values already stored by DispatcherProviderStub.

Cross-Pollination

Model	Round	New Ideas?	Details
gpt-5.5 + maui-expert-reviewer	1	Yes	Do not let SkipDispatcherCreation populate ThreadLocal with null; treat skip as a transient return condition.

Exhausted: No — stopped because Candidate #1 passed targeted regression tests and is demonstrably better than the PR's current fix.
Selected Fix: Candidate #1 — fixes the shared cached-null root cause rather than relying on one test to reset thread-static state.

---

📝 Recommended PR Title & Description

Assessment: ✏️ Recommend updating — the current metadata accurately explains the original one-file PR fix, but the winning pr-plus-reviewer fix also changes DispatcherProviderStub and adds cached-null regression coverage, so the title/description would become stale if that candidate is applied.

Recommended title

[Core] Dispatching: Avoid cached null dispatchers after skip-mode tests

Recommended description

Target branch: net11.0
Refs: dotnet/maui#36192
Attempt: 1/5
Outcome: de-flake (test-quality flake — Step 4.7 bucket b)

## Summary

`Microsoft.Maui.UnitTests.Dispatching.DispatcherTests` is intermittently failing on the `net11.0` `maui-pr` pipeline with a `NullReferenceException` that disappears on retry (failing in builds 1481555 / 1476157, green in 1484832 / 1486080). This is a test-quality flake caused by dispatcher test thread-state leakage, not a product defect or infrastructure problem, so it is de-flaked rather than muted.

## Root cause

`BackgroundThreadDoesNotGetDispatcherFromMainThread` sets the `[ThreadStatic]` flag `DispatcherProviderStubOptions.SkipDispatcherCreation = true` inside a `Task.Run(...)` delegate, which executes on a ThreadPool thread.

There are two related leakage paths:

1. The `[ThreadStatic]` flag can remain `true` on the pooled thread after the task completes unless the test resets it.
2. `DispatcherProviderStub` can cache `null` in its per-thread `ThreadLocal<IDispatcher?>` when `Dispatcher.GetForCurrentThread()` runs while skip mode is enabled. Resetting the flag alone does not clear that cached `null`, so later calls on the same thread/provider can still return `null`.

Because ThreadPool reuse and async continuation scheduling are non-deterministic, a later dispatcher test can intermittently resume on contaminated state and throw `NullReferenceException`.

## Fix

- Reset `DispatcherProviderStubOptions.SkipDispatcherCreation` in a `finally` in `BackgroundThreadDoesNotGetDispatcherFromMainThread`, so the pooled thread's `[ThreadStatic]` flag is returned to a clean state.
- Change `DispatcherProviderStub.GetForCurrentThread()` so skip mode returns `null` directly without populating `ThreadLocal<IDispatcher?>`.
- Add `SkippedDispatcherCreationDoesNotCacheNullDispatcher` to verify dispatcher creation recovers on the same thread/provider after skip mode is disabled.

The existing `Assert.Null(dispatcher)` assertion is preserved — the test still verifies that a background thread does not inherit the main-thread dispatcher. No assertion is weakened, no `[Retry]`/`[Repeat]`/`[Ignore]` is added, and no timeout is touched.

## Validation

- Targeted `DispatcherTests` validation passed: 10 passed, 0 failed, 0 skipped.
- The new regression covers the cached-null failure mode that a flag-only reset does not address.

<sub>Automated CI-fix candidate (net11.0). Draft for human review — please verify before merging.</sub>

---

🏁 Report — Final Recommendation

Comparative Report — PR #36221

Candidates compared

Rank	Candidate	Result	Assessment
1	pr-plus-reviewer	✅ PASS	Best candidate. It preserves the PR's pooled-thread [ThreadStatic] cleanup and also fixes the expert-reviewer-confirmed cached-null root cause in DispatcherProviderStub. Adds focused regression coverage proving same-thread/provider recovery after skip mode is disabled.
2	try-fix-1	✅ PASS	Strong candidate. It fixes the cached-null root cause by making skip mode bypass ThreadLocal<IDispatcher?> and adds a regression test. It is slightly lower than pr-plus-reviewer because it removes the PR's explicit finally cleanup instead of combining both protections.
3	pr	⚠️ Gate inconclusive; expert review NEEDS_CHANGES	Incomplete. Resetting the [ThreadStatic] flag is useful, but it does not clear a null value already cached in DispatcherProviderStub's ThreadLocal<IDispatcher?>, so the intermittent null-dispatcher failure mode can remain.

Candidate details

pr

The raw PR changes only src/Core/tests/UnitTests/Dispatching/DispatcherTests.cs and wraps SkipDispatcherCreation = true in try/finally. This cleans the ThreadPool thread's [ThreadStatic] flag after the negative dispatcher assertion.

The expert review found a remaining correctness issue: the provider can cache null in ThreadLocal<IDispatcher?> while skip mode is enabled. Resetting the flag later does not invalidate that cached value.

try-fix-1

try-fix-1 changes DispatcherProviderStub so skip mode returns null before touching ThreadLocal<IDispatcher?>, and adds SkippedDispatcherCreationDoesNotCacheNullDispatcher. Its targeted validation passed with 10 dispatcher tests passing, and the new regression reportedly fails without the shared-provider fix.

This fixes the deeper root cause identified by review. The only reason it does not win is that pr-plus-reviewer keeps the PR's explicit flag cleanup while adding the same shared-provider fix and regression coverage.

pr-plus-reviewer

pr-plus-reviewer starts from the PR fix and applies the expert reviewer's actionable feedback:

1. Preserve the finally reset of DispatcherProviderStubOptions.SkipDispatcherCreation.
2. Make DispatcherProviderStub.GetForCurrentThread() return null directly when skip mode is enabled, without caching null in ThreadLocal<IDispatcher?>.
3. Add SkippedDispatcherCreationDoesNotCacheNullDispatcher to cover same-thread/provider recovery after skip mode is reset.

Sandbox validation passed:

dotnet test src/Core/tests/UnitTests/Core.UnitTests.csproj --filter "FullyQualifiedName~Microsoft.Maui.UnitTests.Dispatching.DispatcherTests"

Passed!  - Failed:     0, Passed:    10, Skipped:     0, Total:    10

Winner

Winner: pr-plus-reviewer

This is the single best candidate because it combines the PR's thread-static cleanup with the reviewed shared-provider fix that prevents transient skip mode from permanently caching null. It passed the targeted dispatcher regression suite and ranks above the raw PR, whose remaining cached-null failure mode was confirmed by expert review.

---

🧭 Next Steps — review latest findings

No alternative fix was selected for this run. Review the session findings and CI results before merging.

[PureWeen]

/azp run maui-pr-uitests

[PureWeen]

/azp run maui-pr-devicetests

[azure-pipelines]

Azure Pipelines:
Successfully started running 1 pipeline(s).

[azure-pipelines]

Azure Pipelines:
Successfully started running 1 pipeline(s).

[github-actions]

♻️ Attempt 1/10 — red is an unrelated CI flake, not caused by this PR (head 7014480409bd04fa3d0e2572d15314d62d731a37)

Since the last scan, CI was re-run on this head and most legs are now green. The remaining reds are UI-test categories unrelated to this PR's DispatcherTests.CreateTimerNonRepeatingDoesNotRepeat unit-test fix — CollectionView (Android/MacCatalyst), Image,... (MacCatalyst/iOS), and Cells,...,Dispatcher,...,DragAndDrop/Editor,... (WinUI) — none of which is the target Dispatcher unit test. Several MacCatalyst legs are cancelled (re-run in flight).

A maintainer re-run (/azp run maui-pr-uitests) should settle the remaining legs and clear the flake. No code change required.

Generated by CI Failure Fixer (net11.0) · 636 AIC · ⌖ 18.1 AIC · ⊞ 5.2K · ◷

[kubaflo]

🔍 AI-generated review (multi-model orchestration on behalf of @kubaflo) — informational; draft PR, not a gate.

Code Review — PR #36221 (draft)

Verdict: LGTM (as draft) · Confidence: high (localized, test-only, non-shipping)

What this changes

Wraps the SkipDispatcherCreation = true inside BackgroundThreadDoesNotGetDispatcherFromMainThread's await Task.Run(...) (DispatcherTests.cs:63) in a try/finally that resets the flag to false. This stops the [ThreadStatic] flag from leaking onto a pooled thread and causing a later test's async continuation (resuming on that reused pooled thread) to get a null dispatcher → intermittent NullReferenceException.

Verification

• ✅ DispatcherProviderStubOptions.SkipDispatcherCreation is [ThreadStatic] (DispatcherStub.cs:121), so the PR's rationale is accurate.
• ✅ Correctly targets only the leaking site. There are two = true assignments: line 47 (BackgroundThreadDoesNotHaveDispatcher) and line 63 (this one). Line 47 runs directly inside DispatcherTest.Run, which executes on a dedicated new Thread (DispatcherStub.cs:144) that dies after the test — its [ThreadStatic] state dies with it, no leak, no reset needed. Only line 63's Task.Run hops to the shared thread pool, so only it needs the finally reset. The fix is precise and complete for the identified vector.
• ✅ Reset value false matches the [ThreadStatic] bool default; finally guarantees cleanup even if the assertion throws.

ℹ️ Cross-PR note — complementary to #36708 (not redundant)

#36708 serializes DispatcherTests/MainThreadBridgeTests into a non-parallel collection to fix a different race: cross-class parallel swaps of the process-global DispatcherProvider (SetCurrent(null)). That serialization does not clean pooled-thread [ThreadStatic] state, so it does not fix the SkipDispatcherCreation leak this PR addresses. Conversely, this PR does not address the global-provider swap. Both races are real and distinct — ideally both land (or are merged into one PR). Recommend not closing either as a duplicate.

Blast radius: test-only; no shipping code; no startup path.

---

Reviewed via independent multi-model pass (Claude Opus 4.8 · GPT-5.5 · Gemini 3.1 Pro) cross-pollinated into one verdict. Draft → informational comment only; never an approval.

[azure-pipelines]

Azure Pipelines:
Successfully started running 1 pipeline(s).
There may be pipelines that require an authorized user to comment /azp run to run.

[Copilot]

Pull request overview

This PR targets an intermittent DispatcherTests failure on net11.0 by preventing [ThreadStatic] test state from leaking on ThreadPool threads, which can cause later async continuations to observe a null dispatcher.

Changes:

• Wraps DispatcherProviderStubOptions.SkipDispatcherCreation assignment inside Task.Run(...) with a try/finally to ensure the ThreadPool thread is returned in a clean state.
• Adds an explanatory comment documenting the specific leak mechanism and its impact on test flakiness.