Skip to content

Bug: wpfuidispatchertests-ungated-static-swap #648

Description

@drmoisan
  • Work Mode: minor-audit

Summary

QuickFiler.Test/Controllers/WpfUiDispatcherTests.cs swaps the process-wide static
UtilitiesCS.Threading.UiThread._dispatcher to a running WPF dispatcher by raw reflection
(typeof(UiThread).GetField("_dispatcher", BindingFlags.NonPublic | BindingFlags.Static) then
field.SetValue(null, dispatcher)), restores it in a plain finally, and participates in neither of
the two locks introduced by #493. After #493 lands it remains an ungated mutator of the same static
and can still lose an update against a transaction held by the QuickFiler pump fixtures.

Unlike the originating #493 defect, this call site does restore the previous value, so it is a
lesser, distinct concern rather than a recurrence of the no-restore bug. What it lacks is
participation in the lock protocol: it never acquires UiThreadDispatcherFixture.FieldLock, so its
read-modify-write can interleave with a fixture transaction, and its restore is an unconditional
write rather than the fixture's ReferenceEquals compare-then-write.

Proposed fix: route the swap through the shared fixture that #493 created -
await UiThreadDispatcherFixture.BeginTransactionAsync(), then transaction.Install(dispatcher),
and replace the finally restore with transaction.Dispose(), which restores conditionally and then
releases the gate, in that order. Do not reintroduce a second reflection lookup;
UiThreadDispatcherFixture is intended to be the single owner of every mutation of that static made
from this assembly's owned files, and #493's AC-4 gates that uniqueness.
UiThreadDispatcherFixture and UiThreadDispatcherTransaction live in
QuickFiler.Test/Controllers/QfcItemController.UiThreadDispatcherFixture.cs; both are internal to
QuickFiler.Test, so no new grant or assembly reference is needed.

This was recorded as accepted residual risk R-1 of #493 and its § Rollout & Follow-up item 3
asked that it be promoted as its own small issue once the shared fixture exists. It now exists.

Out of scope: the cross-assembly mutators in UtilitiesCS.Test (ProgressTracker_Tests.cs,
ProgressTrackerAsync_Tests.cs, IdleAsyncQueue_Tests.cs) mutate the same process-wide static and
are not covered here. No test-side lock inside QuickFiler.Test can reach them. They are
accepted residual risk R-2 of #493 and overlap #584.

References: motivating fix #493; adjacent open issue on the same static #584; originating defect
report #230.

Environment

  • OS/version: Windows 11 Pro 10.0.26200 (defect is environment-independent; it is a test-isolation
    defect in source, not a platform behavior)
  • Python version: n/a - C# / .NET Framework 4.8 (QuickFiler.Test, MSTest)
  • Command/flags used: vstest.console.exe QuickFiler.Test\bin\Debug\QuickFiler.Test.dll /EnableCodeCoverage /InIsolation
  • Data source or fixture: QfcItemControllerTestSupport.StartRunningDispatcher()

Steps to Reproduce

  1. Inspect QuickFiler.Test/Controllers/WpfUiDispatcherTests.cs, test
    Invoke_InvokeAsync_BeginInvoke_ExecuteDelegateOnDispatcherThread.
  2. Observe the raw reflection write to UiThread._dispatcher and the unconditional finally restore,
    with no acquisition of UiThreadDispatcherFixture.FieldLock or TransactionGate.
  3. Note that after Bug: uithread-dispatcher-static-swap-no-restore #493, all mutations from the owned files hold FieldLock for the whole
    read-modify-write, so this site is the remaining ungated writer inside QuickFiler.Test.

Note: the race is dormant under current CI settings, so a deterministic red run is not expected
without forcing class-level parallelism (see Impact / Severity).

Expected Behavior

Every mutation of UiThread._dispatcher originating in QuickFiler.Test goes through
UiThreadDispatcherFixture, holds FieldLock for the entire read-modify-write, and restores
conditionally via ReferenceEquals compare-then-write.

Actual Behavior

WpfUiDispatcherTests.cs mutates the static directly by reflection without holding either lock, and
restores unconditionally. A concurrent fixture transaction can therefore be clobbered, and this
site's restore can overwrite a value another transaction installed.

Logs / Screenshots

  • Attached minimal logs or screenshot
  • Snippet: no failing run is attached. The defect is a latent ordering hazard that is dormant under
    the CI settings described below; it is evidenced by source inspection rather than a red test.

Impact / Severity

  • Blocker
  • High
  • Medium
  • Low

Low. That assembly runs sequentially in CI (.github/workflows/_mstest-coverage.yml supplies no
/Settings:), so the race is dormant there; it is reachable only under the repo runsettings, which
force <Scope>ClassLevel</Scope> with Workers=0. The swap is single-class and short-lived. This is
a small, bounded change.

Source

From: docs/features/potential/2026-08-27-wpfuidispatchertests-ungated-static-swap.md

Activity

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