Skip to content

fix(quickfiler-test): bound TransactionGate acquisition in UiThreadDispatcherFixture (882) - #934

Merged
drmoisan merged 13 commits into
mainfrom
bug/quickfiler-transactiongate-permit-leak-unexcluded-882
Sep 29, 2026
Merged

drmoisan merged 13 commits into
mainfrom
bug/quickfiler-transactiongate-permit-leak-unexcluded-882

Conversation

@drmoisan

Copy link
Copy Markdown
Owner

Suggested title

fix(quickfiler-test): bound TransactionGate acquisition in UiThreadDispatcherFixture (issue 882)

Summary

  • UiThreadDispatcherFixture.BeginTransactionAsync now acquires the process-wide one-permit TransactionGate with a bounded wait (production default 120000 ms) instead of the unbounded parameterless SemaphoreSlim.WaitAsync().
  • A failed acquisition throws System.TimeoutException carrying the token TRANSACTIONGATE_ACQUIRE_TIMEOUT, naming TransactionGate and the elapsed bound, before any UiThreadDispatcherTransaction exists and before any counter is incremented.
  • An internal overload BeginTransactionAsync(TimeSpan bound) exposes the bound to tests; the parameterless overload delegates to it, so no call site changes.
  • One regression test is added to the existing fixture test class, run under the parallel test regime (Workers 0, ClassLevel).
  • Only two C# files change, both in the QuickFiler.Test project. No shipped add-in production file is modified.

Why

  • Issue Bug: quickfiler-transactiongate-permit-leak-unexcluded #882 carries forward an open question from issue Bug: quickfiler-itemviewer-ui-marshalling-seam #743: the one-permit TransactionGate could leak or late-release a permit, and nothing in the repository excluded that possibility. With the parameterless WaitAsync() a leaked permit produces an unbounded wait that surfaces only as a runner hang.
  • The bound converts that failure mode into a named, attributable test failure. Per the spec's determinism ruling, a real-time bound in test code is permitted when it returns immediately once the condition holds and its expiry is reported as a failure rather than used to reach the expected state; WaitAsync(TimeSpan) satisfies both clauses.
  • The 120000 ms default is twice the 60000 ms MSTest timeout that bounds the longest legitimate hold, and half the four-minute runner hang guard.

What Changed

Test-support fixture

  • QuickFiler.Test/Controllers/QfcItemController.UiThreadDispatcherFixture.cs
    • New constant TransactionGateAcquireTimeoutMs = 120000.
    • Parameterless BeginTransactionAsync() delegates to BeginTransactionAsync(TimeSpan) with the production default.
    • Bounded overload: the contended pre-check stays before the wait; the wait result is branched on; on false a TimeoutException is thrown; _transactionAcquisitions is incremented only on the successful branch.
    • Class and method XML documentation updated to state the bound and the failure type; the UiThreadDispatcherTransaction cref is updated to BeginTransactionAsync() because the method group is now overloaded.

Tests

  • QuickFiler.Test/Controllers/QfcItemController.UiThreadDispatcherFixtureTests.cs
    • New test BeginTransactionAsync_ZeroBoundWhileThisTestHoldsThePermit_ThrowsTimeoutExceptionAndReleasesNothing: holds a transaction, probes the internal overload with TimeSpan.Zero, asserts TimeoutException with the token, asserts acquisitions minus releases equals exactly one, asserts the contended counter advanced, asserts the holder's own disposal does not throw SemaphoreFullException, then round-trips a further transaction through the production entry point.
    • Carries [Timeout(GateTimeoutMs)]; no DoNotParallelize, retry, sleep, delay, or elapsed-time assertion.

Docs and evidence

  • Feature folder docs/features/active/2026-09-13-quickfiler-transactiongate-permit-leak-unexcluded-882/: spec v1.1, atomic plan, research records, baseline / regression-testing / qa-gates evidence, and the policy, code-review, and feature audits.

Architecture / How It Fits Together

  • TransactionGate provides mutual exclusion between install-to-restore transactions over the static UtilitiesCS.UiThread._dispatcher. UiThreadDispatcherTransaction is the only releaser and is constructed only on the branch where the bounded wait returned true, so the failure path has no object to dispose and no release to omit.
  • Every existing using and try/finally around the acquisition sites stays correct unchanged, because a throw from BeginTransactionAsync occurs before the scope is entered or the assignment completes.

Verification

