Compare commits

...

4 Commits

Author SHA1 Message Date
Joseph Doherty a37c304f26 chore(plans): tasks 13-14 complete; as-built note on the hub-connection-factory removal
ci / nightly-windev (push) Has been skipped
ci / windows-x86 (push) Failing after 25s
ci / java (push) Successful in 2m18s
ci / portable (push) Successful in 9m2s
2026-08-16 04:22:34 -04:00
Joseph Doherty 7cfb2e727d fix(dashboard): parallel bounded alarm drains, best-effort disposal catch-alls, ConfigureAwait alignment 2026-08-16 04:20:59 -04:00
Joseph Doherty 3faa272db9 fix(dashboard): bounded alarm drains, feed resubscribe, live pill, drop dead hub factory; doc corrections 2026-08-16 04:12:25 -04:00
Joseph Doherty e1ff05c605 docs: deferred-table closure, stale-comment sweep, as-built execution record 2026-08-15 21:33:28 -04:00
17 changed files with 345 additions and 140 deletions
+14 -10
View File
@@ -215,24 +215,28 @@ Three authorization policies are registered out of these options:
### SignalR hubs
When the dashboard is enabled, three hubs are mapped under `/hubs/*`:
When the dashboard is enabled, three hubs are mapped under `/hubs/*`. They are
the **remote** surface — for clients outside the gateway process. Server-rendered
pages do not use them: a page runs in this process and reads the producing
services through in-process seams (`IDashboardSnapshotFeed`,
`IDashboardSessionEventSubscriber`, `IGatewayAlarmService`) rather than opening a
loopback WebSocket back into its own heap.
- `GET /hubs/snapshot` — pushes `DashboardSnapshot` whenever the snapshot
service produces a new one. Drives every page that inherits
`DashboardPageBase`; replaces the earlier polling loop.
service produces a new one. Idle-gated on connected clients, so it stays
dormant unless a remote client connects.
- `GET /hubs/alarms` — re-broadcasts the `AlarmFeedMessage` stream from the
central alarm monitor to all connected clients (group `__alarms__`).
- `GET /hubs/events` — per-session MxEvent feed. Clients call
`SubscribeSession(sessionId)` to join `session:{id}`. Events are mirrored
from the corresponding gRPC `StreamEvents` call as a fire-and-forget
side-effect; the dashboard only sees events while a gRPC client is also
subscribed to that session.
from the session's own event distributor, gated on `EventsHubViewerRegistry`
so an unwatched session pays nothing.
`GET /hubs/token` (cookie-only) mints a 5-minute data-protected bearer
token for the calling user; the Blazor pages use it via
`DashboardHubConnectionFactory` to authenticate the SignalR connection.
The factory refreshes the token on every (re)connect, so the short lifetime
(SEC-05) is transparent to clients. The token is not server-side revocable;
token for the calling user, so a remote hub client can authenticate the
SignalR connection without forwarding the HttpOnly dashboard cookie. Such a
client is expected to re-fetch on every (re)connect, which makes the short
lifetime (SEC-05) transparent. The token is not server-side revocable;
its short lifetime bounds exposure of a captured token (see
[GatewayDashboardDesign](./GatewayDashboardDesign.md)).
+53 -11
View File
@@ -118,8 +118,11 @@ so it consumes the producing services directly through in-process seams —
`IDashboardSnapshotFeed`, `IDashboardSessionEventSubscriber`, and
`IGatewayAlarmService` — instead of opening a loopback WebSocket back into its
own heap. `DashboardHubConnectionFactory`, the helper a circuit used to open
those connections, stays registered for out-of-tree consumers, but no in-repo
page resolves it.
those connections, has been deleted along with the `Microsoft.AspNetCore.SignalR.Client`
package reference: nothing in this process dials a hub, and a registered-but-unused
client factory only invites a page to reintroduce the loopback. Remote consumers
build their own connection; the hubs, `/hubs/token`, and `HubTokenService` remain
for them.
## Dashboard Data Source
@@ -195,7 +198,27 @@ instead of buffering without bound or stalling the pump.
`DashboardPageBase` seeds `Snapshot` synchronously from
`IDashboardSnapshotService.GetSnapshot()` in `OnInitializedAsync` so the first
render is non-empty, then calls `InvokeAsync(StateHasChanged)` for every snapshot
the feed yields. On dispose it cancels the watch and waits at most **5 seconds**
the feed yields.
A subscription is not a lifetime, so the watch is a loop, not a single enumeration:
the feed detaches its subscribers whenever a pump's source faults or completes, and
a page that treated that as terminal would sit on its last snapshot until the
operator navigated. On any end other than its own cancellation the page waits one
second — honouring its own token, so teardown is not delayed — and resubscribes. The
fault is logged at Warning once per fault *transition*, not once per retry: a feed
that is down stays down for many iterations, and one line per second per open page
is noise. The last rendered snapshot stays on screen throughout, and the next page
load still seeds from `IDashboardSnapshotService.GetSnapshot()`.
That resubscribe is also the feed's primary recovery path, not just the page's:
only a subscriber that finds no live generation starts a pump, so a page coming back
is what restarts the enumeration. `Reset`'s belt-and-braces restart — if subscribers
of *other* generations are still attached when a generation dies, it starts a fresh
pump for them and re-tags them — remains the backstop for the case where no
subscriber is left to drive recovery, but it is no longer the only thing standing
between a faulted feed and a permanently stale page.
On dispose the page cancels the watch and waits at most **5 seconds**
for the loop to drain, logging a warning on timeout. The bound is deliberate: the
loop marshals renders through the renderer's dispatcher and disposal can run on
that same dispatcher, so an unconditional wait would hang on a wedged dispatcher.
@@ -214,6 +237,21 @@ Detaching cancels the pump, disposes the subscription (which releases the viewer
registration and completes the channel, so the pump has an exit even if
cancellation is missed), then drains under its own timeout.
The page's live/offline pill tracks that pump rather than a connection: it is set
live on attach and cleared when the pump exits, through the same dispatcher-owned
identity check the render batch uses, so a stale pump cannot darken the pill of the
subscription that replaced it. Because detach clears the subscription field before
cancelling, a detach-driven exit leaves the pill to the incoming subscription; what
the pill therefore reports is the case it exists for — the channel completing under
a page that is still watching.
`AlarmsPage` owns two loops of its own (the 3 s alarm poll and the provider-status
badge) and bounds their drain at 5 seconds on dispose, for the same reason
`DashboardPageBase` bounds its watch drain: both loops render through the renderer's
dispatcher, and disposal can run on it. The two are drained concurrently, so the
bound on disposal is 5 seconds in total rather than per loop — a wedged dispatcher
blocks both loops at once, and draining them in sequence would time out twice.
### SignalR hubs (remote clients)
Updates for out-of-process clients flow over three SignalR hubs, all guarded by the
@@ -329,9 +367,14 @@ ordering — register before becoming a delivery target, deregister after ceasin
be one — so the widest a race window opens is a redaction clone that reaches
nobody, never a dropped event that was owed to a live viewer.
Redaction happens once per event, not once per audience: `Publish` produces a
single redacted clone and hands that same instance to the in-process subscribers
and to the hub group. In-process delivery runs first and synchronously — it cannot
Redaction happens once per event, not once per audience: with
`Dashboard:ShowTagValues` false (the default) `Publish` produces a single redacted
clone and hands that same instance to the in-process subscribers and to the hub
group. With `ShowTagValues` true there is no clone at all — the original `MxEvent`
instance is handed to both audiences — so the "one clone per event" cost holds only
in the redacting configuration, and in the value-showing one both audiences share a
reference to the session pipeline's own event object. In-process delivery runs first
and synchronously — it cannot
throw, and it must not be skipped by the guard clause around the hub send — into
per-subscriber bounded drop-oldest channels, so a page that falls behind loses its
oldest queued events rather than blocking the session's event pipeline.
@@ -715,11 +758,10 @@ dashboard mints short-lived bearer tokens for the connection:
5. The hubs' `[Authorize(Policy = HubClientsPolicy)]` accepts the resulting
identity.
`DashboardHubConnectionFactory` (scoped to the Blazor circuit) wraps the
HubConnectionBuilder and supplies a fresh token via `AccessTokenProvider` on
every (re)connect, so the short 5-minute lifetime is transparent to whoever uses
it. It remains registered, but no in-repo page opens a hub connection any more;
external clients implement the equivalent refresh themselves.
There is no in-repo hub client: the helper that once wrapped `HubConnectionBuilder`
for a circuit was deleted when the pages moved to the in-process seams. An external
client re-fetches `/hubs/token` on every (re)connect itself, which is what makes the
short 5-minute lifetime transparent.
Caveat — logout does not revoke outstanding tokens. Logout clears the dashboard
cookie, but hub bearer tokens are self-contained, data-protection-encrypted, and
+5 -1
View File
@@ -136,7 +136,11 @@ cost stays bounded: a pure-event pass — the event hot path — still pays
exactly one flush however many frames drain together, a run of control
frames still pays one for the whole run (never one per heartbeat, the
syscall-per-frame cost the coalescing removed), and only a pass that actually
mixes both classes pays a second.
mixes both classes pays a second. Mixed passes are not exotic: command replies
travel at Control priority alongside heartbeats and faults, so a session under
sustained command traffic concurrent with event streaming can hit them
routinely. The cost stays bounded either way — one extra flush per class
transition present in the pass, not per frame.
One consequence of the boundary flush is worth stating: a control frame whose
run has already been flushed and completed is out of the drain's
+65 -1
View File
@@ -130,7 +130,7 @@
1. `IDashboardSnapshotFeed` (singleton): `IAsyncEnumerable<DashboardSnapshot> WatchAsync(CancellationToken ct)`. Internally: per-subscriber `Channel<DashboardSnapshot>` with capacity 1 and `BoundedChannelFullMode.DropOldest` (a dashboard viewer only ever wants the latest snapshot; a slow circuit must never buffer unboundedly or stall others).
2. **Idle gating (the invariant this task must not lose):** the feed enumerates `IDashboardSnapshotService.WatchSnapshotsAsync` on a background task started when the subscriber count goes 0→1 and cancelled when it goes 1→0. While zero subscribers, the feed holds no timer and builds no snapshot. Guard subscriber add/remove with a plain lock; restart cleanly on resubscribe (mirror the start/stop discipline of `GatewayAlarmMonitor.StreamAsync` registration, `GatewayAlarmMonitor.cs:739-752`). If the underlying watch throws or completes, complete all subscriber channels with the error and reset so the next subscriber restarts it (mirror `DashboardSnapshotPublisher.ExecuteAsync`'s reconnect-after-delay posture, but per-feed).
3. `DashboardPageBase`: remove the HubConnection path (`:62` and the factory usage); keep the synchronous first render via `snapshotService.GetSnapshot()` (`:37`); then a background loop `await foreach (var s in feed.WatchAsync(_cts.Token)) { Snapshot = s; await InvokeAsync(StateHasChanged); }` started in `OnAfterRenderAsync(firstRender)` or `OnInitializedAsync` (match current lifecycle), cancelled + awaited in `DisposeAsync`. Update the class XML doc that narrates the hub subscription history (`:7-14`).
4. Hubs, `DashboardSnapshotPublisher`, `DashboardSnapshotHubConnectionCounter`, `DashboardHubConnectionFactory`, and `/hubs/token` all stay — they remain the remote/external surface. Do not touch them.
4. Hubs, `DashboardSnapshotPublisher`, `DashboardSnapshotHubConnectionCounter`, `DashboardHubConnectionFactory`, and `/hubs/token` all stay — they remain the remote/external surface. Do not touch them. *(As-built deviation, integration-fix commit: once the pages stopped using it, `DashboardHubConnectionFactory` had zero consumers, so the final integration review had it deleted along with the `Microsoft.AspNetCore.SignalR.Client` package reference. Hubs, publisher, counter, and `/hubs/token` remain the remote surface as specified.)*
5. Auth: the pages are mapped behind `ViewerPolicy` (`DashboardEndpointRouteBuilderExtensions.cs:136`), which remains the gate for in-process consumption; add one comment on `WatchAsync` saying so.
6. Tests (`DashboardSnapshotFeedTests`): (a) zero subscribers → underlying service's `WatchSnapshotsAsync` never enumerated (fake service counts enumerations/`MoveNextAsync`); (b) first subscriber starts exactly one enumeration; two subscribers share it; (c) last unsubscribe cancels it; resubscribe restarts it; (d) slow subscriber observes latest-wins (push 3 snapshots, read 1, it is the newest) while a fast subscriber sees all; (e) underlying fault completes subscribers with the error and a fresh subscriber restarts.
@@ -308,3 +308,67 @@ Push the branch to origin, then on windev (`ssh windev`, clone `C:\build\mxacces
| Structural alarm-truncation degraded-status signal | Contract-level design (proto change candidate) — separate effort. |
| SEC-25 per-session dashboard event ACL | Security roadmap item; Task 6 deliberately preserves the current posture. |
| `MxAccessWriteCompletionCache` clone | Different lifecycle than the value cache; consciously kept (Task 12.5). |
---
## As-built notes (execution record)
Where the delivered work differs from the task text above, or where the route to it
is worth keeping, this is the record.
**Task 3 — `ReadEventsAsync` retained.** The method was not removed after
`MapWorkerEventsAsync` inlined the read-then-map chain: a second caller reaches it
through `ISessionManager.ReadEventsAsync`. That interface member itself has zero
production call sites — only test fakes implement and exercise it. Deleting it is a
mechanical but wide change (~15 test-fake touches), so it is recorded as a follow-up
rather than done here.
**Task 5 — dashboard event feed, two review rounds.** Review caught two races that
the first cut did not have. First, subscription lifetime: subscriptions are now
generation-tagged, a generation ends at the time the fault is *observed* (not when it
is raised), `Reset` is scoped to the dying generation so it cannot cancel its
successor, and a backstop restart covers the case where no subscriber is left to
drive recovery. Second, `UnsubscribeAsync` needed a generation-scoped idle gate so a
teardown for an old generation cannot tear down the new one. Both fixes are pinned by
tests verified against mutations of the fixed code.
**Task 6 — subscribe API placement.** The subscribe surface lives on
`IDashboardSessionEventSubscriber`, with DI forwarding to a single instance so every
consumer shares one feed. Batches that arrive for a session the renderer has already
moved off are dropped by a subscription identity check inside the renderer dispatch,
which is what makes a stale batch harmless rather than a cross-session leak.
**Task 10 — the delivered property is the delivery point, not awaited latency.** The
spec asked for control-frame completion to be observable before the pass's event
writes. That is unachievable in the enqueue-then-contend shape: a caller that loses
the write-lock race does not run again until the winning drainer releases the lock,
so its `await` cannot return early no matter when its frame completes. What shipped
is the honest half: control frames are written *and flushed* at the class-transition
boundary, so the priority class governs the frame's delivery point rather than only
its byte order. Getting the awaited-latency win too requires unparking the lock-race
loser from the winner's pass — a change to the write-lock shape, recorded as a
follow-up. One extra `FlushFileBuffers` per mixed pass is the accepted cost.
**Task 11 — teardown ordering and unconditional fault observation.** Teardown disposes
the session-owned transport first, then observes the read that dispose abandoned.
Fault observation is unconditional — a `ContinueWith(..., TaskContinuationOptions.OnlyOnFaulted)`
continuation, so the budget that bounds the wait is diagnostics-only and can never be
the reason a fault goes unobserved. The same continuation covers heartbeat and drain
overrun. `DisposeTransportStream` is exception-total: no dispose path can throw out of
teardown.
**Task 12 — three clones removed, plan rationale corrected in-code.** All three
`OnDataChange` value-cache clones are gone. The plan's stated reason for keeping the
`MxAccessWriteCompletionCache` clone ("different lifecycle than the value cache") is
wrong and was corrected where the code documents it: the clone is kept on
*provenance* grounds — the cached payload comes from a caller-supplied object the
worker does not own — not on lifecycle grounds.
**Task 13 — windev verification.** Solution build 0 warnings / 0 errors after
clearing stale `Contracts` `obj` artifacts (an infrastructure problem on the box, not
a regression from this branch). Worker x86: 509/509 (+8 new). Gateway: 1059/1059,
including `SecretsStorePathGuardTests` — the first fully green Windows gateway run,
that suite having been red before this branch. One load flake
(`InvokeAsync_WhenWorkerHandshakingThenReadyWithinTimeout_Succeeds`) passed in
isolation and on re-run, consistent with the load-sensitivity caveat documented in
`docs/GatewayTesting.md`.
@@ -13,8 +13,8 @@
{ "id": 10, "subject": "Task 10: Control-frame completion decoupling in WorkerFrameWriter", "status": "completed", "blockedBy": [9] },
{ "id": 11, "subject": "Task 11: Worker pipe-read teardown — dispose-to-unblock and observe", "status": "completed", "blockedBy": [9] },
{ "id": 12, "subject": "Task 12: Value-cache clone removal per the aliasing audit", "status": "completed", "blockedBy": [9] },
{ "id": 13, "subject": "Task 13: Phase B gate — windev full verification", "status": "pending", "blockedBy": [10, 11, 12] },
{ "id": 14, "subject": "Task 14: Wrap-up — deferred table closure, docs sweep, final review", "status": "pending", "blockedBy": [13] }
{ "id": 13, "subject": "Task 13: Phase B gate — windev full verification", "status": "completed", "blockedBy": [10, 11, 12] },
{ "id": 14, "subject": "Task 14: Wrap-up — deferred table closure, docs sweep, final review", "status": "completed", "blockedBy": [13] }
],
"lastUpdated": "2026-08-15T00:00:00Z"
"lastUpdated": "2026-08-16T00:00:00Z"
}
@@ -619,6 +619,11 @@ Commit anything found: `docs: remediation plan doc sweep`
| Event-path triple async-iterator flattening | LOW-rated; touches the most invariant-dense code in the gateway for two `MoveNextAsync` hops per event. Reconsider after Tasks 3/4 land and if profiling still shows it. |
| `SessionEventDistributor._subscribers` `ConcurrentDictionary` → plain `Dictionary` (Task 25 / Task 3 review) | Every mutation is already inside `_lifecycleLock`, so the concurrent type buys nothing. Behavior-neutral refactor with no measurable win, proposed after the windev gate was already green — not worth re-running the verification matrix for. Comment cleanups from the same review landed; this swap did not. |
All six rows above were resolved on 2026-08-15 by the follow-up plan
[`docs/plans/2026-08-15-deferred-remediation.md`](2026-08-15-deferred-remediation.md)
(branch `perf/deferred-remediation`), which also fixed the pre-existing Windows
`SecretsStorePathGuardTests` failure.
## Execution notes for the orchestrator
- Branch: `git checkout -b perf/review-remediation` before Task 1.
+11 -7
View File
@@ -117,13 +117,17 @@ project without binding to a metrics exporter.
effective configuration into immutable DTOs for read-only dashboard rendering.
The Blazor Server dashboard mounts at the host root and renders those snapshots
at `/`, `/sessions`, `/workers`, `/events`, `/galaxy`, `/alarms`, `/apikeys`,
and `/settings`. Pages connect to `/hubs/snapshot` (a SignalR hub published by
`DashboardSnapshotPublisher`) and refresh on every push instead of polling.
`/hubs/alarms` broadcasts `AlarmFeedMessage` values from the central alarm
monitor; `/hubs/events` mirrors per-session `MxEvent` traffic from
`EventStreamService` to clients subscribed to `session:{id}`. The dashboard
uses local Bootstrap CSS and JavaScript plus a small local stylesheet; it does
not use a Blazor UI component library.
and `/settings`. Pages run inside this process, so they consume the producing
services directly through in-process seams — `IDashboardSnapshotFeed`,
`IDashboardSessionEventSubscriber`, and `IGatewayAlarmService` — and re-render on
every update instead of polling. The three SignalR hubs are the **remote**
surface, for clients outside the gateway process: `/hubs/snapshot` pushes
`DashboardSnapshot` from `DashboardSnapshotPublisher`, `/hubs/alarms` broadcasts
`AlarmFeedMessage` values from the central alarm monitor, and `/hubs/events`
mirrors per-session `MxEvent` traffic to clients subscribed to `session:{id}`.
No in-repo page opens a hub connection. The dashboard uses local Bootstrap CSS
and JavaScript plus a small local stylesheet; it does not use a Blazor UI
component library.
`/browse` walks the `IGalaxyHierarchyCache` tree and reads subscribed tag
values live through `IDashboardLiveDataService`, which owns one shared,
@@ -21,6 +21,13 @@ public abstract class DashboardPageBase : ComponentBase, IAsyncDisposable
/// </summary>
private static readonly TimeSpan WatchDrainTimeout = TimeSpan.FromSeconds(5);
/// <summary>
/// Delay between a feed subscription ending and the resubscribe that replaces it.
/// Long enough that a feed failing on every attempt cannot spin, short enough that
/// the page is stale for about a snapshot interval rather than until navigation.
/// </summary>
private static readonly TimeSpan ResubscribeDelay = TimeSpan.FromSeconds(1);
private readonly CancellationTokenSource _watchCancellation = new();
private Task? _watchTask;
@@ -88,31 +95,71 @@ public abstract class DashboardPageBase : ComponentBase, IAsyncDisposable
GC.SuppressFinalize(this);
}
/// <summary>
/// Renders every snapshot the feed yields, resubscribing whenever the subscription ends
/// for any reason other than this page going away.
/// </summary>
/// <remarks>
/// The feed detaches a subscriber when its pump's source faults or completes, so a single
/// enumeration is not a lifetime: without the outer loop the first fault froze the page on
/// its last snapshot until the operator navigated. Resubscribing is also what restarts the
/// feed — only a subscriber that finds no live generation starts a pump — so the page is
/// the recovery path, not merely its beneficiary.
/// </remarks>
/// <param name="cancellationToken">Cancelled by <see cref="DisposeAsync"/> when the page goes away.</param>
/// <returns>A task that completes when the page is disposed.</returns>
private async Task WatchSnapshotsAsync(CancellationToken cancellationToken)
{
try
// One log line per fault *transition*, not per retry: a feed that is down stays down
// for many iterations, and a warning per second per open page is noise, not signal.
bool faultLogged = false;
while (!cancellationToken.IsCancellationRequested)
{
await foreach (DashboardSnapshot snapshot in SnapshotFeed
.WatchAsync(cancellationToken)
.ConfigureAwait(false))
try
{
Snapshot = snapshot;
await InvokeAsync(StateHasChanged).ConfigureAwait(false);
await foreach (DashboardSnapshot snapshot in SnapshotFeed
.WatchAsync(cancellationToken)
.ConfigureAwait(false))
{
Snapshot = snapshot;
faultLogged = false;
await InvokeAsync(StateHasChanged).ConfigureAwait(false);
}
}
catch (OperationCanceledException)
{
// The page is going away.
return;
}
catch (Exception error) when (!faultLogged)
{
// The last rendered snapshot stays on screen while the retry runs, and the
// snapshot service keeps serving GetSnapshot() for the next page load.
faultLogged = true;
Logger?.LogWarning(
error,
"Live snapshot updates failed for dashboard page {Page}; retrying every {Delay}. "
+ "It keeps the last rendered snapshot until they resume.",
GetType().Name,
ResubscribeDelay);
}
catch (Exception)
{
// Same fault, already reported above; the retry below is unconditional.
}
// The enumeration's own disposal (run by the await foreach on every exit path)
// is what releases the dead subscription, so the delay below is only paced —
// there is nothing left of the old subscription to unwind here.
try
{
await Task.Delay(ResubscribeDelay, cancellationToken).ConfigureAwait(false);
}
catch (OperationCanceledException)
{
return;
}
}
catch (OperationCanceledException)
{
// The page is going away.
}
catch (Exception error)
{
// The feed is best-effort: the last rendered snapshot stays on screen and the
// snapshot service keeps serving GetSnapshot() for the next page load. Logged
// once here, on the way out of the loop — never per snapshot.
Logger?.LogWarning(
error,
"Live snapshot updates ended for dashboard page {Page}; it keeps the last rendered snapshot.",
GetType().Name);
}
}
}
@@ -168,6 +168,13 @@
private int _maxSeverity = 1000;
private string _search = string.Empty;
// Upper bound on waiting for either background loop while disposing, mirroring
// DashboardPageBase's snapshot-watch drain: both loops marshal renders through the
// renderer's dispatcher and disposal can run on that same dispatcher, so the wait is
// bounded rather than unconditional — an unconditional one hangs teardown for good on
// a wedged dispatcher.
private static readonly TimeSpan LoopDrainTimeout = TimeSpan.FromSeconds(5);
private readonly CancellationTokenSource _cts = new();
private Task? _pollTask;
@@ -316,6 +323,14 @@
catch (OperationCanceledException)
{
}
catch
{
// Catch-all for the same reason ProviderStatusLoopAsync has one: a teardown race
// can fault InvokeAsync (a disposed renderer) after cancellation has already been
// requested. Letting that fault the task would surface it out of the drain in
// DisposeAsync, skipping _cts.Dispose(). The loop ends here and the page holds its
// last rendered rows — it is being disposed or has nothing left to poll with.
}
}
private async Task RefreshAlarmsAsync()
@@ -334,31 +349,41 @@
/// <inheritdoc />
public async ValueTask DisposeAsync()
{
await _cts.CancelAsync();
await _cts.CancelAsync().ConfigureAwait(false);
if (_pollTask is not null)
{
try
{
await _pollTask;
}
catch (OperationCanceledException)
{
}
}
if (_providerStatusTask is not null)
{
try
{
await _providerStatusTask;
}
catch (OperationCanceledException)
{
}
}
// Drained together, not one after the other: the wedged dispatcher this bound exists
// for blocks both loops at once, so sequential drains would time out twice and make
// the real bound 10 seconds. DrainAsync tolerates a null task.
await Task.WhenAll(DrainAsync(_pollTask), DrainAsync(_providerStatusTask))
.ConfigureAwait(false);
_cts.Dispose();
GC.SuppressFinalize(this);
}
// The accepted cost of the bound is an abandoned loop that keeps its alarm-service
// subscription (and its poll timer) until it does unwind; the alternative — waiting
// forever on a dispatcher that is not draining — wedges the circuit teardown itself.
private static async Task DrainAsync(Task? loop)
{
if (loop is null)
{
return;
}
try
{
await loop.WaitAsync(LoopDrainTimeout).ConfigureAwait(false);
}
catch (TimeoutException)
{
}
catch (OperationCanceledException)
{
}
catch
{
// Other disposal-time errors are best-effort.
}
}
}
@@ -1,5 +1,9 @@
@page "/sessions/{SessionId}"
@inherits DashboardPageBase
@* Load-bearing: DisposeAsync below hides the base method with `new`, so Blazor only calls
it because this directive re-declares IAsyncDisposable on the derived component. Drop
this line and the base's DisposeAsync runs instead — the event subscription and pump
leak, silently. *@
@implements IAsyncDisposable
@using ZB.MOM.WW.MxGateway.Contracts.Proto
@using ZB.MOM.WW.MxGateway.Server.Dashboard.Hubs
@@ -358,6 +362,43 @@ else
// Either the renderer went away mid-dispatch, or the drain below timed out
// and disposed the cancellation source this loop is still reading.
}
finally
{
await MarkDisconnectedAsync(subscription).ConfigureAwait(false);
}
}
// This pump is the only thing feeding the "live" pill, so the pill goes dark the moment
// the pump stops — the subscription's channel completing under a page that is still
// watching (the broadcaster dropped it, the session ended) is exactly what the pill
// exists to show, and without this it read "live" until navigation.
// A pump whose subscription has already been replaced must not touch it: the newer
// subscription's pump owns the pill now. Same dispatcher-owned identity check the
// render batch uses — and detach nulls _eventSubscription before cancelling, so a
// detach-driven exit correctly falls through here without repainting.
private async Task MarkDisconnectedAsync(IDashboardEventSubscription subscription)
{
try
{
await InvokeAsync(() =>
{
if (!ReferenceEquals(_eventSubscription, subscription))
{
return;
}
_eventsConnected = false;
StateHasChanged();
}).ConfigureAwait(false);
}
catch (ObjectDisposedException)
{
// The renderer went away; there is no pill left to repaint.
}
catch (OperationCanceledException)
{
// The circuit is tearing down; same.
}
}
private async Task DetachEventsAsync()
@@ -401,6 +442,11 @@ else
: string.Empty;
}
// `new` hides DashboardPageBase.DisposeAsync rather than overriding it (the base method
// is not virtual), so this runs only via the IAsyncDisposable interface slot the
// `@implements IAsyncDisposable` directive at the top of this file re-declares on the
// derived type. Remove either half and disposal silently resolves to the base method:
// the snapshot watch is cancelled, the event subscription and pump are not.
public new async ValueTask DisposeAsync()
{
await DetachEventsAsync();
@@ -45,8 +45,10 @@ public static class DashboardServiceCollectionExtensions
services.AddSingleton<DashboardApiKeyAuthorization>();
services.AddSingleton<IDashboardApiKeyManagementService, DashboardApiKeyManagementService>();
services.AddSingleton<IDashboardSessionAdminService, DashboardSessionAdminService>();
// Singleton, and the only consumer scope left is HubTokenAuthenticationHandler plus
// the /hubs/token endpoint: server-rendered pages read the in-process feeds, so
// nothing in this process builds a hub connection or needs a token for one.
services.AddSingleton<HubTokenService>();
services.AddScoped<Hubs.DashboardHubConnectionFactory>();
services.AddScoped<IDashboardBrowseService, DashboardBrowseService>();
// Singleton: EventsHub instances are transient (one per hub invocation), so the
// subscriber bookkeeping they share with the broadcaster must outlive them.
@@ -14,10 +14,12 @@ namespace ZB.MOM.WW.MxGateway.Server.Dashboard;
/// <remarks>
/// This service is registered as a singleton in
/// <see cref="DashboardServiceCollectionExtensions.AddGatewayDashboard"/> and
/// is shared by two consumer scopes: <c>DashboardHubConnectionFactory</c>
/// (scoped, per-circuit; calls <see cref="Issue"/> from the cookie-authenticated
/// dashboard) and <c>HubTokenAuthenticationHandler</c> (transient, per-request;
/// calls <see cref="Validate"/> from the SignalR negotiate / connection path).
/// is shared by two consumer scopes: the <c>/hubs/token</c> endpoint (calls
/// <see cref="Issue"/> for a cookie-authenticated caller) and
/// <c>HubTokenAuthenticationHandler</c> (transient, per-request; calls
/// <see cref="Validate"/> from the SignalR negotiate / connection path). Both
/// serve external/remote hub consumers — server-rendered dashboard pages read the
/// in-process feeds and never mint a hub token.
/// The underlying <see cref="ITimeLimitedDataProtector"/> is thread-safe, so
/// minting and validating concurrently from any number of callers is safe;
/// future maintainers should preserve the singleton lifetime to keep the
@@ -31,9 +33,10 @@ public sealed class HubTokenService
// revocable. A short lifetime bounds the exposure window of a token captured from a proxy
// or log after logout (the cookie is cleared on logout, but outstanding tokens are not), and
// bounds how long a stale role set survives a role change. Five minutes is transparent to
// clients because DashboardHubConnectionFactory mints a fresh token on every (re)connect;
// see docs/GatewayDashboardDesign.md. Heavier jti-denylist revocation is deliberately
// deferred until per-session hub ACLs land, when tokens gain session binding.
// clients that re-fetch from /hubs/token on every (re)connect, which is what a remote hub
// consumer is expected to do; see docs/GatewayDashboardDesign.md. Heavier jti-denylist
// revocation is deliberately deferred until per-session hub ACLs land, when tokens gain
// session binding.
internal static readonly TimeSpan TokenLifetime = TimeSpan.FromMinutes(5);
private readonly ITimeLimitedDataProtector _protector;
@@ -1,39 +0,0 @@
using Microsoft.AspNetCore.Components;
using Microsoft.AspNetCore.Components.Authorization;
using Microsoft.AspNetCore.SignalR.Client;
namespace ZB.MOM.WW.MxGateway.Server.Dashboard.Hubs;
/// <summary>
/// Client-side helper that builds a <see cref="HubConnection"/> targeted at a
/// dashboard hub. Mints a fresh data-protected bearer token via
/// <see cref="HubTokenService"/> on every (re)connect so the connection
/// authenticates against <see cref="DashboardAuthenticationDefaults.HubAuthenticationScheme"/>
/// without needing to forward the browser's HttpOnly cookie.
/// </summary>
public sealed class DashboardHubConnectionFactory(
NavigationManager navigation,
HubTokenService tokens,
AuthenticationStateProvider authState)
{
/// <summary>Creates a new hub connection to the specified hub path.</summary>
/// <param name="hubPath">The relative hub path (e.g., "/hubs/snapshot").</param>
/// <returns>A configured hub connection with automatic reconnection and token authentication.</returns>
public HubConnection Create(string hubPath)
{
ArgumentException.ThrowIfNullOrWhiteSpace(hubPath);
Uri hubUrl = navigation.ToAbsoluteUri(hubPath);
return new HubConnectionBuilder()
.WithUrl(hubUrl, options =>
{
options.AccessTokenProvider = async () =>
{
AuthenticationState state = await authState.GetAuthenticationStateAsync().ConfigureAwait(false);
return tokens.Issue(state.User);
};
})
.WithAutomaticReconnect()
.Build();
}
}
@@ -56,9 +56,9 @@ public delegate void SubscriberOverflowHandler(bool isOnlySubscriber, bool isInt
/// <see cref="Func{T, TResult}"/> producing an
/// <see cref="IAsyncEnumerable{T}"/> of already-mapped public
/// <see cref="MxEvent"/>s, given a <see cref="CancellationToken"/>. This is the
/// cleanest seam: it can pass
/// <c>ct =&gt; session.ReadEventsAsync(ct).Select(mapper.MapEvent)</c> (or a
/// channel reader's <c>ReadAllAsync</c>), while unit tests pass a plain
/// cleanest seam: production passes <c>GatewaySession.MapWorkerEventsAsync</c>,
/// which reads the worker event channel and maps each frame in one iterator
/// (inlining what used to be a read-then-<c>Select</c> chain), while unit tests pass a plain
/// channel reader's <c>ReadAllAsync</c> with no real session. The pump owns the
/// single consumption of this enumerable; fan-out happens on the public
/// <see cref="MxEvent"/> after mapping, mirroring today's
@@ -751,9 +751,9 @@ public sealed class SessionEventDistributor : IAsyncDisposable
}
// Disconnect ONLY this subscriber: it is already out of the fan-out set (removed above),
// so complete its channel with the overflow fault. Its gRPC reader's MoveNextAsync then
// throws the SessionManagerException, which EventStreamService surfaces to the client
// exactly as the pre-epic per-RPC overflow did. The pump and every other subscriber are
// so complete its channel with the overflow fault. EventStreamService consumes the channel
// directly, so its next WaitToReadAsync throws the SessionManagerException, which it surfaces
// to the client exactly as the pre-epic per-RPC overflow did. The pump and every other subscriber are
// untouched. This runs even when the handler above threw — the subscriber must never be
// left attached with an un-completed channel.
subscriber.Channel.Writer.TryComplete(new SessionManagerException(
@@ -27,7 +27,6 @@
<PackageReference Include="Serilog.AspNetCore" Version="10.0.0" />
<PackageReference Include="Serilog.Sinks.Console" Version="6.1.1" />
<PackageReference Include="Serilog.Sinks.File" Version="7.0.0" />
<PackageReference Include="Microsoft.AspNetCore.SignalR.Client" Version="10.0.0" />
<PackageReference Include="Microsoft.Data.Sqlite" Version="10.0.7" />
<!-- Security pin: GHSA-2m69-gcr7-jv3q. Microsoft.Data.Sqlite pulls the vulnerable
native 2.1.11; 2.1.12 patches it. Bumping Sqlite does not clear it. -->
@@ -27,23 +27,20 @@ public sealed class DashboardHubsRegistrationTests
endpoint.Metadata.GetMetadata<IEndpointNameMetadata>()?.EndpointName == "DashboardHubToken");
}
/// <summary>Verifies that dashboard build registers hub token service and connection factory.</summary>
/// <summary>
/// Verifies that dashboard build registers the hub token service. It is a singleton
/// shared by the <c>/hubs/token</c> endpoint and <c>HubTokenAuthenticationHandler</c>;
/// there is deliberately no client-side hub-connection factory to resolve, because
/// server-rendered pages read the in-process feeds instead of dialling their own hubs.
/// </summary>
/// <returns>A task that represents the asynchronous operation.</returns>
[Fact]
public async Task Build_WhenDashboardEnabled_RegistersHubTokenServiceAndConnectionFactory()
public async Task Build_WhenDashboardEnabled_RegistersHubTokenService()
{
await using WebApplication app = GatewayApplication.Build([]);
// HubTokenService is singleton; DashboardHubConnectionFactory is scoped
// (it captures NavigationManager and AuthenticationStateProvider which
// are themselves per-circuit).
HubTokenService tokens = app.Services.GetRequiredService<HubTokenService>();
Assert.NotNull(tokens);
using IServiceScope scope = app.Services.CreateScope();
DashboardHubConnectionFactory factory = scope.ServiceProvider
.GetRequiredService<DashboardHubConnectionFactory>();
Assert.NotNull(factory);
}
/// <summary>
@@ -179,7 +179,9 @@ public sealed class WorkerFrameWriter
/// remaining events are written, so a batch in flight does not delay a control frame's delivery.
/// Every frame's "written and flushed before completion" contract is unchanged. An event batch that
/// a control frame cuts into therefore pays one extra flush; an uninterrupted batch still pays
/// exactly one.
/// exactly one. Mixed passes are not exotic — command replies are Control priority too, so
/// sustained command traffic concurrent with event streaming can hit them routinely; the cost
/// stays bounded at one extra flush per class transition present in the pass.
/// </summary>
/// <param name="envelopes">Envelopes to write, in order.</param>
/// <param name="priority">Scheduling priority for the whole batch.</param>