fix(host,auditlog): close Host.Tests env-var race + harden hosted-service shutdown (#15) #19

Merged
dohertj2 merged 3 commits from fix/host-tests-env-race-15 into main 2026-07-17 15:07:33 -04:00
Owner

Fixes #15.

What

Closes the Host.Tests env-var race from #15, and fixes a real product defect that verifying it surfaced.

Test harness (#15)

CentralDbTestEnvironment sets five process-wide environment variables, and Program's AddEnvironmentVariables() reads them at an unpredictable point during host boot. With xUnit collection parallelization on, one fixture's teardown could clear a var mid-boot for a sibling — and since the secrets adoption (G-4) three of those keys are ${secret:...} references that fail closed, turning a previously benign empty value into a hard SecretNotFoundException.

  • New HostBootCollection serializes every fixture that boots a real host while depending on that shared state; the narrower ActorSystem collection is folded into it.
  • Site-role fixtures stay parallel — they call Configuration.Sources.Clear(), dropping the env-var provider, so they can't race.
  • CentralDbTestEnvironment now fails fast if two instances are ever live at once, making a future regression deterministic rather than intermittent, and fixture teardown is try/finally so a throwing host teardown can't strand the vars.

Product fix (surfaced while verifying)

The fail-fast guard immediately caught a latent bug: four hosted services (AuditLogPartitionMaintenanceService, SiteAuditBacklogReporter, SiteAuditRetentionService, SiteEventLogFailureCountReporter) shared a copy-pasted lifecycle with two disposed-CTS races — StopAsync cancelling an already-disposed CTS, and StartAsync reading _cts.Token lazily inside the Task.Run lambda. Either faults the task the host awaits during shutdown, which in Host.Tests skipped the fixture's env-var restore and contaminated the rest of the run. All four now capture the token eagerly, tolerate a disposed CTS, and cancel-before-dispose; SiteAuditBacklogReporter also gains the outer OperationCanceledException guard its sibling already had.

Verification

  • Host.Tests: 5 consecutive full runs, 279/279 green, zero occurrences of either exception (was ~1-in-3 red before).
  • AuditLog.Tests (354) / HealthMonitoring.Tests (97) green; each of the 5 new regression tests proven red against the unfixed code, then green.
  • Full solution build: 0 warnings, 0 errors.

Trade-off

Host.Tests runtime goes from ~2m30s to ~4m10s — serializing the six Central-boot classes is most of the assembly's parallelism. Inherent to the fix; flagged for reviewer awareness.

Note

Also carries one orthogonal housekeeping commit (docs: sister-repo index + working notes): a CLAUDE.md sister-repo update plus working-note files, unrelated to the fix. ScadaBridge-docs-issues.md is a large generated report — drop that commit before merge if it shouldn't live in the repo.

Fixes #15. ## What Closes the `Host.Tests` env-var race from #15, and fixes a real product defect that verifying it surfaced. ### Test harness (#15) `CentralDbTestEnvironment` sets five **process-wide** environment variables, and `Program`'s `AddEnvironmentVariables()` reads them at an unpredictable point during host boot. With xUnit collection parallelization on, one fixture's teardown could clear a var mid-boot for a sibling — and since the secrets adoption (G-4) three of those keys are `${secret:...}` references that fail closed, turning a previously benign empty value into a hard `SecretNotFoundException`. - New `HostBootCollection` serializes every fixture that boots a real host while depending on that shared state; the narrower `ActorSystem` collection is folded into it. - Site-role fixtures stay parallel — they call `Configuration.Sources.Clear()`, dropping the env-var provider, so they can't race. - `CentralDbTestEnvironment` now fails fast if two instances are ever live at once, making a future regression deterministic rather than intermittent, and fixture teardown is `try/finally` so a throwing host teardown can't strand the vars. ### Product fix (surfaced while verifying) The fail-fast guard immediately caught a latent bug: four hosted services (`AuditLogPartitionMaintenanceService`, `SiteAuditBacklogReporter`, `SiteAuditRetentionService`, `SiteEventLogFailureCountReporter`) shared a copy-pasted lifecycle with two disposed-CTS races — `StopAsync` cancelling an already-disposed CTS, and `StartAsync` reading `_cts.Token` lazily inside the `Task.Run` lambda. Either faults the task the host awaits during shutdown, which in `Host.Tests` skipped the fixture's env-var restore and contaminated the rest of the run. All four now capture the token eagerly, tolerate a disposed CTS, and cancel-before-dispose; `SiteAuditBacklogReporter` also gains the outer `OperationCanceledException` guard its sibling already had. ## Verification - `Host.Tests`: 5 consecutive full runs, 279/279 green, zero occurrences of either exception (was ~1-in-3 red before). - `AuditLog.Tests` (354) / `HealthMonitoring.Tests` (97) green; each of the 5 new regression tests proven red against the unfixed code, then green. - Full solution build: 0 warnings, 0 errors. ## Trade-off `Host.Tests` runtime goes from ~2m30s to ~4m10s — serializing the six Central-boot classes is most of the assembly's parallelism. Inherent to the fix; flagged for reviewer awareness. ## Note Also carries one orthogonal housekeeping commit (`docs: sister-repo index + working notes`): a CLAUDE.md sister-repo update plus working-note files, unrelated to the fix. `ScadaBridge-docs-issues.md` is a large generated report — drop that commit before merge if it shouldn't live in the repo.
dohertj2 added 3 commits 2026-07-17 15:04:57 -04:00
The host does not guarantee IHostedService.StopAsync is driven before the DI
container is disposed — WebApplicationFactory's teardown reaches Dispose first
— so cancelling the internal CTS from StopAsync threw ObjectDisposedException
and aborted the host's whole shutdown sequence. Four services shared the same
copy-pasted lifecycle and the same two races: StopAsync cancelling an already-
disposed CTS, and StartAsync reading _cts.Token lazily inside the Task.Run
lambda, which faults the loop task the host awaits when Dispose wins that race.

Each service now captures the token on the caller's thread, tolerates a
disposed CTS, and cancels-before-disposing so the loop is always signalled and
its pending Task.Delay sees a cancelled token rather than a dead source.
SiteAuditBacklogReporter also gains the outer OperationCanceledException guard
its sibling SiteAuditRetentionService already carried (arch-review 04 R2, R7),
without which a shutdown landing mid-probe threw TaskCanceledException out of
Host.StopAsync.

Surfaced while verifying the Gitea #15 test-harness fix: in Host.Tests the
aborted teardown skipped the fixture's env-var restore, contaminating every
later test in the run.

Refs: Gitea #15
CentralDbTestEnvironment sets five process-wide environment variables, and
Program's AddEnvironmentVariables() reads them at an unpredictable point during
host boot. With xUnit collection parallelization on, one fixture's teardown
could clear a var mid-boot for a sibling. Since the secrets adoption (G-4)
three of those keys are ${secret:...} references that fail closed, turning a
previously benign empty value into a SecretNotFoundException that aborts the
boot — an intermittent CI failure.

Adds a HostBootCollection that serializes every fixture booting a real host
while depending on that shared state, folding in the narrower "ActorSystem"
collection so its members stay serialized with each other as before. Site-role
fixtures stay parallel: they call Configuration.Sources.Clear(), dropping the
env-var provider, so they cannot participate in the race.

CentralDbTestEnvironment now also fails fast if two instances are ever live at
once, making a regression (a fixture added outside the collection) deterministic
rather than intermittent — this is what surfaced the disposed-CTS defect fixed
in the previous commit. Fixture teardown is try/finally so a throwing host
teardown can no longer strand the vars for the rest of the run.

Cost: Host.Tests runs ~2m30s -> ~4m10s; the serialized Central-boot classes are
most of the assembly's parallelism.

Fixes: Gitea #15
CLAUDE.md: the historian SDK is now owned inside HistorianGateway at
histsdk/ (the separate ~/Desktop/histsdk repo was folded in with history),
so the sister-project list no longer points at a standalone repo.

Also checks in working notes that were sitting untracked in the tree:
deferred.md (remaining deferred work snapshot), ScadaBridge-docs-fixed.md and
ScadaBridge-docs-issues.md (documentation-analysis report output).
dohertj2 merged commit 4d869de9c2 into main 2026-07-17 15:07:33 -04:00
Sign in to join this conversation.
No Reviewers
No Label
1 Participants
Notifications
Due Date
No due date set.
Dependencies

No dependencies set.

Reference: dohertj2/ScadaBridge#19