Completed (from committed evidence)

  • Fail-before: compile-level dossier evidence/regression-testing/fail-before-exception.2026-09-29T09-06.md. Before the fix the new test does not compile (error CS1501: No overload for method 'BeginTransactionAsync' takes 1 arguments), so a runtime fail-before run is structurally impossible.
  • Pass-after: the fixture test class run under the parallel CLI runsettings, 8 of 8 passed (evidence/regression-testing/pass-after-scoped-run.md).
  • Final QA loop, clean in one iteration (evidence/qa-gates/qa-loop-closure.md):
    • dotnet tool run csharpier check .: exit 0, 1623 files checked, no drift.
    • msbuild TaskMaster.sln /t:Rebuild /m /p:Configuration=Debug "/p:Platform=Any CPU" /p:EnableNETAnalyzers=true /p:EnforceCodeStyleInBuild=true: Build succeeded., 0 warnings, 0 errors.
    • msbuild TaskMaster.sln /t:Rebuild /m /p:Configuration=Debug "/p:Platform=Any CPU" /p:TreatWarningsAsErrors=true: Build succeeded., 0 warnings, 0 errors.
    • QuickFiler.Test under dotnet-coverage with the parallel CLI runsettings: 1469 total, 1469 passed, 0 failed (baseline 1468 plus the new test) (evidence/qa-gates/mstest-test-result-summary.md).
  • Coverage: recorded as a QuickFiler.Test-only observation (lines 24.42%, branches 23.20%). Both changed files are test code outside the first-party coverage denominator, so no first-party coverage movement is possible; the repository-wide figure is not measured in this PR (evidence/qa-gates/qa-coverage-comparison.md).
  • Feature review: PASS, 0 blocking findings; spec acceptance criteria AC1 to AC12 all checked off.

Recommended

  • Rely on the CI analyzer, nullable, format, and repository-wide test jobs for the full-suite and repository-wide coverage figures.

Backward Compatibility / Migration Notes

  • No breaking change. Both overloads are internal to the test project, and existing callers resolve to the parameterless overload unchanged.
  • No project file, runsettings, or script is modified.

Risks and Mitigations

  • Risk: a legitimately slow holder under heavy machine load could exceed 120000 ms and now fail by name instead of eventually completing. Mitigation: the bound is twice the 60000 ms per-test timeout that already bounds any legitimate hold, so a legitimate holder is terminated by its own timeout first.
  • Risk: the intermittent Transaction_SecondCallerCannotInstallUntilTheFirstRestores test tracked in issue Bug: quickfiler-teardown-review-residuals #823 remains out of scope; it passed in every run recorded here.
  • Rollback: revert the two C# files; no other file depends on the new overload or constant.

Review Guide

  1. QuickFiler.Test/Controllers/QfcItemController.UiThreadDispatcherFixture.cs (the bounded overload and counter ordering).
  2. QuickFiler.Test/Controllers/QfcItemController.UiThreadDispatcherFixtureTests.cs (the new test, lines 395 to 456).
  3. docs/features/active/2026-09-13-quickfiler-transactiongate-permit-leak-unexcluded-882/spec.md and the evidence/ folders. The remaining diff is feature-folder documentation and evidence.

Follow-ups

  • The review recorded non-blocking plan-text residuals (a PowerShell comma-operator precedence defect in the P4-T22 hygiene payload, corrected in-band with positive controls, and a P4-T24 HEAD-listing clause that cannot hold after mid-plan commits). They affect the plan text only, not the delivery.

GitHub Auto-close

  • None (no verified auto-close issue in the PR context; issue 882 is closed separately after merge).

🤖 Generated with Claude Code

https://claude.ai/code/session_01KNZiXntshsLY8vqqCHUvHm

…d atomic plan for bounded TransactionGate acquisition

Preparation for issue 882 in run bugs-2026-09-28 after merging origin/main. Adds the 2026-09-28 research refresh, revises spec.md to v1.1 (parallel-safe single test, counter placement, evidence constraints), and replaces the plan stub with the atomic plan that cleared executor preflight in five rounds.

Co-Authored-By: Claude Opus 5.5 noreply@anthropic.com

Claude-Session: https://claude.ai/code/session_01KNZiXntshsLY8vqqCHUvHm
…and throw TRANSACTIONGATE_ACQUIRE_TIMEOUT on expiry (#882)
@drmoisan
drmoisan merged commit cca2728 into main Sep 29, 2026
6 checks passed
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.

1 participant