Merge perf/deferred-remediation: deferred perf findings closed — in-process dashboard feeds, worker teardown/frame-writer fixes, value-cache clone removal, first green Windows secrets test
ci / nightly-windev (push) Has been skipped
ci / windows-x86 (push) Successful in 1m22s
ci / java (push) Successful in 2m37s
ci / portable (push) Successful in 9m0s

This commit is contained in:
Joseph Doherty
2026-08-16 04:38:53 -04:00
39 changed files with 3728 additions and 403 deletions
+14 -10
View File
@@ -215,24 +215,28 @@ Three authorization policies are registered out of these options:
### SignalR hubs ### 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 - `GET /hubs/snapshot` — pushes `DashboardSnapshot` whenever the snapshot
service produces a new one. Drives every page that inherits service produces a new one. Idle-gated on connected clients, so it stays
`DashboardPageBase`; replaces the earlier polling loop. dormant unless a remote client connects.
- `GET /hubs/alarms` — re-broadcasts the `AlarmFeedMessage` stream from the - `GET /hubs/alarms` — re-broadcasts the `AlarmFeedMessage` stream from the
central alarm monitor to all connected clients (group `__alarms__`). central alarm monitor to all connected clients (group `__alarms__`).
- `GET /hubs/events` — per-session MxEvent feed. Clients call - `GET /hubs/events` — per-session MxEvent feed. Clients call
`SubscribeSession(sessionId)` to join `session:{id}`. Events are mirrored `SubscribeSession(sessionId)` to join `session:{id}`. Events are mirrored
from the corresponding gRPC `StreamEvents` call as a fire-and-forget from the session's own event distributor, gated on `EventsHubViewerRegistry`
side-effect; the dashboard only sees events while a gRPC client is also so an unwatched session pays nothing.
subscribed to that session.
`GET /hubs/token` (cookie-only) mints a 5-minute data-protected bearer `GET /hubs/token` (cookie-only) mints a 5-minute data-protected bearer
token for the calling user; the Blazor pages use it via token for the calling user, so a remote hub client can authenticate the
`DashboardHubConnectionFactory` to authenticate the SignalR connection. SignalR connection without forwarding the HttpOnly dashboard cookie. Such a
The factory refreshes the token on every (re)connect, so the short lifetime client is expected to re-fetch on every (re)connect, which makes the short
(SEC-05) is transparent to clients. The token is not server-side revocable; lifetime (SEC-05) transparent. The token is not server-side revocable;
its short lifetime bounds exposure of a captured token (see its short lifetime bounds exposure of a captured token (see
[GatewayDashboardDesign](./GatewayDashboardDesign.md)). [GatewayDashboardDesign](./GatewayDashboardDesign.md)).
+217 -43
View File
@@ -98,6 +98,7 @@ ZB.MOM.WW.MxGateway.Server
StatusBadge.razor StatusBadge.razor
FaultList.razor FaultList.razor
DashboardSnapshotService.cs DashboardSnapshotService.cs
DashboardSnapshotFeed.cs
DashboardAuthorizationHandler.cs DashboardAuthorizationHandler.cs
DashboardAuthenticator.cs DashboardAuthenticator.cs
DashboardApiKeyAuthorization.cs DashboardApiKeyAuthorization.cs
@@ -110,9 +111,18 @@ ZB.MOM.WW.MxGateway.Server
``` ```
The dashboard exposes three named SignalR hubs in addition to Blazor Server's The dashboard exposes three named SignalR hubs in addition to Blazor Server's
internal circuit; pages connect to those hubs from within the circuit via the internal circuit. The hubs are the **remote** surface: they publish snapshot,
`DashboardHubConnectionFactory` helper. The hubs publish snapshot, alarm, and alarm, and per-session event updates to clients outside the gateway process.
per-session event updates that the pages render in place of polling. Server-rendered Blazor pages do not use them. A page runs inside this process,
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, 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 ## Dashboard Data Source
@@ -159,7 +169,104 @@ gateway internals.
## Realtime Updates ## Realtime Updates
Updates flow over three SignalR hubs, all guarded by the Realtime data reaches two audiences over two seams:
- **in-process**, for the server-rendered Blazor pages, which run inside the
gateway process and read the producing services directly;
- **SignalR hubs**, for clients outside the process.
Pages originally took the hub path too, which put a loopback WebSocket, a
hub-token mint, and a serialize/deserialize round trip between a Blazor component
and an object already in its own heap. The in-process seams remove that hop. The
hubs stay for the audience that genuinely needs a wire.
### In-process page feeds
| Page | Seam | Producer |
|---|---|---|
| every page deriving from `DashboardPageBase` | `IDashboardSnapshotFeed.WatchAsync` | `DashboardSnapshotFeed` (singleton) multicasting one `IDashboardSnapshotService.WatchSnapshotsAsync` enumeration |
| `SessionDetailsPage` | `IDashboardSessionEventSubscriber.Subscribe(sessionId)` | `DashboardEventBroadcaster` — the same singleton the session mirror publishes to, registered behind both interfaces |
| `AlarmsPage` | `IGatewayAlarmService.StreamAsync` | the central alarm monitor, **provider status only**; the alarm rows still come from the 3 s `QueryAlarmsAsync` poll |
The snapshot feed multicasts rather than handing each page its own enumeration:
`WatchSnapshotsAsync` is not multicast on its own — each enumeration owns a timer
and builds its own snapshot per tick — so a subscription per page would multiply
the snapshot cost by the number of open pages. Each subscriber reads through a
capacity-1 drop-oldest channel, so a circuit that renders slowly skips snapshots
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.
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.
The accepted cost is that an abandoned loop still holds its feed subscription — the
feed's idle gate stays open until it unwinds — and the warning is the operator's
only signal that a circuit teardown wedged.
`SessionDetailsPage` subscribes for the current session id and renders the most
recent N events (default 50) in a "Recent events" table. Its pump drains everything
queued and renders once per batch rather than once per event, and it re-checks
**inside the renderer dispatch** — where the subscription field is written, making
the check an unsynchronized read of dispatcher-owned state — that the batch's
subscription is still the live one. A batch read before a session switch would
otherwise render the previous session's events under the new session's heading.
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.
Both loops handle faults *inside* the loop and retry: only cancellation ends them.
A failing alarm query or a render that faults on one tick leaves the page's last
rows in place and is retried on the next tick — a fault that stopped polling for the
life of the page would leave stale rows behind with nothing to say so. The poll loop
also surfaces the fault in the same `Alarm query failed` banner that a query error
uses, and clears it on the first tick that succeeds; that dispatch is itself
best-effort, because the fault being reported may be an `InvokeAsync` against a
disposed renderer, in which case reporting fails the same way and the loop simply
exits on its next cancellation check. Neither loop method can fault, which is what
the bounded drain relies on — it must never observe a faulted loop task, since that
would surface out of `DisposeAsync` and skip the `CancellationTokenSource` dispose.
### SignalR hubs (remote clients)
Updates for out-of-process clients flow over three SignalR hubs, all guarded by the
`MxGateway.Dashboard.HubClients` policy (cookie OR `MxGateway.Dashboard.HubToken` `MxGateway.Dashboard.HubClients` policy (cookie OR `MxGateway.Dashboard.HubToken`
bearer). Each hub class is `[Authorize(Policy = HubClientsPolicy)]`. bearer). Each hub class is `[Authorize(Policy = HubClientsPolicy)]`.
@@ -167,46 +274,68 @@ bearer). Each hub class is `[Authorize(Policy = HubClientsPolicy)]`.
|---|---|---|---|---| |---|---|---|---|---|
| `DashboardSnapshotHub` | `/hubs/snapshot` | `DashboardSnapshotPublisher` (BackgroundService consuming `IDashboardSnapshotService.WatchSnapshotsAsync`) | `DashboardSnapshot` | Sent to all connected clients on every snapshot tick, but only while at least one client is connected (see "Idle gating" below); new connections receive the current snapshot synchronously in `OnConnectedAsync`. | | `DashboardSnapshotHub` | `/hubs/snapshot` | `DashboardSnapshotPublisher` (BackgroundService consuming `IDashboardSnapshotService.WatchSnapshotsAsync`) | `DashboardSnapshot` | Sent to all connected clients on every snapshot tick, but only while at least one client is connected (see "Idle gating" below); new connections receive the current snapshot synchronously in `OnConnectedAsync`. |
| `AlarmsHub` | `/hubs/alarms` | `AlarmsHubPublisher` (BackgroundService consuming `IGatewayAlarmService.StreamAsync(filter: null)`) | `AlarmFeedMessage` (`active_alarm` / `snapshot_complete` / `transition`) | Connected clients auto-join `__alarms__`; all clients receive every message. Publisher auto-reconnects every 5s on stream faults. | | `AlarmsHub` | `/hubs/alarms` | `AlarmsHubPublisher` (BackgroundService consuming `IGatewayAlarmService.StreamAsync(filter: null)`) | `AlarmFeedMessage` (`active_alarm` / `snapshot_complete` / `transition`) | Connected clients auto-join `__alarms__`; all clients receive every message. Publisher auto-reconnects every 5s on stream faults. |
| `EventsHub` | `/hubs/events` | `DashboardEventBroadcaster` invoked by each session's internal dashboard-mirror subscriber on its `SessionEventDistributor` (registered when the session becomes Ready) | `MxEvent` | Clients call `SubscribeSession(sessionId)` to join `session:{id}`, which also registers them in `EventsHubViewerRegistry` — the mirror is gated on that registry (see "Mirror gating" below). The dashboard is a first-class distributor subscriber, so it receives the session's events whether or not a gRPC client is streaming. It sees RAW session events — not the per-gRPC-subscriber `AfterWorkerSequence` filtering that `EventStreamService` applies at its own boundary — because the dashboard is a separate LDAP-authenticated monitoring view meant to show the session's full event activity. Tag values are stripped from the mirrored `MxEvent` copy by `DashboardEventBroadcaster` when `Dashboard:ShowTagValues` is false (the default) — event metadata (tag reference, quality, status, timestamps) still renders, but the value fields are blanked, so no value leaks through this seam. The per-session hub ACL that would scope a Viewer to specific sessions is still outstanding (SEC-25 / remediation roadmap item 12); the value redaction is the near-term hardening that closes the value-leak seam independently of that ACL. | | `EventsHub` | `/hubs/events` | `DashboardEventBroadcaster` invoked by each session's internal dashboard-mirror subscriber on its `SessionEventDistributor` (registered when the session becomes Ready) | `MxEvent` | Clients call `SubscribeSession(sessionId)` to join `session:{id}`, which also registers them in `EventsHubViewerRegistry` — the mirror is gated on that registry, which counts hub and in-process viewers alike (see "Mirror gating" below). The dashboard is a first-class distributor subscriber, so it receives the session's events whether or not a gRPC client is streaming. It sees RAW session events — not the per-gRPC-subscriber `AfterWorkerSequence` filtering that `EventStreamService` applies at its own boundary — because the dashboard is a separate LDAP-authenticated monitoring view meant to show the session's full event activity. Tag values are stripped from the mirrored `MxEvent` copy by `DashboardEventBroadcaster` when `Dashboard:ShowTagValues` is false (the default) — event metadata (tag reference, quality, status, timestamps) still renders, but the value fields are blanked, so no value leaks through this seam. The per-session ACL that would scope a Viewer to specific sessions is still outstanding for this seam and the in-process one alike (SEC-25 / remediation roadmap item 12); the value redaction is the near-term hardening that closes the value-leak seam independently of that ACL. |
`DashboardPageBase` opens a `DashboardSnapshotHub` connection via the connection ### Default cadences
factory in `OnInitializedAsync`, seeds `Snapshot` synchronously from
`IDashboardSnapshotService.GetSnapshot()` so the first render is non-empty, and
calls `InvokeAsync(StateHasChanged)` on every `SnapshotUpdated` push. SignalR's
`WithAutomaticReconnect` handles transient disconnects.
`SessionDetailsPage` additionally opens an `EventsHub` connection for the Both seams consume the same producing services, so they share these cadences:
current session id and renders the most recent N events (default 50) in a
"Recent events" table with a live/offline connection pill.
Default cadences:
- snapshot service produces one snapshot per - snapshot service produces one snapshot per
`MxGateway:Dashboard:SnapshotIntervalMilliseconds` (default 1s); `MxGateway:Dashboard:SnapshotIntervalMilliseconds` (default 1s);
- alarm publisher emits on each transition observed by the central monitor; - alarm publisher emits on each transition observed by the central monitor;
- event publisher emits per event fanned by the session's `SessionEventDistributor` - event publisher emits per event fanned by the session's `SessionEventDistributor`
to its internal dashboard-mirror subscriber (independent of any gRPC `StreamEvents`). to its internal dashboard-mirror subscriber (independent of any gRPC `StreamEvents`);
- the alarms page's provider-status badge resubscribes one second after its
`IGatewayAlarmService.StreamAsync` enumeration ends — the monitor completes a
subscriber's stream when it falls behind and again when it restarts, both
recoverable by resubscribing — and holds its last value in between. The page's
alarm rows are independent of that stream and refresh on the 3 s poll.
### Idle gating and snapshot cost ### Idle gating and snapshot cost
A snapshot is not free: each one takes a session-registry snapshot and sorts it, A snapshot is not free: each one takes a session-registry snapshot and sorts it,
copies the metrics dictionaries under the global metrics lock, and projects copies the metrics dictionaries under the global metrics lock, and projects
sessions, workers, faults, and the Galaxy summary. Without gating that work ran sessions, workers, faults, and the Galaxy summary. Without gating that work ran
once a second for the life of the process even when no browser was connected. once a second for the life of the process even when nothing was watching.
`DashboardSnapshotHub` counts live connections into the singleton Gating is two-tier, because the two seams have independent audiences and each must
be able to reach zero on its own.
**Hub tier.** `DashboardSnapshotHub` counts live connections into the singleton
`DashboardSnapshotHubConnectionCounter` (`OnConnectedAsync` / `OnDisconnectedAsync`, `DashboardSnapshotHubConnectionCounter` (`OnConnectedAsync` / `OnDisconnectedAsync`,
clamped at zero). `DashboardSnapshotPublisher` reads that count before advancing the clamped at zero). `DashboardSnapshotPublisher` reads that count before advancing the
snapshot enumerator: while it is zero the publisher does not call `MoveNextAsync` at snapshot enumerator: while it is zero the publisher does not call `MoveNextAsync` at
all, so the producing iterator stays suspended at its `yield` and builds nothing — all, so the producing iterator stays suspended at its `yield` and builds nothing —
the gate removes the snapshot *build*, not just the broadcast. The publisher the gate removes the snapshot *build*, not just the broadcast. The publisher
re-checks once a second while idle, so the first viewer to connect resumes the tick re-checks once a second while idle, so the first client to connect resumes the tick
within roughly one snapshot interval. That viewer does not wait for it either: within roughly one snapshot interval, and `OnConnectedAsync` pushes the current
`DashboardPageBase` seeds its first render synchronously from snapshot to that connection immediately. Now that no in-repo page connects to the
`IDashboardSnapshotService.GetSnapshot()`, and `OnConnectedAsync` pushes a snapshot hub, this tier stays idle unless a remote client connects.
to the new connection immediately.
Two per-tick costs inside the snapshot itself are bounded independently of the gate: **In-process tier.** `DashboardSnapshotFeed` gates its own pump on its subscriber
list: the first subscriber starts the pump, the last one leaving cancels it and
awaits it, so a gateway with no page open runs no timer and builds no snapshots on
this seam either. Successive pumps are chained through the previous pump's task, so
an unsubscribe immediately followed by a resubscribe restarts a fresh pump without
ever running two enumerations at once. A page does not wait for the pump's first
tick — `DashboardPageBase` seeds its first render synchronously from
`IDashboardSnapshotService.GetSnapshot()`.
That gate is generation-scoped rather than a plain subscriber count. Each pump owns
a generation, each subscriber is tagged with the generation it joined under, a pump
ends its generation the instant its source faults or completes — before the possibly
slow enumerator disposal — and a dying pump only ever detaches its own generation's
subscribers. Two races motivate the extra state:
- a subscriber arriving mid-teardown must start a fresh generation rather than
attach to a pump that is about to detach everybody and leave nobody watching;
- an unsubscribe must compare its own generation against the live one before it
cancels anything. Subscribers of an ending generation linger in the list until
that pump's reset runs, so counting the whole list would let them hold the idle
gate open, and cancelling on their behalf would stop a live pump that other
viewers depend on.
Two per-tick costs inside the snapshot itself are bounded independently of either gate:
- the effective configuration (`EffectiveGatewayConfiguration`) is built once and - the effective configuration (`EffectiveGatewayConfiguration`) is built once and
cached. It is a projection of `IOptions<GatewayOptions>`, which the gateway binds cached. It is a projection of `IOptions<GatewayOptions>`, which the gateway binds
@@ -220,26 +349,48 @@ Two per-tick costs inside the snapshot itself are bounded independently of the g
retried on the next tick and the previous summaries stay on screen. retried on the next tick and the previous summaries stay on screen.
Avoid pushing every MXAccess data-change event into a wider broadcast group. Avoid pushing every MXAccess data-change event into a wider broadcast group.
The current design routes events strictly through `session:{id}` groups; the Events are routed strictly per session (`session:{id}` groups on the hub,
snapshot hub continues to carry aggregate event counters and rates. per-session subscriber lists in process); the snapshot seams continue to carry
aggregate event counters and rates.
### Mirror gating ### Mirror gating
Each session's dashboard-mirror subscriber calls Each session's dashboard-mirror subscriber calls
`DashboardEventBroadcaster.Publish` for every event the session produces, `DashboardEventBroadcaster.Publish` for every event the session produces,
independently of whether any browser is watching that session. SignalR does not independently of whether anything is watching that session. `Publish` returns
expose group membership, so the broadcaster cannot ask whether `session:{id}` is immediately when `EventsHubViewerRegistry.HasViewers(sessionId)` is false,
empty. `EventsHubViewerRegistry` (singleton) supplies that answer: `EventsHub` **before** the redaction clone. That matters because redaction is on by default
mirrors its own `AddToGroup` / `RemoveFromGroup` calls into it, and
`OnDisconnectedAsync` releases every subscription a dropped connection held — the
only reliable signal for a browser tab that closes without unsubscribing.
`Publish` returns immediately when `HasViewers(sessionId)` is false, **before**
the redaction clone. That matters because redaction is on by default
(`Dashboard:ShowTagValues` false), so the unwatched steady state — nobody on any (`Dashboard:ShowTagValues` false), so the unwatched steady state — nobody on any
session-details page — previously paid a deep protobuf clone plus a send to an session-details page — previously paid a deep protobuf clone plus a send to an
empty group for every event of every session. Behaviour for a watched session is empty group for every event of every session. Behaviour for a watched session is
unchanged. unchanged.
The registry counts both audiences, which is what lets one gate serve both seams.
`EventsHub` mirrors its own `AddToGroup` / `RemoveFromGroup` calls into it —
SignalR does not expose group membership, so the broadcaster cannot ask whether
`session:{id}` is empty — and `OnDisconnectedAsync` releases every subscription a
dropped connection held, the only reliable signal for a browser tab that closes
without unsubscribing. An in-process subscription registers the same way under a
synthetic `inproc-`-prefixed connection id, which cannot collide with a SignalR
connection id and makes the origin obvious in a debugger; disposing it removes the
viewer and releases the synthetic connection, because that id never reconnects and
nothing else would ever release its per-connection entry. Both paths use the same
ordering — register before becoming a delivery target, deregister after ceasing to
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: 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.
The mirror subscriber itself is still registered on the `SessionEventDistributor` The mirror subscriber itself is still registered on the `SessionEventDistributor`
for the session's whole lifetime; only the per-event work is gated. Starting and for the session's whole lifetime; only the per-event work is gated. Starting and
stopping the mirror lease lazily with the first and last viewer was considered stopping the mirror lease lazily with the first and last viewer was considered
@@ -369,8 +520,9 @@ panel. The panel shows each subscribed tag's live value, MXAccess data type,
quality and source timestamp, refreshed every two seconds. The subscription quality and source timestamp, refreshed every two seconds. The subscription
panel is the explicit opt-in tag-value surface: it always shows values panel is the explicit opt-in tag-value surface: it always shows values
regardless of `Dashboard:ShowTagValues`, which governs the diagnostic regardless of `Dashboard:ShowTagValues`, which governs the diagnostic
session/worker views and the per-session `EventsHub` mirror (values are session/worker views and the per-session event mirror — both its hub and
redacted from the mirrored events when the flag is false). in-process audiences (values are redacted from the mirrored events when the flag
is false).
### Alarms page ### Alarms page
@@ -380,7 +532,11 @@ defaults to showing unacknowledged `Active` alarms; filters add acknowledged
alarms and narrow by area, severity range, and a reference/source/description alarms and narrow by area, severity range, and a reference/source/description
text search. Cleared alarms are not retained — the gateway holds no text search. Cleared alarms are not retained — the gateway holds no
alarm-history store, so the page reflects only the live active set. The page is alarm-history store, so the page reflects only the live active set. The page is
read-only; it does not acknowledge alarms. If `MxGateway:Alarms:Enabled` is read-only; it does not acknowledge alarms. A provider-status badge tracks the
central monitor's health from `IGatewayAlarmService.StreamAsync` in process — the
alarm service is already a multi-subscriber fan-out, so the badge needs no SignalR
client, no loopback socket, and no hub token — while the alarm rows themselves
still come from the three-second poll. If `MxGateway:Alarms:Enabled` is
false the central monitor never starts, and the page says so instead of showing false the central monitor never starts, and the page says so instead of showing
an empty list with no explanation. an empty list with no explanation.
@@ -536,6 +692,14 @@ Three authorization policies are registered:
cookie OR a `MxGateway.Dashboard.HubToken` bearer (used by WebSocket upgrades cookie OR a `MxGateway.Dashboard.HubToken` bearer (used by WebSocket upgrades
where the cookie can't be forwarded). where the cookie can't be forwarded).
The in-process page feeds carry no authentication of their own, and need none:
`MapRazorComponents<App>()` applies `RequireAuthorization(ViewerPolicy)` to the
component endpoints, so a page can only run inside a circuit whose principal is
already an authorized Viewer. The hub-token flow below therefore covers only the
remote hub surface. Neither seam scopes a Viewer to particular sessions — SEC-25
(the per-session ACL) is outstanding for both, and the mirror's value redaction
remains the near-term mitigation, unchanged by the move in-process.
Two environmental bypasses still apply, both scoped to **read-only** access: Two environmental bypasses still apply, both scoped to **read-only** access:
`MxGateway:Authentication:Mode = Disabled` and `MxGateway:Dashboard:AllowAnonymousLocalhost` `MxGateway:Authentication:Mode = Disabled` and `MxGateway:Dashboard:AllowAnonymousLocalhost`
(default `true`, loopback only) each satisfy a requirement that includes the Viewer (default `true`, loopback only) each satisfy a requirement that includes the Viewer
@@ -580,12 +744,15 @@ surface is affected. Never enable in production.
### Hub bearer flow ### Hub bearer flow
This flow serves remote hub clients only; in-process pages are authorized by the
component endpoint's `ViewerPolicy` and never mint a token.
SignalR connections cannot reuse the `__Host-` cookie when the JS client SignalR connections cannot reuse the `__Host-` cookie when the JS client
upgrades to WebSocket — the cookie's `SameSite=Strict; Path=/` keeps it from upgrades to WebSocket — the cookie's `SameSite=Strict; Path=/` keeps it from
being forwarded by the browser's WebSocket layer in some edge cases. The being forwarded by the browser's WebSocket layer in some edge cases. The
dashboard mints short-lived bearer tokens for the connection: dashboard mints short-lived bearer tokens for the connection:
1. The cookie-authenticated Blazor page calls `GET /hubs/token` 1. The cookie-authenticated client calls `GET /hubs/token`
(gated by `ViewerPolicy`, cookie-only). (gated by `ViewerPolicy`, cookie-only).
2. `HubTokenService.Issue(user)` serializes the user's name, NameIdentifier, 2. `HubTokenService.Issue(user)` serializes the user's name, NameIdentifier,
and role claims to JSON, encrypts with the ASP.NET Core data-protection and role claims to JSON, encrypts with the ASP.NET Core data-protection
@@ -603,9 +770,10 @@ dashboard mints short-lived bearer tokens for the connection:
5. The hubs' `[Authorize(Policy = HubClientsPolicy)]` accepts the resulting 5. The hubs' `[Authorize(Policy = HubClientsPolicy)]` accepts the resulting
identity. identity.
`DashboardHubConnectionFactory` (scoped to the Blazor circuit) wraps the There is no in-repo hub client: the helper that once wrapped `HubConnectionBuilder`
HubConnectionBuilder and supplies a fresh token via `AccessTokenProvider` on for a circuit was deleted when the pages moved to the in-process seams. An external
every (re)connect, so the short 5-minute lifetime is transparent to clients. 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 Caveat — logout does not revoke outstanding tokens. Logout clears the dashboard
cookie, but hub bearer tokens are self-contained, data-protection-encrypted, and cookie, but hub bearer tokens are self-contained, data-protection-encrypted, and
@@ -700,8 +868,14 @@ Integration tests should verify:
- a user in a Viewer-mapped LDAP group can render every page but cannot - a user in a Viewer-mapped LDAP group can render every page but cannot
invoke the Admin-only management actions, invoke the Admin-only management actions,
- a user with no mapped LDAP group cannot sign in at all, - a user with no mapped LDAP group cannot sign in at all,
- live snapshot updates when a fake session changes state are delivered - live snapshot updates when a fake session changes state reach a page through
via the `/hubs/snapshot` push, not by polling. the in-process `IDashboardSnapshotFeed` and reach a remote client through the
`/hubs/snapshot` push — neither by polling;
- the snapshot feed's idle gate: no subscribers means no pump, the last
subscriber of the live generation stops it, and a subscriber arriving
mid-teardown gets a fresh generation rather than a dead one;
- the event mirror's viewer gate counts in-process subscriptions as well as hub
connections, and a disposed in-process subscription releases its viewer count.
## Initial Implementation Slice ## Initial Implementation Slice
+6 -6
View File
@@ -557,12 +557,12 @@ real-clock deadlines into failures.
### Two more findings from the 2026-08-15 windev gate ### Two more findings from the 2026-08-15 windev gate
- `SecretsStorePathGuardTests.CreateBuilder_AcceptsSecretsStoreOutsideContentRoot_AndCreatesIt` - `SecretsStorePathGuardTests.CreateBuilder_AcceptsSecretsStoreOutsideContentRoot_AndCreatesIt`
fails **deterministically on Windows, on `main` as well as on any branch**, so it is not a used to fail deterministically on Windows: creating the builder opens `secrets.db`,
signal about the change under test. Creating the builder opens `secrets.db`, and `Microsoft.Data.Sqlite`'s connection pool kept the file handle alive past the test body, and
`Microsoft.Data.Sqlite`'s connection pool keeps the file handle alive past the test body, the cleanup's recursive directory delete hit a sharing violation Windows enforces and Unix
so the recursive directory delete in the cleanup hits a still-open file — a sharing does not. The cleanup now clears the SQLite connection pool before deleting the temp
violation Windows enforces and Unix does not. Pre-existing and tracked separately; do not directory (the same pattern as `TempDatabaseDirectory` and `PreHostSecretExpansionTests`),
chase it as a regression. Subtract it from the expected pass count on Windows. so the test passes on Windows and macOS alike — count it as a pass on both.
- The `StaWaitHelper` timing tests (`WaitForSignalOrMessages_*`) flake on a loaded box with a - The `StaWaitHelper` timing tests (`WaitForSignalOrMessages_*`) flake on a loaded box with a
signature that reads like a broken wait but is not: the helper wakes on *input being signature that reads like a broken wait but is not: the helper wakes on *input being
present*, so a message posted to the test thread ends the wait early. That is the helper present*, so a message posted to the test thread ends the wait early. That is the helper
+44
View File
@@ -887,6 +887,50 @@ Graceful shutdown sequence:
If shutdown wedges, the gateway kills the process. The worker should be written If shutdown wedges, the gateway kills the process. The worker should be written
so process kill does not corrupt other sessions. so process kill does not corrupt other sessions.
### Ending the pipe read (net48)
Step 8 above cannot be done by cancellation. On .NET Framework 4.8
`NamedPipeClientStream.ReadAsync` accepts a `CancellationToken` and then never
wires it to the overlapped I/O, so a read parked waiting for gateway bytes stays
parked no matter what the worker cancels. Closing the handle is the only thing
that ends it.
`WorkerPipeSession.RunMessageLoopAsync` races one outstanding read against the
heartbeat and event-drain loops, so every fault exit — an event-drain fault, an
event too large to frame, a failed heartbeat write — unwinds while that read is
still pending. The session therefore owns the transport: `RunAsync`'s outermost
`finally` disposes the stream as its last teardown step and then awaits the read
that disposal unblocks. Disposal comes last because in the ordinary case every
frame the session will ever write is already complete by then — the frame writer
signals a write only after it has been written *and* flushed. It is not last
because that is guaranteed: the wait on the heartbeat and drain loops is
budgeted, and a stream write is genuinely uncancellable, so an overrunning write
can still be in flight against the stream being disposed. Disposal is
consequently exception-*total*, catching anything the handle close throws and
logging it, because nothing raised while releasing a handle is more actionable
than the terminal exception that ended the session, and nothing may displace it.
Observation is unconditional; only the *logging* of it is budgeted.
`ObserveBackgroundTaskStopAsync` waits `BackgroundTaskStopTimeout` for a task to
stop and logs what it saw, but when it gives up it hands the task a
fault-observing continuation before returning. Windows owes no deadline for a
completion torn off a closed handle, so a bounded await on its own would reopen
the very orphaning window it was added to close. The same helper — and so the
same guarantee — covers the abandoned read, the heartbeat loop, and the
event-drain loop. This matters because the worker installs no
`TaskScheduler.UnobservedTaskException` handler: an unheld faulted task would
otherwise surface only at finalization, still holding the reader's reused
length-prefix buffer and its pooled payload buffer.
Two invariants follow. Nothing may call `WorkerFrameReader.ReadAsync` again once
a read has been abandoned — a second read would race the first for those buffers
and could return a pooled buffer twice — which `Debug.Assert`s at both read-issue
sites guard. And `WorkerPipeClient`'s `using` on the pipe stays as a backstop for
the paths the session never reaches (a session factory that throws), not as the
primary owner; disposal is idempotent, so its second `Dispose` is a no-op.
Graceful shutdown leaves no pending read at all, so the observation step is a
no-op on that path.
`MxAccessStaSession.ShutdownGracefullyAsync` implements the current cleanup `MxAccessStaSession.ShutdownGracefullyAsync` implements the current cleanup
path. It first calls `StaCommandDispatcher.RequestShutdown()` so new commands path. It first calls `StaCommandDispatcher.RequestShutdown()` so new commands
are rejected and queued commands that have not started receive are rejected and queued commands that have not started receive
+50 -11
View File
@@ -88,7 +88,9 @@ priority order. A caller enqueues its frame into the control or event queue
under a lock, then contends for a single write lock; whichever caller wins under a lock, then contends for a single write lock; whichever caller wins
drains every frame queued at that moment, control frames first and each class drains every frame queued at that moment, control frames first and each class
in FIFO order, so a command reply, fault, heartbeat, or shutdown in FIFO order, so a command reply, fault, heartbeat, or shutdown
acknowledgement is never delayed behind a backlog of queued events. Priority acknowledgement is never delayed behind a backlog of queued events — neither
in the bytes written nor in the flush that marks them delivered (see the
class-boundary flush under flush coalescing below). Priority
only reorders *which frame writes next* — it does not affect the sequence only reorders *which frame writes next* — it does not affect the sequence
value a frame receives (see below), so a caller cannot infer priority class value a frame receives (see below), so a caller cannot infer priority class
from the wire sequence. from the wire sequence.
@@ -117,13 +119,40 @@ Two failure shapes are distinguished during a drain pass:
and every frame still queued, then stops draining entirely so no caller and every frame still queued, then stops draining entirely so no caller
waits forever on a stream that will not recover. waits forever on a stream that will not recover.
Flushes are coalesced across a drained batch: each frame in the batch is Flushes are coalesced across a *run of same-class frames* inside a drain
written to the stream without an individual flush, then one `FlushAsync` pass: each frame in the run is written to the stream without an individual
runs after the whole batch, and only then does every successfully-written flush, then one `FlushAsync` runs — at the end of the pass, and additionally
frame's completion resolve — so a caller's `WriteAsync` still does not at every control-to-event boundary — and only then does every
complete until its bytes are both written *and* flushed, but a batch that successfully-written frame of that run resolve its completion. A caller's
happened to contain several queued frames pays one flush instead of one per `WriteAsync` therefore still does not complete until its bytes are both
frame. Note the ordering this implies at the peer: the frames reach the pipe written *and* flushed; what changed is *when* that moment arrives
for a control frame that a pass writes ahead of queued events. It used to be
the end of the pass, so a heartbeat, command reply, fault, or shutdown
acknowledgement was written first but only counted as delivered after up to a
full event batch had been written and flushed behind it. The boundary flush
closes the control run out before the events are written, so the priority
class governs the frame's delivery point and not just its byte order. The
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. 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
written-but-unflushed set, so a *later* failure in the same pass — a broken
write, or a failed end-of-pass flush — no longer reaches back and fails it.
That is the honest outcome: its bytes were flushed, so it was delivered. A
failure of the boundary flush itself is treated exactly like a failed
end-of-pass flush, and additionally fails the event frame the drain had
already claimed off its queue (nothing else would ever complete it) along
with every frame still queued.
Note the ordering all of this implies at the peer: the frames reach the pipe
before the flush that follows them, so the gateway can read a whole batch before the flush that follows them, so the gateway can read a whole batch
while the writer has not yet flushed it. Anything observing the flush itself while the writer has not yet flushed it. Anything observing the flush itself
(a test counting flushes, for instance) must wait for the flush, not infer it (a test counting flushes, for instance) must wait for the flush, not infer it
@@ -134,12 +163,22 @@ drains them together, so a burst of N events costs one flush rather than N —
the coalescing the batch machinery was built for now engages on the event hot the coalescing the batch machinery was built for now engages on the event hot
path, not only when independent producers happen to queue behind a blocked path, not only when independent producers happen to queue behind a blocked
write. Intra-batch order is preserved (FIFO enqueue under one lock), and a write. Intra-batch order is preserved (FIFO enqueue under one lock), and a
concurrently queued control frame is still drained ahead of the batch. A concurrently queued control frame is still drained — and now flushed and
per-frame rejection inside a batch (for example one oversized event) surfaces completed — ahead of the batch's remaining events, which is why a batch a
from the batch's awaited completions as that frame's control frame cuts into pays one extra flush while an uninterrupted batch
still pays exactly one. A per-frame rejection inside a batch (for example one
oversized event) surfaces from the batch's awaited completions as that frame's
`WorkerFrameProtocolException`; the remaining completions are still observed `WorkerFrameProtocolException`; the remaining completions are still observed
so none faults unobserved. so none faults unobserved.
The completion is the frame's delivery point, not necessarily the instant its
caller returns. A caller that loses the race for the write lock only observes
its own completion after the winning drainer releases the lock, so its return
remains bounded by that drain pass even though its control frame was flushed
and completed at the class boundary inside it. The boundary flush is what
makes the delivery point honest; unparking a lock-race loser from the winner's
pass would be a separate change to the enqueue-then-contend shape.
Cancellation of a `WriteAsync`/`WriteBatchAsync` call that is still waiting Cancellation of a `WriteAsync`/`WriteBatchAsync` call that is still waiting
for the write lock when its token fires tombstones the queued frame: the for the write lock when its token fires tombstones the queued frame: the
cancelled caller marks its frame under `_gate`, and the draining lock-holder's cancelled caller marks its frame under `_gate`, and the draining lock-holder's
@@ -0,0 +1,374 @@
# Deferred-Findings Remediation Implementation Plan
> **For Claude:** REQUIRED SUB-SKILL: Use superpowers-extended-cc:subagent-driven-development to implement this plan task-by-task (Opus implementers per the user's instruction).
**Goal:** Resolve the six findings the 2026-08-15 perf-review remediation explicitly deferred (`docs/plans/2026-08-15-perf-review-remediation.md:611-620`) plus the pre-existing Windows-only `SecretsStorePathGuardTests` failure, so the deferred table empties and windev returns to a clean 1046/1046 gateway suite.
**Architecture:** Two phases. Phase A is gateway-side (net10, fully verifiable on macOS): the secrets-test fix, the distributor dictionary swap, event-path iterator flattening, and the dashboard in-process refactor that removes the Blazor pages' loopback SignalR hop while preserving the idle gate, mirror viewer gating, and clone-then-redact invariants. Phase B is worker-side (net48 x86, verified on windev over ssh): control-frame completion decoupling in the two-class frame writer, pipe-read teardown restructuring, and value-cache clone removal per the completed aliasing audit.
**Tech Stack:** .NET 10 / ASP.NET Core / Blazor Server / System.Threading.Channels (gateway); .NET Framework 4.8 x86 (worker); xUnit; windev CI clone `C:\build\mxaccessgw-ci` via `ssh windev`.
**Branch:** `perf/deferred-remediation` off local `main` (`15f188e`).
---
## Ground rules for every implementer subagent
- Shared working tree at `/Users/dohertj2/Desktop/MxAccessGateway`. **NEVER run `git stash`, `git reset`, `git clean`, `git checkout <sha/branch>`, or any command that touches files outside your task's `Files:` list.** Commit with explicit pathspecs only (`git add <your files> && git commit`).
- Build/test lock: before `dotnet build` or `dotnet test`, acquire the lock with `mkdir /private/tmp/claude-501/-Users-dohertj2-Desktop-MxAccessGateway/f36938ae-bbca-4245-b5c9-fac512d69e22/scratchpad/buildlock` (retry loop with sleep until it succeeds); `rmdir` it in ALL exit paths.
- `TreatWarningsAsErrors=true`, `Nullable=enable` repo-wide. Follow `docs/style-guides/CSharpStyleGuide.md`: file-scoped namespaces, `sealed` by default, `Async` suffix, MXAccess-aligned names.
- Worker projects (`ZB.MOM.WW.MxGateway.Worker*`) are net48/x86 and DO NOT COMPILE on macOS. For Phase B tasks: edit carefully, self-review for net48 compatibility (target-typed `new` and file-scoped namespaces ARE valid — `LangVersion=latest`; but no `Span`-based BCL overloads, no `IAsyncDisposable` on BCL types, `Channel` comes from System.Threading.Channels package which the worker already references). Compilation and tests happen at the Task 13 windev gate.
- Update affected docs in the same commit as the source (repo rule), except the dashboard design doc which Task 8 consolidates (deliberate, to avoid parallel edits to one file).
- MXAccess parity: never synthesize events, never mutate an event already handed to the outbound queue or wire.
---
## Phase A — gateway (macOS-verifiable)
### Task 1: Windows-safe cleanup in SecretsStorePathGuardTests
**Classification:** small
**Estimated implement time:** ~3 min
**Parallelizable with:** Task 2, Task 3, Task 4
**Files:**
- Modify: `src/ZB.MOM.WW.MxGateway.Tests/Configuration/SecretsStorePathGuardTests.cs`
- Modify: `docs/GatewayTesting.md` (lines ~557-565, the "fails deterministically on Windows" note)
**Why:** `CreateBuilder_AcceptsSecretsStoreOutsideContentRoot_AndCreatesIt` (lines 84-105) fails deterministically on Windows: `GatewayApplication.CreateBuilder` migrates the secrets store through `SecretsSqliteConnectionFactory` (`Pooling = true`, WAL), disposal returns the connection to the Microsoft.Data.Sqlite pool with the native handle open, and the `finally`'s `Directory.Delete(directory, recursive: true)` (line 103) hits a sharing violation. macOS passes only because Unix unlinks open files. The repo fixes this pattern twice already: `TestSupport/../TempDatabaseDirectory.cs:57` and `Configuration/PreHostSecretExpansionTests.cs:130-153`.
**Spec:**
1. In the failing test's `finally`, before `Directory.Delete`: call `Microsoft.Data.Sqlite.SqliteConnection.ClearAllPools();` and wrap the delete in `try { ... } catch (IOException) { } catch (UnauthorizedAccessException) { }` (best-effort, mirroring `TempDatabaseDirectory.Dispose`). Add a comment mirroring the one in `PreHostSecretExpansionTests.cs:133-137` (WAL + pooling keeps the handle alive past dispose).
2. Leave the rejection test alone (the guard means its file is never created).
3. Update `docs/GatewayTesting.md`: replace the "subtract it from the expected pass count on Windows" paragraph with a short note that the test's cleanup now clears the SQLite pool first and the failure is fixed as of this branch.
**Steps:** edit → `dotnet test src/ZB.MOM.WW.MxGateway.Tests/ZB.MOM.WW.MxGateway.Tests.csproj --filter "FullyQualifiedName~SecretsStorePathGuardTests"` (expect 2/2 on macOS; the real proof is the Task 13 windev gate) → commit `fix(tests): clear the SQLite pool before deleting the secrets path-guard temp dir — Windows sharing violation`.
---
### Task 2: SessionEventDistributor `_subscribers` → plain `Dictionary`
**Classification:** small
**Estimated implement time:** ~3 min
**Parallelizable with:** Task 1, Task 3, Task 4
**Files:**
- Modify: `src/ZB.MOM.WW.MxGateway.Server/Sessions/SessionEventDistributor.cs`
**Why:** All five access sites (`:365`, `:555`, `:771`, `:799`, `:813`) are inside `lock (_lifecycleLock)`; the lock-free hot path reads the copy-on-write `_subscriberSnapshot` array (`:958`, `:302`), never the dictionary. The concurrent type buys nothing. Audit confirmed no external/reflection access.
**Spec:** Change the field at `:107` to `Dictionary<long, Subscriber>`; `TryRemove(subscriber.Id, out _)` at `:799` becomes `Remove(subscriber.Id)`. Reword the type remarks at `:69-80`, `:111-123`, and `:298-300` where they name `ConcurrentDictionary` by design — the invariant to state is now: "the dictionary is only ever touched under `_lifecycleLock`; lock-free readers use `_subscriberSnapshot`."
**Steps:** edit → `dotnet test ... --filter "FullyQualifiedName~SessionEventDistributorTests"` (29 facts, expect all green) → commit `refactor(sessions): _subscribers to plain Dictionary — every access is under _lifecycleLock`.
---
### Task 3: Merge the session event-source pass-through iterator
**Classification:** standard
**Estimated implement time:** ~5 min
**Parallelizable with:** Task 1, Task 2, Task 4
**Files:**
- Modify: `src/ZB.MOM.WW.MxGateway.Server/Sessions/GatewaySession.cs` (`MapWorkerEventsAsync` ~:767-776, `ReadEventsAsync` ~:1517-1530)
- Test: `src/ZB.MOM.WW.MxGateway.Tests/Gateway/Sessions/GatewaySessionTests.cs` (existing)
**Why:** The worker→distributor source chain nests three compiler-generated async iterators per event: `WorkerClient.ReadEventsCoreAsync``GatewaySession.ReadEventsAsync` (pure pass-through: `TouchClientActivity(); yield return`) → `GatewaySession.MapWorkerEventsAsync` (`yield return mapper.MapEvent(...)`). The pass-through layer is two extra `MoveNextAsync` state-machine hops per event for no semantic value.
**Spec:**
1. FIRST grep all callers of `ReadEventsAsync`. If `MapWorkerEventsAsync` is its only caller, inline it: `MapWorkerEventsAsync` calls `GetReadyWorkerClientAsync`, iterates `client.ReadEventsAsync(ct)` directly, calls `TouchClientActivity()` per event, and `yield return mapper.MapEvent(workerEvent)`. Delete `ReadEventsAsync`. If other callers exist, keep the method for them but make `MapWorkerEventsAsync` self-contained as above — do NOT change any caller outside this file; report the finding.
2. Behavior must be byte-identical: same activity-touch cadence (per event), same exception propagation (WorkerClientException flows to the distributor pump unchanged), no event synthesis, worker order preserved.
3. `WorkerClient.ReadEventsCoreAsync`'s single-reader claim (`_eventsReaderClaimed`) must still be exercised exactly once per attach — do not add a second call site.
**Steps:** grep callers → edit → `dotnet test ... --filter "FullyQualifiedName~GatewaySession"` and `--filter "FullyQualifiedName~SessionEventDistributorTests"` → commit `perf(sessions): fold the ReadEventsAsync pass-through into MapWorkerEventsAsync — one fewer iterator per event`.
---
### Task 4: EventStreamService direct channel reads in the live loop
**Classification:** high-risk
**Estimated implement time:** ~5 min
**Parallelizable with:** Task 1, Task 2, Task 3
**Files:**
- Modify: `src/ZB.MOM.WW.MxGateway.Server/Grpc/EventStreamService.cs`
- Test: `src/ZB.MOM.WW.MxGateway.Tests/Gateway/Grpc/EventStreamServiceTests.cs` (existing 17 facts — must pass unchanged)
**Why:** The subscriber-side live loop materializes `subscriber.Reader.ReadAllAsync(ct).GetAsyncEnumerator(ct)` (`:109-111`) — a BCL async-iterator wrapper costing a state-machine hop per event on the hottest gateway path. Direct `ChannelReader` consumption (`WaitToReadAsync` + drain-with-`TryRead`) removes it.
**Spec:**
1. Replace the enumerator with direct reads: `while (await reader.WaitToReadAsync(ct)) { while (reader.TryRead(out MxEvent? mxEvent)) { ...existing per-event body... } }`; loop ends when `WaitToReadAsync` returns false (channel completed).
2. EVERY invariant in the current body survives, verbatim where possible:
- ReplayGap sentinel emitted exactly once, first, only when `replayGap` (`:133-139`) — untouched, it precedes the live loop.
- Replay batch stitching (`:141-150`) — untouched.
- Per-RPC dedup watermark `if (mxEvent.WorkerSequence <= afterWorkerSequence) continue;` (`:179-182`) — must apply to every live event.
- `WorkerClientException` catch → `session.MarkFaulted` → metrics → rethrow (`:164-174`): a completed-with-exception channel surfaces its exception from `WaitToReadAsync` — the catch must wrap the wait/read, preserving identical fault classification. Terminal `SessionManagerException(EventQueueOverflow)` propagates unchanged.
- `finally` ordering (`:192-200`): with no enumerator to dispose, the remaining order is backlog-gauge registration disposal → lease disposal → `metrics.StreamDisconnected("Detached")`. Keep the comments explaining why.
3. Cancellation: `WaitToReadAsync(ct)` throws `OperationCanceledException` on detach — must reach the same code path the enumerator's cancellation did (the gRPC layer treats it as client disconnect). Verify against `StreamEventsAsync_WhenCanceled_DetachesSubscriber`.
4. No public-surface change; `MxAccessGatewayService` (`:151-179`) is untouched.
**Steps:** edit → run the full `EventStreamServiceTests` class + `GatewayEndToEndReconnectReplayTests` + `GatewayEndToEndMultiSubscriberTests` → commit `perf(grpc): consume the subscriber channel directly in StreamEventsAsync — drops the ReadAllAsync iterator hop`.
---
### Task 5: In-process dashboard snapshot feed + page switch
**Classification:** high-risk
**Estimated implement time:** ~8 min (accepted overage; splitting further would split one invariant)
**Parallelizable with:** Task 6, Task 7
**Files:**
- Create: `src/ZB.MOM.WW.MxGateway.Server/Dashboard/IDashboardSnapshotFeed.cs`
- Create: `src/ZB.MOM.WW.MxGateway.Server/Dashboard/DashboardSnapshotFeed.cs`
- Modify: `src/ZB.MOM.WW.MxGateway.Server/Dashboard/Components/DashboardPageBase.cs`
- Modify: `src/ZB.MOM.WW.MxGateway.Server/Dashboard/DashboardServiceCollectionExtensions.cs` (one `AddSingleton` line)
- Create: `src/ZB.MOM.WW.MxGateway.Tests/Gateway/Dashboard/DashboardSnapshotFeedTests.cs`
**Why:** Eight pages inherit `DashboardPageBase` and each opens a loopback `/hubs/snapshot` HubConnection (`DashboardPageBase.cs:62`) — a WebSocket round trip back into the same process per circuit. `IDashboardSnapshotService.WatchSnapshotsAsync` exists but is NOT multicast (each enumeration = its own `PeriodicTimer` + snapshot build), so pages must not call it directly; a shared feed does one enumeration and fans out.
**Spec:**
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. *(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.
**Steps:** write feed tests first (fail) → implement feed → page switch → `dotnet test ... --filter "FullyQualifiedName~DashboardSnapshotFeed"` then `--filter "FullyQualifiedName~Dashboard"` (whole dashboard test folder) → commit `feat(dashboard): in-process snapshot feed replaces the pages' loopback /hubs/snapshot hop`.
---
### Task 6: In-process session event subscription + SessionDetailsPage switch
**Classification:** high-risk
**Estimated implement time:** ~8 min
**Parallelizable with:** Task 5, Task 7
**Files:**
- Modify: `src/ZB.MOM.WW.MxGateway.Server/Dashboard/Hubs/DashboardEventBroadcaster.cs`
- Modify: `src/ZB.MOM.WW.MxGateway.Server/Dashboard/Hubs/EventsHubViewerRegistry.cs` (only if a member is needed for synthetic connection ids; prefer reusing the existing API)
- Modify: `src/ZB.MOM.WW.MxGateway.Server/Dashboard/Components/Pages/SessionDetailsPage.razor` (the `/hubs/events` connection at `:271,297`)
- Test: `src/ZB.MOM.WW.MxGateway.Tests/Gateway/Dashboard/DashboardEventBroadcasterTests.cs` (extend)
**Why:** `SessionDetailsPage` opens a loopback `/hubs/events` connection. The broadcaster already short-circuits on `!viewerRegistry.HasViewers(sessionId)` BEFORE the redaction deep clone (`DashboardEventBroadcaster.cs:51-56`) — the mirror viewer gating shipped last round. An in-process subscription must keep feeding that registry or every unwatched session pays `MxEvent.Clone()` per event again.
**Spec:**
1. Add to `DashboardEventBroadcaster` an in-process subscribe API: `IDashboardEventSubscription Subscribe(string sessionId)` returning a disposable that exposes `ChannelReader<MxEvent> Reader` (bounded, capacity ~256, `DropOldest` — this is a UI mirror, loss is acceptable and already documented for the hub path). On subscribe: register a synthetic connection id (e.g. `"inproc-" + Guid.NewGuid().ToString("N")`) with `EventsHubViewerRegistry.AddViewer(connectionId, sessionId)`; on dispose: `RemoveViewer` + `ReleaseConnection` in the order the hub uses (`EventsHub.cs:86,99`). Registry stays the single source of truth for `HasViewers`.
2. `Publish` (`:39-86`): after the existing `HasViewers` check and the clone-then-redact (`RedactValues` `:97-109`), `TryWrite` the SAME redacted clone to each in-process subscriber of that session, in addition to the hub group send. The source `MxEvent` is shared with the gRPC stream and replay ring — the existing never-mutate-in-place rule holds; in-process subscribers receive the redacted clone only.
3. `SessionDetailsPage`: replace the HubConnection + `SubscribeSession` invoke with `broadcaster.Subscribe(SessionId)` and a read loop marshalling to the renderer via `InvokeAsync(StateHasChanged)`; dispose the subscription in `DisposeAsync`. Keep the existing per-session ACL posture (any Viewer may watch any session — SEC-25 is tracked separately; do not widen or narrow it here).
4. Tests to add in `DashboardEventBroadcasterTests`: (a) in-process subscriber receives the redacted event when `ShowTagValues=false` and the source event is not mutated; (b) subscribing flips `HasViewers` so `Publish` stops short-circuiting (proves mirror gating integration); (c) disposing the last in-process subscriber restores the no-viewers short-circuit (no clone, no send — reuse the existing `Publish_WithNoRegisteredViewers_DoesNotCloneOrSend` fake pattern); (d) hub viewers and in-process viewers are independently counted.
**Steps:** tests first → implement → `dotnet test ... --filter "FullyQualifiedName~DashboardEventBroadcaster"` + `--filter "FullyQualifiedName~EventsHubViewerRegistry"` + `--filter "FullyQualifiedName~GatewaySessionDashboardMirror"` → commit `feat(dashboard): in-process session event subscription feeds the viewer registry — SessionDetailsPage drops its /hubs/events hop`.
---
### Task 7: AlarmsPage provider-status via IGatewayAlarmService
**Classification:** standard
**Estimated implement time:** ~4 min
**Parallelizable with:** Task 5, Task 6
**Files:**
- Modify: `src/ZB.MOM.WW.MxGateway.Server/Dashboard/Components/Pages/AlarmsPage.razor` (`:194` HubConnection, `:281-304` poll loop untouched)
**Why:** `AlarmsPage` opens `/hubs/alarms` but only consumes `ProviderStatus` payloads from it (alarm rows come from the 3 s `QueryAlarmsAsync` poll). `IGatewayAlarmService.StreamAsync` (`GatewayAlarmMonitor.cs:724-777`) is already a true multi-subscriber in-process fan-out.
**Spec:** Replace the HubConnection with a background loop over `alarmService.StreamAsync(alarmFilterPrefix: null, ct)`, handling only `PayloadOneofCase.ProviderStatus` (skip snapshot/live alarm payloads — the poll stays authoritative for rows). The monitor's drop policy completes a lagging subscriber's channel (`:700-712`): on completion or fault, delay ~1 s and resubscribe (matching the hub path's `WithAutomaticReconnect` posture). Dispose via the page's existing cancellation. Leave the poll loop alone.
**Steps:** edit → `dotnet build src/ZB.MOM.WW.MxGateway.Server``dotnet test ... --filter "FullyQualifiedName~Alarms"` → commit `feat(dashboard): AlarmsPage reads provider status from IGatewayAlarmService in-process`.
---
### Task 8: Dashboard design-doc update (consolidated)
**Classification:** small
**Estimated implement time:** ~4 min
**Parallelizable with:** none (runs after 5, 6, 7 land)
**Files:**
- Modify: `docs/GatewayDashboardDesign.md` (sections at ~:112-114, :162-178, :190-217, :228-247, :535-541, :581-595)
**Spec:** Rewrite the affected sections to describe: pages consume in-process seams (`IDashboardSnapshotFeed`, `DashboardEventBroadcaster.Subscribe`, `IGatewayAlarmService.StreamAsync`); the three hubs and `/hubs/token` remain as the remote/external surface; idle gating is now two-tier (hub connection counter gates the hub publisher; feed subscriber count gates the in-process pump — while nobody watches, neither builds a snapshot); mirror gating counts hub viewers AND in-process viewers through the one registry; clone-then-redact still happens once in the broadcaster before any delivery; ViewerPolicy on the component endpoint is the in-process auth gate; SEC-25 per-session ACL gap unchanged. Present tense, why-not-what, no marketing.
**Commit:** `docs(dashboard): in-process page feeds, two-tier idle gating, hubs as the external surface`
---
### Task 9: Phase A gate — full gateway suite on macOS
**Classification:** trivial (verification only)
**Parallelizable with:** none (after Tasks 1-8)
Run `dotnet build src/ZB.MOM.WW.MxGateway.NonWindows.slnx` (expect 0 warnings) and the full `dotnet test src/ZB.MOM.WW.MxGateway.Tests/ZB.MOM.WW.MxGateway.Tests.csproj` (expect ≥1046 passed, 0 failed; new feed/broadcaster tests raise the count). Fix-forward any failure before Phase B.
---
## Phase B — worker (net48 x86, verified on windev)
### Task 10: Control-frame completion decoupling in WorkerFrameWriter
**Classification:** high-risk
**Estimated implement time:** ~6 min
**Parallelizable with:** Task 11, Task 12
**Files:**
- Modify: `src/ZB.MOM.WW.MxGateway.Worker/Ipc/WorkerFrameWriter.cs`
- Modify: `src/ZB.MOM.WW.MxGateway.Worker.Tests/Ipc/WorkerFrameProtocolTests.cs`
- Modify: `docs/WorkerFrameProtocol.md` (~:120-131 completion-semantics paragraph)
**Why:** Wire ordering is already correct — `DequeueNext` (`:383-413`) re-checks `_controlFrames` before every frame. The coupling is completion latency: `DrainQueuedFramesAsync` (`:304-361`) defers the single `FlushAsync` and ALL `TrySetResult` calls to after the whole drain pass, so a heartbeat/command-reply/fault/shutdown-ack `Task` awaited by its writer does not resolve until up to 128 event frames behind it are written and flushed. The XML docs claim "never delayed behind an event backlog" — true of bytes, false of the awaited completion.
**Spec:**
1. Record the priority class on `PendingFrame` (`:23-48`), set at construction in `WriteAsync` (`:109`) and `WriteBatchAsync` (`:192`).
2. In `DrainQueuedFramesAsync`: when `DequeueNext` returns an `Event` frame while `written` contains one or more not-yet-completed `Control` frames, first `FlushAsync` + complete + clear `written`, then continue draining. Exit-path flush at `:339-360` unchanged. Net effect: a control frame's completion never waits on an event frame dequeued after it; the pure-event 128-batch hot path still pays exactly one flush (guarded by the existing `WriteAsync_WhenBatchDrainedTogether_FlushesOnce` and `EventBurst_DrainLoopCoalescesFlushes`); a pure-control burst still pays one flush. Do NOT flush per control frame unconditionally — that reintroduces the pre-WRK-12 syscall-per-heartbeat cost.
3. Failure handling: `FailFrames(written, ...)` / `FailAllQueued` (`:327-336`) operate on the current `written` list; after an early flush+complete+clear, frames already completed must not be failable — verify the clear ordering makes that structurally true, and extend the fault-injection tests if the early-flush path adds a new failure window (a `FlushAsync` fault with a partially-completed pass).
4. New test (use the existing `GatedWriteStream` harness ~`:880`): queue a control frame behind N gated event frames within one drain pass; assert the control frame's `WriteAsync` task completes before the last event write is released. Keep all 9 existing writer tests green — sequence stamping (`:431-483`), claim/tombstone interlock (`:244-274`), and wire order must be untouched.
5. `docs/WorkerFrameProtocol.md`: update the completion-semantics paragraph — completion now resolves at the class-transition flush, still meaning "written AND flushed".
**Commit:** `perf(worker): control-frame completions resolve at the class-transition flush, not after the event batch`
---
### Task 11: Worker pipe-read teardown — dispose-to-unblock and observe the abandoned read
**Classification:** high-risk
**Estimated implement time:** ~8 min
**Parallelizable with:** Task 10, Task 12
**Files:**
- Modify: `src/ZB.MOM.WW.MxGateway.Worker/Ipc/WorkerPipeSession.cs` (`RunMessageLoopAsync` `:267-310`, ctor `:55-68`)
- Modify: `src/ZB.MOM.WW.MxGateway.Worker/Ipc/WorkerPipeClient.cs` (`:143-159`) — only if ownership must move; prefer not
- Modify: `src/ZB.MOM.WW.MxGateway.Worker.Tests/Ipc/WorkerPipeSessionTests.cs`
- Modify: `src/ZB.MOM.WW.MxGateway.Worker/Ipc/WorkerFrameReader.cs` (comment only)
**Why:** On net48, `NamedPipeClientStream.ReadAsync` ignores its `CancellationToken` (`WorkerFrameReader.cs:109-111`). Fault-path exits (event-drain fault, oversized event, watchdog, heartbeat write failure) leave `readTask` pending; it is unblocked only when `WorkerPipeClient`'s `using` disposes the pipe, at which point it faults with `ObjectDisposedException`/`IOException` on a Task nobody observes (the finally at `:303-309` awaits only heartbeat and drain). The frame-pooling change (GWC-30) makes this sharper: the abandoned read owns the per-instance `_lengthPrefix` buffer and possibly a rented ArrayPool payload — the reader's single-consumer invariant holds today only because nothing ever reads again after abandonment.
**Spec — constraints, implementer designs within them:**
1. **No unobserved faulted Task.** After the stream is disposed, `readTask`'s fault must be awaited/observed (reuse `ObserveBackgroundTaskStopAsync`'s timeout-and-log shape, `:312-348`) before `WorkerPipeClient.RunAsync` returns.
2. **Ordering: final writes complete before disposal.** The shutdown ack (`WriteShutdownAckAsync` `:1064-1069`) and fault frames (`TryWriteFaultAsync` `:1164+`) are written after the message loop exits on some paths — trace every exit path and place the stream disposal AFTER the last possible write on each. The clean design: `WorkerPipeSession` keeps a reference to the ctor `Stream`; `RunAsync`'s outermost finally (after runtime-session disposal and any fault write, `:133-145`) disposes the stream and then observes `readTask` (stored in a field by `RunMessageLoopAsync`). `WorkerPipeClient`'s `using` then double-disposes harmlessly. If the trace shows a fault write that happens in `WorkerPipeClient` after `session.RunAsync` returns (there is none known), fall back to moving observation into `WorkerPipeClient`.
3. **Never a second read.** After abandonment, no code path may call `_reader.ReadAsync` again (pooled-buffer use-after-return). The message loop already guarantees this (`return` before reassignment on the graceful path); keep it structurally true and assert it in a comment on `_lengthPrefix` (`WorkerFrameReader.cs:23-25`).
4. **Graceful path unchanged:** `WorkerShutdown`/`ShutdownWorker` exits have no pending read; disposal+observation must be a no-op there (observe a completed/absent task).
5. Document the net48 token-ignoring fact where the read is issued (`RunMessageLoopAsync` and/or `ReadExactlyOrThrowAsync`) — the research found zero comments acknowledging it.
6. Tests (net48 project, real `PipePair` harness `:2433-2485`): (a) fault-path exit (reuse the `RunAsync_EventFrameTooLarge_...` shape `:868`) — assert `RunAsync` completes within the existing 5 s bound AND, via a `TaskScheduler.UnobservedTaskException` hook armed in the test with a forced GC, that no unobserved exception leaks; (b) graceful shutdown still completes with no pending read; (c) the session disposes the stream (harness observes the gateway-side stream faulting its own pending read promptly rather than at `PipePair.Dispose`).
**Commit:** `fix(worker): session-owned stream disposal unblocks and observes the net48 pipe read at teardown`
---
### Task 12: Value-cache clone removal per the aliasing audit
**Classification:** standard
**Estimated implement time:** ~5 min
**Parallelizable with:** Task 10, Task 11
**Files:**
- Modify: `src/ZB.MOM.WW.MxGateway.Worker/MxAccess/MxAccessValueCache.cs` (`Set` `:82,83,97`; `CachedValue` `:275`)
- Modify: `src/ZB.MOM.WW.MxGateway.Worker.Tests/MxAccess/MxAccessValueCacheTests.cs` (rewrite the `:58` test)
- Modify: `src/ZB.MOM.WW.MxGateway.Worker.Tests/MxAccess/MxAccessCommandExecutorTests.cs` (add cached-read test)
**Why (audit result):** All three clones in `Set``Value.Clone()` (deep, recursive for arrays), `SourceTimestamp.Clone()`, `Statuses.Clone()` (container + N proxies) — are removable. The event is fully stamped BEFORE `Set` runs (`Enqueue` at `MxAccessBaseEventSink.cs:263` precedes `postPublish` at `:288`; sequence/timestamp stamped inside `Enqueue`, `MxAccessEventQueue.cs:269-270`) and the queue's ownership invariant forbids later mutation. The alias already exists on the read side: `SucceededRead` (`MxAccessSession.cs:1086,1091,1096`) hands the cache's own `Value`/`SourceTimestamp` instances into every `BulkReadResult`, which downstream only wraps and serializes. Worker↔gateway is a process boundary — no gateway consumer can alias.
**Spec:**
1. Remove all three clones; `CachedValue` stores the event's own references.
2. Ownership contract comment on `Set` and on `CachedValue`: the cache holds borrowed references into an enqueued, write-once `MxEvent`; consumers may read and serialize, never mutate; mutation would additionally invalidate `QueuedEvent.Size` — the enqueue-time memoized serialized size that the byte-budgeted `Drain` charges (`MxAccessEventQueue.cs:499-506`), so a grown message could overshoot the negotiated frame max and fault the session via `MessageTooLarge`.
3. Rewrite `Set_StoresIndependentSnapshot_UnaffectedByLaterEventMutation` (`:58` — it codifies the invariant being reversed) into the aliasing contract: `Set` then `TryGet` returns the same `Value`/`SourceTimestamp`/`Statuses`-element instances (`Assert.Same`), with the doc comment explaining the write-once borrow.
4. Add the missing cached-read-path test in `MxAccessCommandExecutorTests`: seed the cache, dispatch a `ReadBulk` that hits `TryGetCachedReadFor` → assert `WasCached == true` and `result.Value` is reference-equal to the cached instance (closing the coverage gap the audit found — nothing today exercises `WasCached == true` end-to-end in the worker).
5. `MxAccessWriteCompletionCache.Record`'s parallel `statuses.Clone()` (`:76`) is left AS-IS deliberately (different lifecycle, not in the finding) — add one cross-reference comment there pointing at the value-cache ownership contract.
**Commit:** `perf(worker): value cache borrows the write-once event's instances — three clones per OnDataChange removed`
---
### Task 13: Phase B gate — windev full verification
**Classification:** trivial (verification only)
**Parallelizable with:** none (after Tasks 10-12; Phase A gate must be green)
Push the branch to origin, then on windev (`ssh windev`, clone `C:\build\mxaccessgw-ci`): fetch + checkout the branch; `dotnet build src/ZB.MOM.WW.MxGateway.slnx` (0 warnings); `dotnet build src/ZB.MOM.WW.MxGateway.Worker/... -p:Platform=x86`; `dotnet test src/ZB.MOM.WW.MxGateway.Worker.Tests/... -p:Platform=x86` (expect 501+ passed, 0 failed — new tests raise the count); `dotnet test src/ZB.MOM.WW.MxGateway.Tests/...` (expect **0 failed including SecretsStorePathGuardTests** — the Task 1 proof). Known caveat: the reconnect-replay test is load-sensitive on windev; re-run isolated before treating it as a regression (documented in `docs/GatewayTesting.md`).
---
### Task 14: Wrap-up — deferred table closure, docs sweep, final review
**Classification:** small
**Parallelizable with:** none (last)
- Append a closure note to `docs/plans/2026-08-15-perf-review-remediation.md`'s deferred table (one line: resolved by this plan, date, branch).
- Sweep: `gateway.md` / `docs/WorkerFrameProtocol.md` / `docs/GatewayDashboardDesign.md` / `docs/GatewayTesting.md` consistency with as-built behavior; record any accepted deviations in THIS plan's "As-built notes" section (add it).
- Update `.tasks.json` statuses; update auto-memory (`perf-remediation-branch.md` or successor) with the branch state.
- Dispatch the final integration code review (Opus) over `git diff main..perf/deferred-remediation` before reporting done. Merge remains the user's decision.
---
## Explicitly out of scope
| Item | Why |
|---|---|
| wnwrap alarm GUID identity semantics; `ALARM_RECORDS/@COUNT` probe | Need live alarms on windev — external state this plan cannot provide. Still tracked in the prior plan's follow-ups. |
| 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`.
@@ -0,0 +1,20 @@
{
"planPath": "docs/plans/2026-08-15-deferred-remediation.md",
"tasks": [
{ "id": 1, "subject": "Task 1: Windows-safe cleanup in SecretsStorePathGuardTests", "status": "completed" },
{ "id": 2, "subject": "Task 2: SessionEventDistributor _subscribers to plain Dictionary", "status": "completed" },
{ "id": 3, "subject": "Task 3: Merge the session event-source pass-through iterator", "status": "completed" },
{ "id": 4, "subject": "Task 4: EventStreamService direct channel reads in the live loop", "status": "completed" },
{ "id": 5, "subject": "Task 5: In-process dashboard snapshot feed + page switch", "status": "completed" },
{ "id": 6, "subject": "Task 6: In-process session event subscription + SessionDetailsPage switch", "status": "completed" },
{ "id": 7, "subject": "Task 7: AlarmsPage provider-status via IGatewayAlarmService", "status": "completed" },
{ "id": 8, "subject": "Task 8: Dashboard design-doc update (consolidated)", "status": "completed", "blockedBy": [5, 6, 7] },
{ "id": 9, "subject": "Task 9: Phase A gate — full gateway suite on macOS", "status": "completed", "blockedBy": [1, 2, 3, 4, 5, 6, 7, 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": "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-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. | | 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. | | `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 ## Execution notes for the orchestrator
- Branch: `git checkout -b perf/review-remediation` before Task 1. - 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. effective configuration into immutable DTOs for read-only dashboard rendering.
The Blazor Server dashboard mounts at the host root and renders those snapshots The Blazor Server dashboard mounts at the host root and renders those snapshots
at `/`, `/sessions`, `/workers`, `/events`, `/galaxy`, `/alarms`, `/apikeys`, at `/`, `/sessions`, `/workers`, `/events`, `/galaxy`, `/alarms`, `/apikeys`,
and `/settings`. Pages connect to `/hubs/snapshot` (a SignalR hub published by and `/settings`. Pages run inside this process, so they consume the producing
`DashboardSnapshotPublisher`) and refresh on every push instead of polling. services directly through in-process seams — `IDashboardSnapshotFeed`,
`/hubs/alarms` broadcasts `AlarmFeedMessage` values from the central alarm `IDashboardSessionEventSubscriber`, and `IGatewayAlarmService` — and re-render on
monitor; `/hubs/events` mirrors per-session `MxEvent` traffic from every update instead of polling. The three SignalR hubs are the **remote**
`EventStreamService` to clients subscribed to `session:{id}`. The dashboard surface, for clients outside the gateway process: `/hubs/snapshot` pushes
uses local Bootstrap CSS and JavaScript plus a small local stylesheet; it does `DashboardSnapshot` from `DashboardSnapshotPublisher`, `/hubs/alarms` broadcasts
not use a Blazor UI component library. `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 `/browse` walks the `IGalaxyHierarchyCache` tree and reads subscribed tag
values live through `IDashboardLiveDataService`, which owns one shared, values live through `IDashboardLiveDataService`, which owns one shared,
@@ -1,80 +1,165 @@
using Microsoft.AspNetCore.Components; using Microsoft.AspNetCore.Components;
using Microsoft.AspNetCore.SignalR.Client; using Microsoft.Extensions.Logging;
using ZB.MOM.WW.MxGateway.Server.Dashboard.Hubs;
namespace ZB.MOM.WW.MxGateway.Server.Dashboard.Components; namespace ZB.MOM.WW.MxGateway.Server.Dashboard.Components;
/// <summary> /// <summary>
/// Base class for Blazor dashboard pages that watch gateway metrics /// Base class for Blazor dashboard pages that watch gateway metrics snapshots.
/// snapshots. The previous implementation polled /// Pages subscribe to the in-process <see cref="IDashboardSnapshotFeed"/>, which
/// <see cref="IDashboardSnapshotService.WatchSnapshotsAsync"/> directly; we /// multicasts a single <see cref="IDashboardSnapshotService.WatchSnapshotsAsync"/>
/// now subscribe to <see cref="DashboardSnapshotHub"/> so updates are /// enumeration to every circuit. An earlier implementation had each page open its
/// pushed and disconnects survive reconnects via SignalR's /// own SignalR connection to <c>/hubs/snapshot</c> — a loopback WebSocket back into
/// auto-reconnect. /// this same process, per page. The snapshot hub and its publisher remain for
/// external (non-circuit) clients; server-rendered pages no longer use them.
/// </summary> /// </summary>
public abstract class DashboardPageBase : ComponentBase, IAsyncDisposable public abstract class DashboardPageBase : ComponentBase, IAsyncDisposable
{ {
private HubConnection? _hub; /// <summary>
/// Upper bound on waiting for the watch loop while disposing. The loop marshals
/// renders through the renderer's dispatcher and disposal can run on that same
/// dispatcher, so the wait is bounded rather than unconditional.
/// </summary>
private static readonly TimeSpan WatchDrainTimeout = TimeSpan.FromSeconds(5);
/// <summary>Snapshot service used to seed the initial render before the hub connects.</summary> /// <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;
/// <summary>Snapshot service used to seed the initial render before the first feed update.</summary>
[Inject] [Inject]
protected IDashboardSnapshotService SnapshotService { get; set; } = null!; protected IDashboardSnapshotService SnapshotService { get; set; } = null!;
/// <summary>Factory that builds the SignalR connection (mints the hub bearer token).</summary> /// <summary>Shared in-process snapshot feed this page renders from.</summary>
[Inject] [Inject]
protected DashboardHubConnectionFactory HubFactory { get; set; } = null!; protected IDashboardSnapshotFeed SnapshotFeed { get; set; } = null!;
/// <summary>Logger used to report a snapshot subscription that ended or would not drain.</summary>
[Inject]
protected ILogger<DashboardPageBase>? Logger { get; set; }
/// <summary> /// <summary>
/// The most recent gateway metric snapshot. Synchronously seeded from /// The most recent gateway metric snapshot. Synchronously seeded from
/// <see cref="IDashboardSnapshotService.GetSnapshot"/> for the very /// <see cref="IDashboardSnapshotService.GetSnapshot"/> for the very first
/// first render, then refreshed by hub push. /// render, then refreshed from the feed.
/// </summary> /// </summary>
protected DashboardSnapshot? Snapshot { get; private set; } protected DashboardSnapshot? Snapshot { get; private set; }
/// <inheritdoc /> /// <inheritdoc />
protected override async Task OnInitializedAsync() protected override Task OnInitializedAsync()
{ {
Snapshot = SnapshotService.GetSnapshot(); Snapshot = SnapshotService.GetSnapshot();
await ConnectHubAsync().ConfigureAwait(false);
// Deliberately not awaited: the watch loop runs for the lifetime of the page
// and is cancelled and drained by DisposeAsync.
_watchTask = WatchSnapshotsAsync(_watchCancellation.Token);
return Task.CompletedTask;
} }
/// <summary>Disposes the SignalR hub connection created for this page, tolerating disposal-time errors.</summary> /// <summary>Cancels the snapshot subscription created for this page, tolerating disposal-time errors.</summary>
/// <returns>A task that represents the asynchronous operation.</returns> /// <returns>A task that represents the asynchronous operation.</returns>
public async ValueTask DisposeAsync() public async ValueTask DisposeAsync()
{ {
if (_hub is not null)
{
try
{
await _hub.DisposeAsync().ConfigureAwait(false);
}
catch
{
// Disposal-time errors are best-effort.
}
}
GC.SuppressFinalize(this);
}
private async Task ConnectHubAsync()
{
_hub = HubFactory.Create("/hubs/snapshot");
_hub.On<DashboardSnapshot>(DashboardSnapshotHub.SnapshotMessage, async snapshot =>
{
Snapshot = snapshot;
await InvokeAsync(StateHasChanged).ConfigureAwait(false);
});
try try
{ {
await _hub.StartAsync().ConfigureAwait(false); await _watchCancellation.CancelAsync().ConfigureAwait(false);
if (_watchTask is not null)
{
await _watchTask.WaitAsync(WatchDrainTimeout).ConfigureAwait(false);
}
}
catch (TimeoutException)
{
// Accepted limitation: the abandoned loop still holds its feed subscription, so
// the feed's idle gate stays open until it does unwind. There is no way to force
// a detach — the loop is parked on a dispatcher that is not draining — so the
// warning is the operator's only signal that a circuit teardown wedged.
Logger?.LogWarning(
"Dashboard page {Page} did not release its snapshot subscription within {Timeout}; "
+ "the shared snapshot feed stays active until it unwinds.",
GetType().Name,
WatchDrainTimeout);
} }
catch catch
{ {
// Hub is best-effort; the initial GetSnapshot() seed remains // Other disposal-time errors are best-effort.
// valid and the snapshot service keeps populating its cache for }
// the next reconnect cycle.
_watchCancellation.Dispose();
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)
{
// 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)
{
try
{
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;
}
} }
} }
} }
@@ -1,10 +1,9 @@
@page "/alarms" @page "/alarms"
@implements IAsyncDisposable @implements IAsyncDisposable
@using Microsoft.AspNetCore.SignalR.Client @using ZB.MOM.WW.MxGateway.Server.Alarms
@using ZB.MOM.WW.MxGateway.Server.Dashboard.Hubs
@inject IDashboardLiveDataService LiveData @inject IDashboardLiveDataService LiveData
@inject IOptions<GatewayOptions> GatewayOptions @inject IOptions<GatewayOptions> GatewayOptions
@inject DashboardHubConnectionFactory HubFactory @inject IGatewayAlarmService AlarmService
<PageTitle>Dashboard Alarms</PageTitle> <PageTitle>Dashboard Alarms</PageTitle>
@@ -169,17 +168,24 @@
private int _maxSeverity = 1000; private int _maxSeverity = 1000;
private string _search = string.Empty; 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 readonly CancellationTokenSource _cts = new();
private Task? _pollTask; private Task? _pollTask;
private DashboardAlarmProviderStatus _providerStatus = DashboardAlarmProviderStatus.Healthy; private DashboardAlarmProviderStatus _providerStatus = DashboardAlarmProviderStatus.Healthy;
private HubConnection? _alarmsHub; private Task? _providerStatusTask;
/// <inheritdoc /> /// <inheritdoc />
protected override void OnInitialized() protected override void OnInitialized()
{ {
_pollTask = PollLoopAsync(); _pollTask = PollLoopAsync();
_ = AttachAlarmsHubAsync(); _providerStatusTask = ProviderStatusLoopAsync();
} }
private string? ProviderStatusTitle() private string? ProviderStatusTitle()
@@ -189,26 +195,51 @@
: null; : null;
} }
private async Task AttachAlarmsHubAsync() // The badge tracks the central monitor directly rather than looping back through
// /hubs/alarms: the alarm service is an in-process multi-subscriber fan-out, so a
// server-rendered page needs no SignalR client, no loopback socket and no auth token.
// Alarm rows still come from the 3-second poll below — this loop only feeds the badge.
private async Task ProviderStatusLoopAsync()
{ {
_alarmsHub = HubFactory.Create("/hubs/alarms"); while (!_cts.IsCancellationRequested)
_alarmsHub.On<AlarmFeedMessage>(AlarmsHub.AlarmMessage, async message =>
{ {
if (message.PayloadCase == AlarmFeedMessage.PayloadOneofCase.ProviderStatus) try
{ {
_providerStatus = DashboardAlarmProviderStatus.FromFeed(message); await foreach (AlarmFeedMessage message in AlarmService
await InvokeAsync(StateHasChanged).ConfigureAwait(false); .StreamAsync(alarmFilterPrefix: null, _cts.Token)
} .ConfigureAwait(false))
}); {
if (message.PayloadCase != AlarmFeedMessage.PayloadOneofCase.ProviderStatus)
{
continue;
}
try await InvokeAsync(() =>
{ {
await _alarmsHub.StartAsync(_cts.Token).ConfigureAwait(false); _providerStatus = DashboardAlarmProviderStatus.FromFeed(message);
} StateHasChanged();
catch }).ConfigureAwait(false);
{ }
// The badge is best-effort; it stays at the healthy default until }
// the hub reconnects and delivers a fresh provider-status message. catch (OperationCanceledException)
{
return;
}
catch
{
// The monitor completes a subscriber's stream when it falls behind, and
// again when the monitor restarts. Both are recoverable by resubscribing;
// the badge holds its last value in the meantime.
}
try
{
await Task.Delay(TimeSpan.FromSeconds(1), _cts.Token).ConfigureAwait(false);
}
catch (OperationCanceledException)
{
return;
}
} }
} }
@@ -278,19 +309,76 @@
}; };
} }
// Fault handling sits inside the loop, matching ProviderStatusLoopAsync: a query or render
// fault on one tick is transient (a provider blip, a momentarily unavailable session), so it
// is surfaced on the page and retried on the next tick rather than ending polling for the
// life of the page. Cancellation is the only exit. The loop method itself therefore cannot
// fault, which is what DrainAsync in DisposeAsync relies on.
private async Task PollLoopAsync() private async Task PollLoopAsync()
{
if (!await PollOnceAsync().ConfigureAwait(false))
{
return;
}
using PeriodicTimer timer = new(TimeSpan.FromSeconds(3));
while (true)
{
try
{
if (!await timer.WaitForNextTickAsync(_cts.Token).ConfigureAwait(false))
{
return;
}
}
catch (OperationCanceledException)
{
return;
}
if (!await PollOnceAsync().ConfigureAwait(false))
{
return;
}
}
}
// Returns false only when cancellation has ended the poll; a non-cancellation fault returns
// true so the caller waits for the next tick and tries again.
private async Task<bool> PollOnceAsync()
{ {
try try
{ {
await InvokeAsync(RefreshAlarmsAsync).ConfigureAwait(false); await InvokeAsync(RefreshAlarmsAsync).ConfigureAwait(false);
using PeriodicTimer timer = new(TimeSpan.FromSeconds(3)); return true;
while (await timer.WaitForNextTickAsync(_cts.Token).ConfigureAwait(false))
{
await InvokeAsync(RefreshAlarmsAsync).ConfigureAwait(false);
}
} }
catch (OperationCanceledException) catch (OperationCanceledException)
{ {
return false;
}
catch (Exception ex)
{
await ReportPollFaultAsync(ex).ConfigureAwait(false);
return true;
}
}
private async Task ReportPollFaultAsync(Exception fault)
{
try
{
await InvokeAsync(() =>
{
_queryError = fault.Message;
StateHasChanged();
}).ConfigureAwait(false);
}
catch
{
// Reporting is best-effort: the fault being reported may itself be the teardown race
// this catch-all exists for — an InvokeAsync against a disposed renderer — in which
// case the dispatch fails the same way and there is no page left to show it on. The
// poll loop keeps ticking either way and exits on the next cancellation check.
} }
} }
@@ -310,32 +398,41 @@
/// <inheritdoc /> /// <inheritdoc />
public async ValueTask DisposeAsync() public async ValueTask DisposeAsync()
{ {
await _cts.CancelAsync(); await _cts.CancelAsync().ConfigureAwait(false);
if (_alarmsHub is not null) // 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
try // the real bound 10 seconds. DrainAsync tolerates a null task.
{ await Task.WhenAll(DrainAsync(_pollTask), DrainAsync(_providerStatusTask))
await _alarmsHub.DisposeAsync(); .ConfigureAwait(false);
}
catch
{
// Disposal-time errors are best-effort.
}
}
if (_pollTask is not null)
{
try
{
await _pollTask;
}
catch (OperationCanceledException)
{
}
}
_cts.Dispose(); _cts.Dispose();
GC.SuppressFinalize(this); 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,11 +1,15 @@
@page "/sessions/{SessionId}" @page "/sessions/{SessionId}"
@inherits DashboardPageBase @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 @implements IAsyncDisposable
@using Microsoft.AspNetCore.SignalR.Client
@using ZB.MOM.WW.MxGateway.Contracts.Proto @using ZB.MOM.WW.MxGateway.Contracts.Proto
@using ZB.MOM.WW.MxGateway.Server.Dashboard.Hubs @using ZB.MOM.WW.MxGateway.Server.Dashboard.Hubs
@inject AuthenticationStateProvider AuthenticationStateProvider @inject AuthenticationStateProvider AuthenticationStateProvider
@inject IDashboardSessionAdminService SessionAdminService @inject IDashboardSessionAdminService SessionAdminService
@inject IDashboardSessionEventSubscriber EventSubscriber
<PageTitle>Dashboard Session</PageTitle> <PageTitle>Dashboard Session</PageTitle>
@@ -113,8 +117,9 @@ else
@if (_recentEvents.Count == 0) @if (_recentEvents.Count == 0)
{ {
<div class="empty-state"> <div class="empty-state">
Waiting for events. The dashboard mirrors the session's gRPC event stream — events Waiting for events. The dashboard subscribes to this session's events directly, so
appear here only while a gRPC client is also consuming this session's events. rows appear as the session's worker emits them while this page is open — no gRPC
client has to be consuming the session.
</div> </div>
} }
else else
@@ -157,7 +162,18 @@ else
private DashboardSessionSummary? CurrentSession => Snapshot?.Sessions.FirstOrDefault(session => private DashboardSessionSummary? CurrentSession => Snapshot?.Sessions.FirstOrDefault(session =>
string.Equals(session.SessionId, SessionId, StringComparison.Ordinal)); string.Equals(session.SessionId, SessionId, StringComparison.Ordinal));
private HubConnection? _eventsHub; // Upper bound on waiting for the event pump while detaching, mirroring
// DashboardPageBase's snapshot-watch drain: the pump marshals renders through the
// renderer's dispatcher and a detach can run on that same dispatcher, so the wait
// is bounded rather than unconditional.
private static readonly TimeSpan EventPumpDrainTimeout = TimeSpan.FromSeconds(5);
// Written only on the renderer's dispatcher (the lifecycle methods below), and read
// on it from inside the pump's dispatched callback — that pairing is what makes the
// stale-batch guard in PumpEventsAsync reliable.
private IDashboardEventSubscription? _eventSubscription;
private CancellationTokenSource? _eventPumpCancellation;
private Task? _eventPumpTask;
private bool _eventsConnected; private bool _eventsConnected;
private string? _subscribedSessionId; private string? _subscribedSessionId;
private readonly LinkedList<MxEvent> _recentEvents = new(); private readonly LinkedList<MxEvent> _recentEvents = new();
@@ -183,8 +199,11 @@ else
{ {
if (!string.Equals(_subscribedSessionId, SessionId, StringComparison.Ordinal)) if (!string.Equals(_subscribedSessionId, SessionId, StringComparison.Ordinal))
{ {
await DetachEventsHubAsync().ConfigureAwait(false); // Deliberately no ConfigureAwait(false): the resumption must stay on the
await AttachEventsHubAsync().ConfigureAwait(false); // renderer's dispatcher so the new subscription is published to
// _eventSubscription from the same thread the pump's guard reads it on.
await DetachEventsAsync();
AttachEvents();
} }
} }
@@ -261,68 +280,160 @@ else
string ConfirmButtonClass, string ConfirmButtonClass,
Func<System.Security.Claims.ClaimsPrincipal, Task<DashboardSessionAdminResult>> Action); Func<System.Security.Claims.ClaimsPrincipal, Task<DashboardSessionAdminResult>> Action);
private async Task AttachEventsHubAsync() // The dashboard runs in the same process as the event mirror, so this page reads
// the session's mirrored events straight from it. It used to open a loopback
// SignalR connection to /hubs/events — mint a hub token, negotiate, hold a
// WebSocket, serialize every event — to reach data already sitting in memory.
// IDashboardSessionEventSubscriber resolves to the same singleton that serves
// IDashboardEventBroadcaster, and the subscription registers with
// EventsHubViewerRegistry, so the "nobody is watching" gate keeps working for
// both audiences.
// ACL posture is unchanged from the hub path: any dashboard Viewer may watch
// any session (SEC-25 tracks the per-session ACL for both seams).
private void AttachEvents()
{ {
if (string.IsNullOrWhiteSpace(SessionId)) if (string.IsNullOrWhiteSpace(SessionId))
{ {
return; return;
} }
_eventsHub = HubFactory.Create("/hubs/events"); _eventSubscription = EventSubscriber.Subscribe(SessionId);
_eventsHub.On<MxEvent>(EventsHub.EventMessage, async mxEvent => _eventPumpCancellation = new CancellationTokenSource();
{ _eventsConnected = true;
_recentEvents.AddFirst(mxEvent); _subscribedSessionId = SessionId;
while (_recentEvents.Count > MaxRecentEvents)
{
_recentEvents.RemoveLast();
}
await InvokeAsync(StateHasChanged).ConfigureAwait(false); // Deliberately not awaited: the pump runs for as long as the page watches this
}); // session and is cancelled and drained by DetachEventsAsync.
_eventPumpTask = PumpEventsAsync(_eventSubscription, _eventPumpCancellation.Token);
_eventsHub.Closed += _ => }
{
_eventsConnected = false;
return InvokeAsync(StateHasChanged);
};
_eventsHub.Reconnected += _ =>
{
_eventsConnected = true;
return InvokeAsync(StateHasChanged);
};
private async Task PumpEventsAsync(IDashboardEventSubscription subscription, CancellationToken cancellationToken)
{
try try
{ {
await _eventsHub.StartAsync().ConfigureAwait(false); while (await subscription.Reader.WaitToReadAsync(cancellationToken).ConfigureAwait(false))
await _eventsHub.SendAsync("SubscribeSession", SessionId).ConfigureAwait(false); {
_eventsConnected = true; // Drain what is queued and render once: a burst costs one render pass,
_subscribedSessionId = SessionId; // not one per event. Reading past the display cap would be wasted work,
// and anything left queued is picked up on the next pass.
List<MxEvent> batch = new();
while (batch.Count < MaxRecentEvents && subscription.Reader.TryRead(out MxEvent? mxEvent))
{
batch.Add(mxEvent);
}
if (batch.Count == 0)
{
continue;
}
await InvokeAsync(() =>
{
// The batch was read before this callback was dispatched, and a
// session switch can land in between. Rendering it then would show
// the previous session's events under the new session's heading, so
// a batch whose subscription is no longer the live one is dropped.
// Safe as an unsynchronized read: _eventSubscription is written on
// this same dispatcher.
if (!ReferenceEquals(_eventSubscription, subscription))
{
return;
}
foreach (MxEvent mxEvent in batch)
{
_recentEvents.AddFirst(mxEvent);
}
while (_recentEvents.Count > MaxRecentEvents)
{
_recentEvents.RemoveLast();
}
StateHasChanged();
}).ConfigureAwait(false);
}
} }
catch catch (OperationCanceledException)
{ {
_eventsConnected = false; // The page navigated to another session or was disposed.
}
catch (ObjectDisposedException)
{
// 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);
} }
} }
private async Task DetachEventsHubAsync() // 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)
{ {
HubConnection? hub = _eventsHub; try
_eventsHub = null; {
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()
{
IDashboardEventSubscription? subscription = _eventSubscription;
CancellationTokenSource? cancellation = _eventPumpCancellation;
Task? pump = _eventPumpTask;
_eventSubscription = null;
_eventPumpCancellation = null;
_eventPumpTask = null;
_eventsConnected = false; _eventsConnected = false;
_subscribedSessionId = null; _subscribedSessionId = null;
_recentEvents.Clear(); _recentEvents.Clear();
if (hub is not null) // Cancel and drop the subscription before draining. Disposing it releases the
// viewer registration — the whole point of the gate — and completes the channel,
// so the pump has an exit even if cancellation is missed.
cancellation?.Cancel();
subscription?.Dispose();
try
{ {
try if (pump is not null)
{ {
await hub.DisposeAsync().ConfigureAwait(false); await pump.WaitAsync(EventPumpDrainTimeout);
}
catch
{
// Disposal-time errors are best-effort.
} }
} }
catch
{
// Detach-time errors (including a drain timeout) are best-effort.
}
// Disposed after the drain so the pump is no longer reading the token.
cancellation?.Dispose();
} }
private static string EventStatusLabel(MxEvent evt) private static string EventStatusLabel(MxEvent evt)
@@ -332,9 +443,14 @@ else
: string.Empty; : 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() public new async ValueTask DisposeAsync()
{ {
await DetachEventsHubAsync().ConfigureAwait(false); await DetachEventsAsync();
await base.DisposeAsync().ConfigureAwait(false); await base.DisposeAsync().ConfigureAwait(false);
} }
} }
@@ -38,19 +38,32 @@ public static class DashboardServiceCollectionExtensions
services.AddZbLdapAuth(configuration, "MxGateway:Ldap"); services.AddZbLdapAuth(configuration, "MxGateway:Ldap");
services.AddSingleton<IDashboardSnapshotService, DashboardSnapshotService>(); services.AddSingleton<IDashboardSnapshotService, DashboardSnapshotService>();
services.AddSingleton<IDashboardSnapshotFeed, DashboardSnapshotFeed>();
services.AddSingleton<IDashboardLiveDataService, DashboardLiveDataService>(); services.AddSingleton<IDashboardLiveDataService, DashboardLiveDataService>();
services.AddSingleton<IDashboardAuthenticator, DashboardAuthenticator>(); services.AddSingleton<IDashboardAuthenticator, DashboardAuthenticator>();
services.AddSingleton<IGroupRoleMapper<string>, DashboardGroupRoleMapper>(); services.AddSingleton<IGroupRoleMapper<string>, DashboardGroupRoleMapper>();
services.AddSingleton<DashboardApiKeyAuthorization>(); services.AddSingleton<DashboardApiKeyAuthorization>();
services.AddSingleton<IDashboardApiKeyManagementService, DashboardApiKeyManagementService>(); services.AddSingleton<IDashboardApiKeyManagementService, DashboardApiKeyManagementService>();
services.AddSingleton<IDashboardSessionAdminService, DashboardSessionAdminService>(); 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.AddSingleton<HubTokenService>();
services.AddScoped<Hubs.DashboardHubConnectionFactory>();
services.AddScoped<IDashboardBrowseService, DashboardBrowseService>(); services.AddScoped<IDashboardBrowseService, DashboardBrowseService>();
// Singleton: EventsHub instances are transient (one per hub invocation), so the // Singleton: EventsHub instances are transient (one per hub invocation), so the
// subscriber bookkeeping they share with the broadcaster must outlive them. // subscriber bookkeeping they share with the broadcaster must outlive them.
services.AddSingleton<Hubs.EventsHubViewerRegistry>(); services.AddSingleton<Hubs.EventsHubViewerRegistry>();
services.AddSingleton<Hubs.IDashboardEventBroadcaster, Hubs.DashboardEventBroadcaster>();
// One instance behind two interfaces, registered concretely and forwarded: the
// publish side (IDashboardEventBroadcaster, driven by the session pipeline) and
// the in-process subscribe side (IDashboardSessionEventSubscriber, used by the
// session-details page) share subscriber bookkeeping, so resolving them to two
// instances would leave the page subscribed to a mirror nobody publishes to.
services.AddSingleton<Hubs.DashboardEventBroadcaster>();
services.AddSingleton<Hubs.IDashboardEventBroadcaster>(
static provider => provider.GetRequiredService<Hubs.DashboardEventBroadcaster>());
services.AddSingleton<Hubs.IDashboardSessionEventSubscriber>(
static provider => provider.GetRequiredService<Hubs.DashboardEventBroadcaster>());
services.AddSingleton<Hubs.DashboardSnapshotHubConnectionCounter>(); services.AddSingleton<Hubs.DashboardSnapshotHubConnectionCounter>();
services.AddHostedService<Hubs.DashboardSnapshotPublisher>(); services.AddHostedService<Hubs.DashboardSnapshotPublisher>();
services.AddHostedService<Hubs.AlarmsHubPublisher>(); services.AddHostedService<Hubs.AlarmsHubPublisher>();
@@ -0,0 +1,372 @@
using System.Runtime.CompilerServices;
using System.Threading.Channels;
using Microsoft.Extensions.Logging;
using Microsoft.Extensions.Logging.Abstractions;
namespace ZB.MOM.WW.MxGateway.Server.Dashboard;
/// <summary>
/// Fans one <see cref="IDashboardSnapshotService.WatchSnapshotsAsync"/> enumeration out to
/// every dashboard circuit. The underlying watch is not multicast — each enumeration owns a
/// timer and builds its own snapshot per tick — so subscribing per page would multiply the
/// snapshot cost by the number of open pages.
/// </summary>
/// <remarks>
/// <para>
/// The pump is idle-gated: it starts when the first subscriber arrives and is cancelled and
/// awaited when the last subscriber <em>of the live generation</em> leaves, so an unwatched
/// gateway runs no timer and builds no snapshots. Successive pumps are chained through
/// <c>_pumpTask</c>, so a rapid
/// unsubscribe/resubscribe restarts a fresh pump without ever running two enumerations at
/// once.
/// </para>
/// <para>
/// Every subscriber is tagged with the pump generation it joined under, and a dying pump only
/// ever detaches its own generation. A pump ends its generation the instant its source fails
/// or completes — before the (possibly slow) enumerator disposal — so a subscriber arriving
/// while a pump unwinds starts a fresh generation instead of silently attaching to a dead
/// pump that is about to detach everybody and leave nobody watching.
/// </para>
/// </remarks>
public sealed class DashboardSnapshotFeed : IDashboardSnapshotFeed
{
/// <summary>Generation value meaning "no pump is accepting subscribers".</summary>
private const long NoGeneration = 0;
private readonly IDashboardSnapshotService _snapshotService;
private readonly ILogger<DashboardSnapshotFeed> _logger;
private readonly object _gate = new();
private readonly List<Subscription> _subscribers = [];
/// <summary>
/// The most recent pump, completed while idle. A starting pump awaits its predecessor
/// before enumerating, which is what guarantees a single live enumeration.
/// </summary>
private Task _pumpTask = Task.CompletedTask;
/// <summary>Cancellation for the live pump; null when no generation is accepting subscribers.</summary>
private CancellationTokenSource? _pumpCancellation;
/// <summary>The generation new subscribers join, or <see cref="NoGeneration"/> when no pump is live.</summary>
private long _generation = NoGeneration;
/// <summary>Last generation handed out; only ever incremented under <c>_gate</c>.</summary>
private long _lastGeneration = NoGeneration;
/// <summary>Initializes a new instance of the <see cref="DashboardSnapshotFeed"/> class.</summary>
/// <param name="snapshotService">Snapshot source to multicast.</param>
/// <param name="logger">Optional logger for pump faults.</param>
public DashboardSnapshotFeed(
IDashboardSnapshotService snapshotService,
ILogger<DashboardSnapshotFeed>? logger = null)
{
_snapshotService = snapshotService ?? throw new ArgumentNullException(nameof(snapshotService));
_logger = logger ?? NullLogger<DashboardSnapshotFeed>.Instance;
}
/// <inheritdoc />
public async IAsyncEnumerable<DashboardSnapshot> WatchAsync(
[EnumeratorCancellation] CancellationToken cancellationToken)
{
// Capacity 1 + DropOldest: a viewer only ever wants the latest snapshot, so a
// circuit that renders slowly neither buffers without bound nor blocks the pump
// (TryWrite always succeeds) — it just skips the snapshots it was too slow for.
Channel<DashboardSnapshot> channel = Channel.CreateBounded<DashboardSnapshot>(
new BoundedChannelOptions(1)
{
FullMode = BoundedChannelFullMode.DropOldest,
SingleReader = true,
SingleWriter = false,
});
Subscription subscription = Subscribe(channel);
try
{
await foreach (DashboardSnapshot snapshot in channel.Reader
.ReadAllAsync(cancellationToken)
.ConfigureAwait(false))
{
yield return snapshot;
}
}
finally
{
// Untokened on purpose: teardown must run to completion even when this
// subscriber is unwinding because its own token fired.
await UnsubscribeAsync(subscription).ConfigureAwait(false);
}
}
private Subscription Subscribe(Channel<DashboardSnapshot> channel)
{
lock (_gate)
{
// A live generation is joined; otherwise this subscriber starts one. Keying on
// "is a generation live" rather than "is this the first subscriber" is what makes
// a subscriber arriving while a pump unwinds start a fresh pump for itself.
long generation = _pumpCancellation is null ? StartPumpLocked() : _generation;
Subscription subscription = new(channel, generation);
_subscribers.Add(subscription);
return subscription;
}
}
private async Task UnsubscribeAsync(Subscription subscription)
{
CancellationTokenSource? cancellation;
Task pump;
lock (_gate)
{
if (!_subscribers.Remove(subscription))
{
// The pump already detached this subscription (it completed or faulted).
return;
}
if (subscription.Generation != _generation)
{
// This viewer belonged to a generation that has already ended. The live
// pump — if there is one — serves other viewers and must not be cancelled
// on their behalf; the dying pump is stopping under its own steam.
return;
}
if (HasSubscribersLocked(_generation))
{
// Other viewers are still watching the live generation. Counting the whole
// list here would be wrong: subscribers of an ending generation linger in it
// until that pump's Reset runs, and they must not hold the idle gate open.
return;
}
cancellation = _pumpCancellation;
_pumpCancellation = null;
_generation = NoGeneration;
pump = _pumpTask;
}
try
{
cancellation?.Cancel();
}
catch (ObjectDisposedException)
{
// The pump ended on its own and disposed its cancellation source first.
}
try
{
await pump.ConfigureAwait(false);
}
catch (Exception)
{
// A pump fault has already been reported to the subscribers it had; the
// unsubscribing caller is only waiting for the enumeration to stop.
}
}
/// <summary>Reports whether any subscriber is still being served by a generation.</summary>
/// <param name="generation">The generation to look for.</param>
/// <returns>True when at least one subscriber carries that generation.</returns>
private bool HasSubscribersLocked(long generation)
{
foreach (Subscription subscriber in _subscribers)
{
if (subscriber.Generation == generation)
{
return true;
}
}
return false;
}
/// <summary>
/// Starts a pump generation. Must be called while holding <c>_gate</c>; the caller adds
/// the subscribers that belong to the returned generation.
/// </summary>
/// <returns>The new generation identifier.</returns>
private long StartPumpLocked()
{
long generation = ++_lastGeneration;
CancellationTokenSource cancellation = new();
Task previous = _pumpTask;
_generation = generation;
_pumpCancellation = cancellation;
// Task.Run, not a direct call: an async iterator runs synchronously up to its
// first suspension, and the first pull of the underlying watch can read the API
// key table. That must not run on the subscribing circuit's thread, let alone
// while this lock is held.
_pumpTask = Task.Run(() => PumpAsync(generation, previous, cancellation, cancellation.Token));
return generation;
}
private async Task PumpAsync(
long generation,
Task previous,
CancellationTokenSource cancellation,
CancellationToken cancellationToken)
{
try
{
// Never overlap with the enumeration this pump replaces.
await previous.ConfigureAwait(ConfigureAwaitOptions.SuppressThrowing);
// Enumerated by hand rather than with await foreach so the generation can be
// ended the moment the source fails or completes — await foreach would run the
// enumerator's disposal first, and a subscriber arriving during that disposal
// would join a generation that is already doomed.
IAsyncEnumerator<DashboardSnapshot> snapshots = _snapshotService
.WatchSnapshotsAsync(cancellationToken)
.GetAsyncEnumerator(cancellationToken);
try
{
while (true)
{
bool moved;
try
{
moved = await snapshots.MoveNextAsync().ConfigureAwait(false);
}
catch
{
EndGeneration(generation);
throw;
}
if (!moved)
{
EndGeneration(generation);
break;
}
Broadcast(generation, snapshots.Current);
}
}
finally
{
await snapshots.DisposeAsync().ConfigureAwait(false);
}
// The source completed on its own; hand the completion to this generation's
// subscribers and re-arm so the next one starts a fresh enumeration.
Reset(generation, error: null);
}
catch (OperationCanceledException) when (cancellationToken.IsCancellationRequested)
{
// The production DashboardSnapshotService swallows cancellation and yield-breaks,
// so normal teardown exits through the fall-through above (with an ownership-checked
// Reset that finds no subscribers); an implementation that propagates the token
// instead exits here. Both shapes end the generation exactly once.
EndGeneration(generation);
}
catch (Exception error)
{
_logger.LogWarning(error, "Dashboard snapshot feed stopped; the next subscriber restarts it.");
Reset(generation, error);
}
finally
{
cancellation.Dispose();
}
}
private void Broadcast(long generation, DashboardSnapshot snapshot)
{
lock (_gate)
{
foreach (Subscription subscriber in _subscribers)
{
if (subscriber.Generation != generation)
{
continue;
}
// Bounded/DropOldest: always accepted unless the channel is completed.
subscriber.Channel.Writer.TryWrite(snapshot);
}
}
}
/// <summary>
/// Stops handing <paramref name="generation"/> to new subscribers. Called the instant a
/// pump's source fails or completes, before its enumerator is disposed.
/// </summary>
/// <param name="generation">The generation that has ended.</param>
private void EndGeneration(long generation)
{
lock (_gate)
{
EndGenerationLocked(generation);
}
}
/// <summary>Clears the live-pump state if <paramref name="generation"/> still owns it.</summary>
/// <param name="generation">The generation that has ended.</param>
private void EndGenerationLocked(long generation)
{
if (_generation != generation)
{
return;
}
_generation = NoGeneration;
_pumpCancellation = null;
}
/// <summary>
/// Detaches the subscribers of a finished generation and re-arms the feed. Subscribers of
/// any other generation are left alone — they belong to a pump that is still running (or
/// about to), so a dying pump must not take them down with it.
/// </summary>
/// <param name="generation">The generation whose subscribers are being detached.</param>
/// <param name="error">Failure to surface, or null when the source completed cleanly.</param>
private void Reset(long generation, Exception? error)
{
List<Channel<DashboardSnapshot>> detached = [];
lock (_gate)
{
for (int index = _subscribers.Count - 1; index >= 0; index--)
{
if (_subscribers[index].Generation != generation)
{
continue;
}
detached.Add(_subscribers[index].Channel);
_subscribers.RemoveAt(index);
}
EndGenerationLocked(generation);
if (_subscribers.Count > 0 && _pumpCancellation is null)
{
// Belt and braces: subscribers left with no live pump would be frozen for
// good, because only a subscriber that finds no generation starts one.
long restarted = StartPumpLocked();
foreach (Subscription subscriber in _subscribers)
{
subscriber.Generation = restarted;
}
}
}
foreach (Channel<DashboardSnapshot> channel in detached)
{
channel.Writer.TryComplete(error);
}
}
/// <summary>One viewer's delivery channel plus the pump generation serving it.</summary>
/// <param name="channel">Delivery channel for this viewer.</param>
/// <param name="generation">Pump generation this viewer joined under.</param>
private sealed class Subscription(Channel<DashboardSnapshot> channel, long generation)
{
/// <summary>Gets the viewer's delivery channel.</summary>
public Channel<DashboardSnapshot> Channel { get; } = channel;
/// <summary>Gets or sets the pump generation currently serving this viewer.</summary>
public long Generation { get; set; } = generation;
}
}
@@ -14,10 +14,12 @@ namespace ZB.MOM.WW.MxGateway.Server.Dashboard;
/// <remarks> /// <remarks>
/// This service is registered as a singleton in /// This service is registered as a singleton in
/// <see cref="DashboardServiceCollectionExtensions.AddGatewayDashboard"/> and /// <see cref="DashboardServiceCollectionExtensions.AddGatewayDashboard"/> and
/// is shared by two consumer scopes: <c>DashboardHubConnectionFactory</c> /// is shared by two consumer scopes: the <c>/hubs/token</c> endpoint (calls
/// (scoped, per-circuit; calls <see cref="Issue"/> from the cookie-authenticated /// <see cref="Issue"/> for a cookie-authenticated caller) and
/// dashboard) and <c>HubTokenAuthenticationHandler</c> (transient, per-request; /// <c>HubTokenAuthenticationHandler</c> (transient, per-request; calls
/// calls <see cref="Validate"/> from the SignalR negotiate / connection path). /// <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 /// The underlying <see cref="ITimeLimitedDataProtector"/> is thread-safe, so
/// minting and validating concurrently from any number of callers is safe; /// minting and validating concurrently from any number of callers is safe;
/// future maintainers should preserve the singleton lifetime to keep the /// 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 // 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 // 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 // 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; // clients that re-fetch from /hubs/token on every (re)connect, which is what a remote hub
// see docs/GatewayDashboardDesign.md. Heavier jti-denylist revocation is deliberately // consumer is expected to do; see docs/GatewayDashboardDesign.md. Heavier jti-denylist
// deferred until per-session hub ACLs land, when tokens gain session binding. // revocation is deliberately deferred until per-session hub ACLs land, when tokens gain
// session binding.
internal static readonly TimeSpan TokenLifetime = TimeSpan.FromMinutes(5); internal static readonly TimeSpan TokenLifetime = TimeSpan.FromMinutes(5);
private readonly ITimeLimitedDataProtector _protector; private readonly ITimeLimitedDataProtector _protector;
@@ -1,3 +1,4 @@
using System.Threading.Channels;
using Microsoft.AspNetCore.SignalR; using Microsoft.AspNetCore.SignalR;
using Microsoft.Extensions.Options; using Microsoft.Extensions.Options;
using ZB.MOM.WW.MxGateway.Contracts.Proto; using ZB.MOM.WW.MxGateway.Contracts.Proto;
@@ -6,11 +7,13 @@ using ZB.MOM.WW.MxGateway.Server.Configuration;
namespace ZB.MOM.WW.MxGateway.Server.Dashboard.Hubs; namespace ZB.MOM.WW.MxGateway.Server.Dashboard.Hubs;
/// <summary> /// <summary>
/// Broadcasts MxEvents to <see cref="EventsHub"/> clients subscribed to the /// Broadcasts MxEvents to the two dashboard audiences for a session: remote
/// session's group. Fire-and-forget: we hand the send to the hub context /// <see cref="EventsHub"/> clients subscribed to the session's group, and
/// and return immediately so the source gRPC stream is never blocked. /// in-process subscribers opened through
/// Errors are logged once and dropped — keeping the SignalR mirror best-effort /// <see cref="IDashboardSessionEventSubscriber.Subscribe"/>. Fire-and-forget: we
/// preserves the gRPC contract that exists today. /// hand the send to the hub context and return immediately so the source gRPC
/// stream is never blocked. Errors are logged once and dropped — keeping the
/// SignalR mirror best-effort preserves the gRPC contract that exists today.
/// </summary> /// </summary>
/// <remarks> /// <remarks>
/// When <c>MxGateway:Dashboard:ShowTagValues</c> is false (the default), tag /// When <c>MxGateway:Dashboard:ShowTagValues</c> is false (the default), tag
@@ -23,7 +26,10 @@ namespace ZB.MOM.WW.MxGateway.Server.Dashboard.Hubs;
/// </remarks> /// </remarks>
/// <param name="hubContext">Hub context used to send to the session's group.</param> /// <param name="hubContext">Hub context used to send to the session's group.</param>
/// <param name="viewerRegistry"> /// <param name="viewerRegistry">
/// Live-subscriber registry consulted before any per-event work is done. /// Live-subscriber registry consulted before any per-event work is done. Both
/// audiences register here — hub connections by their SignalR connection id,
/// in-process subscriptions by a synthetic one — so the gate stays a single
/// source of truth.
/// </param> /// </param>
/// <param name="options">Gateway options supplying <c>Dashboard:ShowTagValues</c>.</param> /// <param name="options">Gateway options supplying <c>Dashboard:ShowTagValues</c>.</param>
/// <param name="logger">Logger for best-effort mirror failures.</param> /// <param name="logger">Logger for best-effort mirror failures.</param>
@@ -31,10 +37,36 @@ public sealed class DashboardEventBroadcaster(
IHubContext<EventsHub> hubContext, IHubContext<EventsHub> hubContext,
EventsHubViewerRegistry viewerRegistry, EventsHubViewerRegistry viewerRegistry,
IOptions<GatewayOptions> options, IOptions<GatewayOptions> options,
ILogger<DashboardEventBroadcaster> logger) : IDashboardEventBroadcaster ILogger<DashboardEventBroadcaster> logger) : IDashboardEventBroadcaster, IDashboardSessionEventSubscriber
{ {
/// <summary>
/// Queue depth per in-process subscriber. The consumer is a Blazor page
/// rendering the newest handful of events, so a burst it cannot keep up with
/// is dropped oldest-first rather than allowed to grow — same best-effort
/// contract the SignalR mirror already has.
/// </summary>
private const int InProcessQueueCapacity = 256;
private readonly bool _showTagValues = options.Value.Dashboard.ShowTagValues; private readonly bool _showTagValues = options.Value.Dashboard.ShowTagValues;
private readonly object _syncRoot = new();
/// <summary>
/// In-process subscribers per session. Values are treated as immutable once
/// stored: a subscribe or dispose swaps in a new array under
/// <see cref="_syncRoot"/>, so <see cref="Publish"/> can grab the reference
/// and write to it after releasing the lock.
/// </summary>
private readonly Dictionary<string, InProcessSubscription[]> _inProcessSubscribers =
new(StringComparer.Ordinal);
/// <summary>
/// Total live in-process subscribers, read without the lock so the common
/// case — nobody has a session-details page open — never contends on it.
/// Written only under <see cref="_syncRoot"/>.
/// </summary>
private int _inProcessSubscriberCount;
/// <inheritdoc /> /// <inheritdoc />
public void Publish(string sessionId, MxEvent mxEvent) public void Publish(string sessionId, MxEvent mxEvent)
{ {
@@ -55,6 +87,10 @@ public sealed class DashboardEventBroadcaster(
MxEvent outbound = _showTagValues ? mxEvent : RedactValues(mxEvent); MxEvent outbound = _showTagValues ? mxEvent : RedactValues(mxEvent);
// In-process delivery first: it is synchronous, cannot throw, and must not be
// skipped by the early return the hub send's guard clause takes.
DeliverInProcess(sessionId, outbound);
// Wrap the Task acquisition in a try/catch so a hypothetical synchronous throw // Wrap the Task acquisition in a try/catch so a hypothetical synchronous throw
// from SendAsync (e.g. an implementation that throws before returning the Task) // from SendAsync (e.g. an implementation that throws before returning the Task)
// cannot escape Publish. The interface contract is never-throw; fire-and-forget. // cannot escape Publish. The interface contract is never-throw; fire-and-forget.
@@ -85,6 +121,117 @@ public sealed class DashboardEventBroadcaster(
} }
} }
/// <inheritdoc />
public IDashboardEventSubscription Subscribe(string sessionId)
{
ArgumentException.ThrowIfNullOrWhiteSpace(sessionId);
// A synthetic connection id keeps the registry's per-connection bookkeeping
// usable for a subscriber that has no SignalR connection behind it. The
// "inproc-" prefix cannot collide with a SignalR connection id and makes the
// origin obvious in a debugger.
string connectionId = "inproc-" + Guid.NewGuid().ToString("N");
InProcessSubscription subscription = new(this, sessionId, connectionId, InProcessQueueCapacity);
// Register before the subscriber becomes a delivery target, exactly as
// EventsHub.SubscribeSession registers before joining the group: the reverse
// order would leave a window in which this subscriber is a delivery target but
// Publish's gate still reports the session unwatched, silently dropping events
// it should receive. The cost of this order is at worst a redaction clone that
// reaches nobody for the width of the window.
viewerRegistry.AddViewer(connectionId, sessionId);
lock (_syncRoot)
{
_inProcessSubscribers[sessionId] =
_inProcessSubscribers.TryGetValue(sessionId, out InProcessSubscription[]? existing)
? [.. existing, subscription]
: [subscription];
Volatile.Write(ref _inProcessSubscriberCount, _inProcessSubscriberCount + 1);
}
return subscription;
}
/// <summary>
/// Hands the already-redacted event to every in-process subscriber of the
/// session. Writes are non-blocking and lossy by construction, so this never
/// stalls the caller's event pipeline.
/// </summary>
/// <param name="sessionId">Session the event belongs to.</param>
/// <param name="outbound">The event as the dashboard should see it.</param>
private void DeliverInProcess(string sessionId, MxEvent outbound)
{
// The gate above admits hub-only viewers too, so check for in-process
// subscribers before touching the lock at all.
if (Volatile.Read(ref _inProcessSubscriberCount) == 0)
{
return;
}
InProcessSubscription[] subscribers;
lock (_syncRoot)
{
if (!_inProcessSubscribers.TryGetValue(sessionId, out InProcessSubscription[]? found))
{
return;
}
subscribers = found;
}
// The array is never mutated in place, so the writes happen outside the lock.
foreach (InProcessSubscription subscriber in subscribers)
{
subscriber.TryWrite(outbound);
}
}
/// <summary>
/// Removes a disposed subscription from the delivery map and releases its
/// viewer registration. Called at most once per subscription.
/// </summary>
/// <param name="subscription">The subscription being disposed.</param>
private void Unsubscribe(InProcessSubscription subscription)
{
// Drop the delivery target first and deregister after, mirroring
// EventsHub.UnsubscribeSession: the mirror stays enabled for the brief overlap
// rather than dropping events still owed to the session's other subscribers.
lock (_syncRoot)
{
if (_inProcessSubscribers.TryGetValue(subscription.SessionId, out InProcessSubscription[]? existing))
{
InProcessSubscription[] remaining =
[.. existing.Where(candidate => !ReferenceEquals(candidate, subscription))];
// Equal lengths mean it was never in this bucket, so the counter it
// would decrement is not its own to release.
if (remaining.Length != existing.Length)
{
if (remaining.Length == 0)
{
// Drop the key so the map does not grow one entry per session ever viewed.
_inProcessSubscribers.Remove(subscription.SessionId);
}
else
{
_inProcessSubscribers[subscription.SessionId] = remaining;
}
Volatile.Write(ref _inProcessSubscriberCount, _inProcessSubscriberCount - 1);
}
}
}
viewerRegistry.RemoveViewer(subscription.ConnectionId, subscription.SessionId);
// The synthetic connection id is used once and never reconnects, so nothing
// else will ever call ReleaseConnection for it; without this the registry
// would retain an empty per-connection entry per subscription ever opened.
viewerRegistry.ReleaseConnection(subscription.ConnectionId);
}
/// <summary> /// <summary>
/// Produces a deep clone of <paramref name="source"/> with every tag-value /// Produces a deep clone of <paramref name="source"/> with every tag-value
/// field cleared, leaving tag reference, quality, status, and timestamps /// field cleared, leaving tag reference, quality, status, and timestamps
@@ -107,4 +254,75 @@ public sealed class DashboardEventBroadcaster(
return redacted; return redacted;
} }
/// <summary>
/// One in-process subscriber's feed: a bounded, drop-oldest channel plus the
/// registry bookkeeping that keeps <see cref="Publish"/>'s viewer gate honest
/// while the feed is live.
/// </summary>
private sealed class InProcessSubscription : IDashboardEventSubscription
{
private readonly DashboardEventBroadcaster _owner;
private readonly Channel<MxEvent> _channel;
private int _disposed;
/// <summary>Initializes a new instance of the <see cref="InProcessSubscription"/> class.</summary>
/// <param name="owner">Broadcaster to deregister from on disposal.</param>
/// <param name="sessionId">Session whose events this subscription carries.</param>
/// <param name="connectionId">Synthetic connection id registered with the viewer registry.</param>
/// <param name="capacity">Queue depth before the oldest queued event is dropped.</param>
internal InProcessSubscription(
DashboardEventBroadcaster owner,
string sessionId,
string connectionId,
int capacity)
{
_owner = owner;
SessionId = sessionId;
ConnectionId = connectionId;
_channel = Channel.CreateBounded<MxEvent>(new BoundedChannelOptions(capacity)
{
// DropOldest, not Wait: a write must never block the gRPC event
// pipeline that calls Publish, and the newest events are the ones a
// live view wants.
FullMode = BoundedChannelFullMode.DropOldest,
SingleReader = true,
SingleWriter = false,
});
}
/// <inheritdoc />
public ChannelReader<MxEvent> Reader => _channel.Reader;
/// <summary>Gets the session this subscription is watching.</summary>
internal string SessionId { get; }
/// <summary>Gets the synthetic connection id held in the viewer registry.</summary>
internal string ConnectionId { get; }
/// <summary>
/// Queues an event for the subscriber, dropping the oldest queued event when
/// the reader has fallen behind. Never blocks and never throws.
/// </summary>
/// <param name="mxEvent">The event to queue.</param>
internal void TryWrite(MxEvent mxEvent) => _channel.Writer.TryWrite(mxEvent);
/// <summary>
/// Deregisters the subscription and completes its channel so a reader's
/// loop ends. Idempotent — a second call does nothing, so it can never
/// release a viewer count that a sibling subscription owns.
/// </summary>
public void Dispose()
{
if (Interlocked.Exchange(ref _disposed, 1) != 0)
{
return;
}
_owner.Unsubscribe(this);
_channel.Writer.TryComplete();
}
}
} }
@@ -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();
}
}
@@ -0,0 +1,26 @@
using System.Threading.Channels;
using ZB.MOM.WW.MxGateway.Contracts.Proto;
namespace ZB.MOM.WW.MxGateway.Server.Dashboard.Hubs;
/// <summary>
/// A live in-process feed of one session's dashboard-mirrored MxEvents, handed
/// out by <see cref="IDashboardSessionEventSubscriber.Subscribe"/>. Server-side
/// Blazor components read it directly instead of looping back through
/// <see cref="EventsHub"/> over a loopback SignalR connection.
/// </summary>
/// <remarks>
/// Events are delivered exactly as a hub client would see them — the same
/// redacted clone the group send carries, so <c>MxGateway:Dashboard:ShowTagValues</c>
/// governs both paths identically. The feed is a bounded, lossy queue: a
/// consumer that falls behind loses the oldest queued events, matching the
/// best-effort contract the SignalR mirror already has. Disposing the
/// subscription deregisters it, which is what lets the broadcaster go back to
/// skipping all mirror work for a session nobody is watching — so callers must
/// dispose. Dispose is idempotent.
/// </remarks>
public interface IDashboardEventSubscription : IDisposable
{
/// <summary>Gets the reader delivering this session's mirrored events.</summary>
ChannelReader<MxEvent> Reader { get; }
}
@@ -0,0 +1,34 @@
namespace ZB.MOM.WW.MxGateway.Server.Dashboard.Hubs;
/// <summary>
/// In-process subscription seam on the dashboard event mirror. Implemented by
/// <see cref="DashboardEventBroadcaster"/> alongside
/// <see cref="IDashboardEventBroadcaster"/>.
/// </summary>
/// <remarks>
/// The interactive-server dashboard runs in the same process as the broadcaster,
/// so a session-details page has no reason to open a loopback SignalR connection
/// back to <see cref="EventsHub"/> — mint a hub token, negotiate, hold a
/// WebSocket, and serialize every event — just to read events the broadcaster
/// already holds. It subscribes here instead. The registry gate stays honest
/// either way: an in-process subscription registers a synthetic connection id
/// with <see cref="EventsHubViewerRegistry"/> exactly as the hub registers a real
/// one, so <see cref="IDashboardEventBroadcaster.Publish"/> keeps skipping the
/// redaction clone for sessions nobody is watching.
/// <para>
/// It is a separate interface rather than a member of
/// <see cref="IDashboardEventBroadcaster"/> because publishing and consuming are
/// different roles: the session pipeline only ever publishes, and its test
/// doubles should not have to implement a subscription feed.
/// </para>
/// </remarks>
public interface IDashboardSessionEventSubscriber
{
/// <summary>Opens an in-process feed of the session's mirrored events.</summary>
/// <param name="sessionId">Session id whose events the caller wants.</param>
/// <returns>
/// The subscription. Dispose it to stop the feed and release the viewer
/// registration that keeps the mirror enabled for this session.
/// </returns>
IDashboardEventSubscription Subscribe(string sessionId);
}
@@ -0,0 +1,26 @@
namespace ZB.MOM.WW.MxGateway.Server.Dashboard;
/// <summary>
/// In-process multicast over <see cref="IDashboardSnapshotService.WatchSnapshotsAsync"/>.
/// One enumeration of the underlying watch is fanned out to every subscriber, so N
/// dashboard circuits cost one snapshot build per tick instead of N — and while nobody
/// subscribes, nothing runs at all.
/// </summary>
/// <remarks>
/// There is no authentication or authorization gate here: the feed is reached only from
/// Blazor dashboard components, whose endpoints already require
/// <see cref="DashboardAuthenticationDefaults.ViewerPolicy"/>, so every caller is a circuit
/// authorized as Viewer. Remote (non-circuit) consumers still go through
/// <c>/hubs/snapshot</c>, which applies the hub authorization policy itself.
/// </remarks>
public interface IDashboardSnapshotFeed
{
/// <summary>
/// Watches the shared snapshot stream. Each caller gets the snapshots produced while
/// it is subscribed; a caller that reads slowly sees only the newest snapshot rather
/// than a backlog, and never delays the other subscribers.
/// </summary>
/// <param name="cancellationToken">Token that ends this caller's subscription.</param>
/// <returns>An asynchronous stream of dashboard snapshots.</returns>
IAsyncEnumerable<DashboardSnapshot> WatchAsync(CancellationToken cancellationToken);
}
@@ -1,4 +1,5 @@
using System.Runtime.CompilerServices; using System.Runtime.CompilerServices;
using System.Threading.Channels;
using Microsoft.Extensions.Options; using Microsoft.Extensions.Options;
using ZB.MOM.WW.MxGateway.Contracts.Proto; using ZB.MOM.WW.MxGateway.Contracts.Proto;
using ZB.MOM.WW.MxGateway.Server.Configuration; using ZB.MOM.WW.MxGateway.Server.Configuration;
@@ -37,7 +38,7 @@ public sealed class EventStreamService(
// non-blocking. When this subscriber's channel is full the pump applies the per-subscriber // non-blocking. When this subscriber's channel is full the pump applies the per-subscriber
// backpressure policy and completes this subscriber's channel with a SessionManagerException // backpressure policy and completes this subscriber's channel with a SessionManagerException
// (SessionManagerErrorCode.EventQueueOverflow). That terminal fault surfaces here when the // (SessionManagerErrorCode.EventQueueOverflow). That terminal fault surfaces here when the
// reader's MoveNextAsync throws, and it propagates to the gRPC client unchanged. The overflow // reader's WaitToReadAsync throws, and it propagates to the gRPC client unchanged. The overflow
// metric, and (in the legacy single-subscriber FailFast case) the session fault + fault metric, // metric, and (in the legacy single-subscriber FailFast case) the session fault + fault metric,
// are recorded by the distributor's overflow handler so the session, the pump, and other // are recorded by the distributor's overflow handler so the session, the pump, and other
// subscribers are isolated from this subscriber's slowness. // subscribers are isolated from this subscriber's slowness.
@@ -106,9 +107,14 @@ public sealed class EventStreamService(
options.Value.Sessions.MaxEventSubscribersPerSession); options.Value.Sessions.MaxEventSubscribersPerSession);
} }
IAsyncEnumerator<MxEvent> reader = subscriber.Reader // Consume the subscriber channel directly (WaitToReadAsync + an inner TryRead drain)
.ReadAllAsync(cancellationToken) // rather than through ReadAllAsync's IAsyncEnumerable wrapper. This is the hottest
.GetAsyncEnumerator(cancellationToken); // per-event path in the gateway and the wrapper added a second async state machine hop
// per event for no behavioral benefit: WaitToReadAsync observes cancellation and a
// faulted completion exactly as MoveNextAsync did, and TryRead drains what is already
// buffered without allocating a wait. StreamEventsAsync itself stays an async iterator —
// its `yield return` is what feeds the gRPC writer.
ChannelReader<MxEvent> reader = subscriber.Reader;
// GWC-15: register this subscriber's channel as a live backlog source instead of // GWC-15: register this subscriber's channel as a live backlog source instead of
// reconciling the queue-depth gauge on every event. The gauge previously read the // reconciling the queue-depth gauge on every event. The gauge previously read the
@@ -151,15 +157,14 @@ public sealed class EventStreamService(
while (true) while (true)
{ {
MxEvent mxEvent; bool hasMore;
try try
{ {
if (!await reader.MoveNextAsync().ConfigureAwait(false)) // A cleanly completed channel returns false here (end of stream); a channel
{ // completed WITH a fault rethrows that fault from the wait once the buffer
break; // is drained — the same surface MoveNextAsync presented, so the terminal
} // SessionManagerException(EventQueueOverflow) still propagates unchanged.
hasMore = await reader.WaitToReadAsync(cancellationToken).ConfigureAwait(false);
mxEvent = reader.Current;
} }
catch (WorkerClientException workerException) catch (WorkerClientException workerException)
{ {
@@ -173,24 +178,36 @@ public sealed class EventStreamService(
throw; throw;
} }
// Per-RPC filter stays at the subscriber boundary: each request may resume if (!hasMore)
// from a different AfterWorkerSequence, so the shared pump fans raw events and
// this loop drops the ones at or below the caller's watermark.
if (mxEvent.WorkerSequence <= afterWorkerSequence)
{ {
continue; break;
} }
// The queue-depth gauge is maintained lazily via the backlog registration above // Drain everything already buffered before waiting again. TryRead never throws;
// (GWC-15): the metric reads this subscriber's channel Count only when scraped, // a fault left on the channel is observed by the next WaitToReadAsync above.
// so there is no per-event gauge bookkeeping on this hot path. while (reader.TryRead(out MxEvent? mxEvent))
yield return mxEvent; {
// Per-RPC filter stays at the subscriber boundary: each request may resume
// from a different AfterWorkerSequence, so the shared pump fans raw events
// and this loop drops the ones at or below the caller's watermark. It
// applies to every live event, drained or awaited alike.
if (mxEvent.WorkerSequence <= afterWorkerSequence)
{
continue;
}
// The queue-depth gauge is maintained lazily via the backlog registration
// above (GWC-15): the metric reads this subscriber's channel Count only when
// scraped, so there is no per-event gauge bookkeeping on this hot path.
yield return mxEvent;
}
} }
} }
finally finally
{ {
await reader.DisposeAsync().ConfigureAwait(false); // Nothing to dispose for the reader: consuming the ChannelReader directly means
// there is no enumerator wrapper holding the cancellation registration.
//
// Remove this subscriber's live backlog contribution before disposing the lease so // Remove this subscriber's live backlog contribution before disposing the lease so
// the gauge stops counting a channel that is about to be completed; after this the // the gauge stops counting a channel that is about to be completed; after this the
// gauge reflects only the remaining subscribers (zero when none remain). // gauge reflects only the remaining subscribers (zero when none remain).
@@ -764,11 +764,24 @@ public sealed class GatewaySession
// The distributor's single event source. Drains the worker event stream once (the // The distributor's single event source. Drains the worker event stream once (the
// distributor guarantees a single consumer) and maps each frame to the public MxEvent, // distributor guarantees a single consumer) and maps each frame to the public MxEvent,
// preserving worker order. Mirrors the former ProduceEventsAsync mapping exactly. // preserving worker order. Mirrors the former ProduceEventsAsync mapping exactly.
//
// This deliberately duplicates the three lines of ReadEventsAsync rather than enumerating
// it: every worker event crosses this source, and routing it through a second pure
// pass-through iterator cost two extra MoveNextAsync state-machine hops per event for no
// semantic value. ReadEventsAsync stays for ISessionManager.ReadEventsAsync; keep the two
// bodies in step. Only one of them may run per attach — WorkerClient.ReadEventsAsync
// single-reader-claims the event channel and throws on a second consumer — and on the
// distributor path that one consumer is this method.
private async IAsyncEnumerable<MxEvent> MapWorkerEventsAsync( private async IAsyncEnumerable<MxEvent> MapWorkerEventsAsync(
[EnumeratorCancellation] CancellationToken cancellationToken) [EnumeratorCancellation] CancellationToken cancellationToken)
{ {
MxAccessGrpcMapper mapper = _eventStreaming.Mapper; MxAccessGrpcMapper mapper = _eventStreaming.Mapper;
await foreach (WorkerEvent workerEvent in ReadEventsAsync(cancellationToken) IWorkerClient workerClient = await GetReadyWorkerClientAsync(cancellationToken).ConfigureAwait(false);
TouchClientActivity(_eventStreaming.TimeProvider.GetUtcNow());
await foreach (WorkerEvent workerEvent in workerClient
.ReadEventsAsync(cancellationToken)
.WithCancellation(cancellationToken)
.ConfigureAwait(false)) .ConfigureAwait(false))
{ {
yield return mapper.MapEvent(workerEvent); yield return mapper.MapEvent(workerEvent);
@@ -1513,6 +1526,13 @@ public sealed class GatewaySession
/// Reads events from the worker as an asynchronous enumerable stream. /// Reads events from the worker as an asynchronous enumerable stream.
/// </summary> /// </summary>
/// <param name="cancellationToken">Token to cancel the asynchronous operation.</param> /// <param name="cancellationToken">Token to cancel the asynchronous operation.</param>
/// <remarks>
/// Backs <c>ISessionManager.ReadEventsAsync</c>. The distributor does <em>not</em> come
/// through here — <c>MapWorkerEventsAsync</c> inlines this body to save a per-event
/// iterator hop, so changes made here belong there too. The two are mutually exclusive
/// per attach: <see cref="IWorkerClient.ReadEventsAsync"/> claims the worker event
/// channel for a single reader and throws on the second consumer.
/// </remarks>
/// <returns>An asynchronous stream of worker events.</returns> /// <returns>An asynchronous stream of worker events.</returns>
public async IAsyncEnumerable<WorkerEvent> ReadEventsAsync( public async IAsyncEnumerable<WorkerEvent> ReadEventsAsync(
[EnumeratorCancellation] CancellationToken cancellationToken) [EnumeratorCancellation] CancellationToken cancellationToken)
@@ -1,4 +1,3 @@
using System.Collections.Concurrent;
using System.Threading.Channels; using System.Threading.Channels;
using ZB.MOM.WW.MxGateway.Contracts.Proto; using ZB.MOM.WW.MxGateway.Contracts.Proto;
@@ -57,27 +56,29 @@ public delegate void SubscriberOverflowHandler(bool isOnlySubscriber, bool isInt
/// <see cref="Func{T, TResult}"/> producing an /// <see cref="Func{T, TResult}"/> producing an
/// <see cref="IAsyncEnumerable{T}"/> of already-mapped public /// <see cref="IAsyncEnumerable{T}"/> of already-mapped public
/// <see cref="MxEvent"/>s, given a <see cref="CancellationToken"/>. This is the /// <see cref="MxEvent"/>s, given a <see cref="CancellationToken"/>. This is the
/// cleanest seam: it can pass /// cleanest seam: production passes <c>GatewaySession.MapWorkerEventsAsync</c>,
/// <c>ct =&gt; session.ReadEventsAsync(ct).Select(mapper.MapEvent)</c> (or a /// which reads the worker event channel and maps each frame in one iterator
/// channel reader's <c>ReadAllAsync</c>), while unit tests pass a plain /// (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 /// 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 /// single consumption of this enumerable; fan-out happens on the public
/// <see cref="MxEvent"/> after mapping, mirroring today's /// <see cref="MxEvent"/> after mapping, mirroring today's
/// <c>EventStreamService.ProduceEventsAsync</c> ordering. /// <c>EventStreamService.ProduceEventsAsync</c> ordering.
/// </para> /// </para>
/// <para> /// <para>
/// <b>Concurrency.</b> The subscriber set is a /// <b>Concurrency.</b> The subscriber set is a plain
/// <see cref="ConcurrentDictionary{TKey, TValue}"/> keyed by a monotonic id, used /// <see cref="Dictionary{TKey, TValue}"/> keyed by a monotonic id, used for keyed
/// for keyed add/remove only. Fan-out does NOT enumerate the dictionary: every /// add/remove only. It needs no concurrent collection type because it is never
/// mutation (<see cref="Register"/>, <see cref="RegisterWithReplay"/>, lease /// touched outside the <c>_lifecycleLock</c> critical section: every mutation
/// disposal, overflow disconnect) happens inside the <c>_lifecycleLock</c> critical /// (<see cref="Register"/>, <see cref="RegisterWithReplay"/>, lease disposal,
/// section and rebuilds an immutable copy-on-write <c>Subscriber[]</c> snapshot, /// overflow disconnect) and every read (the terminal completion sweep, the snapshot
/// which the pump reads once per event. This matters because /// rebuild) holds that lock, and each mutation rebuilds an immutable copy-on-write
/// <c>ConcurrentDictionary.Values</c> is a PROPERTY that acquires every internal /// <c>Subscriber[]</c> snapshot inside the same section. The lock-free readers see only that snapshot,
/// lock and materializes a fresh <c>List</c> plus a read-only wrapper on each call /// never the dictionary: the pump reads it once per event and
/// — per event, on the hot fan-out path. The subscriber set is tiny (one to a /// <see cref="SubscriberCount"/> reads its length. Fan-out therefore does NOT
/// handful) and mutates rarely, so paying a full array rebuild per registration to /// enumerate the dictionary — it walks a captured array, with no dictionary
/// make fan-out a bare array walk is the right trade. No lock is held across an /// traversal and no per-event allocation on the hot path. The subscriber set is
/// tiny (one to a handful) and mutates rarely, so paying a full array rebuild per
/// registration to buy that is the right trade. No lock is held across an
/// <c>await</c>. Each subscriber channel has a single writer — the pump — so /// <c>await</c>. Each subscriber channel has a single writer — the pump — so
/// per-channel writes never race. A subscriber registered after the pump captured /// per-channel writes never race. A subscriber registered after the pump captured
/// the array for the in-flight event misses that event, which matches "late /// the array for the in-flight event misses that event, which matches "late
@@ -104,7 +105,11 @@ public sealed class SessionEventDistributor : IAsyncDisposable
private readonly TimeSpan _shutdownTimeout; private readonly TimeSpan _shutdownTimeout;
private readonly ILogger<SessionEventDistributor> _logger; private readonly ILogger<SessionEventDistributor> _logger;
private readonly TimeProvider _timeProvider; private readonly TimeProvider _timeProvider;
private readonly ConcurrentDictionary<long, Subscriber> _subscribers = new(); // Keyed subscriber set. Touched ONLY under _lifecycleLock (add in RegisterSubscriber and
// RegisterWithReplay, remove in RemoveSubscriber, read in CompleteAllSubscribers and
// RebuildSubscriberSnapshot), which is why a plain Dictionary suffices: lock-free readers
// never see this field, they read _subscriberSnapshot below.
private readonly Dictionary<long, Subscriber> _subscribers = [];
private readonly CancellationTokenSource _shutdownCts = new(); private readonly CancellationTokenSource _shutdownCts = new();
private readonly object _lifecycleLock = new(); private readonly object _lifecycleLock = new();
@@ -120,7 +125,7 @@ public sealed class SessionEventDistributor : IAsyncDisposable
// may legitimately observe the previous array, which IS the documented "late subscribers // may legitimately observe the previous array, which IS the documented "late subscribers
// see events after they register" window. Where visibility must be guaranteed — the // see events after they register" window. Where visibility must be guaranteed — the
// RegisterWithReplay handoff — it comes from the _replayLock edge, not from Volatile. // RegisterWithReplay handoff — it comes from the _replayLock edge, not from Volatile.
// See the type remarks for why fan-out must not touch ConcurrentDictionary.Values. // See the type remarks for why fan-out walks this array instead of enumerating _subscribers.
private Subscriber[] _subscriberSnapshot = []; private Subscriber[] _subscriberSnapshot = [];
// Replay ring buffer. Appended on the pump thread and queried from arbitrary // Replay ring buffer. Appended on the pump thread and queried from arbitrary
@@ -295,9 +300,10 @@ public sealed class SessionEventDistributor : IAsyncDisposable
/// (gRPC) subscribers and excludes the internal dashboard subscriber. /// (gRPC) subscribers and excludes the internal dashboard subscriber.
/// </summary> /// </summary>
/// <remarks> /// <remarks>
/// Read from the copy-on-write snapshot rather than <c>ConcurrentDictionary.Count</c> /// Read from the copy-on-write snapshot rather than the dictionary, because this
/// (which acquires every internal lock). The snapshot is rebuilt in the same /// property is a lock-free reader and the dictionary may only be touched under
/// <c>_lifecycleLock</c> section that mutates the dictionary, so the two never diverge. /// <c>_lifecycleLock</c>. The snapshot is rebuilt in the same <c>_lifecycleLock</c>
/// section that mutates the dictionary, so the two never diverge.
/// </remarks> /// </remarks>
public int SubscriberCount => Volatile.Read(ref _subscriberSnapshot).Length; public int SubscriberCount => Volatile.Read(ref _subscriberSnapshot).Length;
@@ -648,7 +654,7 @@ public sealed class SessionEventDistributor : IAsyncDisposable
// register". A subscriber UNREGISTERED after the capture is still written to, // register". A subscriber UNREGISTERED after the capture is still written to,
// and TryWrite on its completed channel returns false — from here that is // and TryWrite on its completed channel returns false — from here that is
// indistinguishable from a real overflow. The window predates the // indistinguishable from a real overflow. The window predates the
// copy-on-write array (ConcurrentDictionary.Values materialized its list up // copy-on-write array (enumerating the dictionary materialized its values up
// front too) and its outcome is NOT benign, so telling a graceful unregister // front too) and its outcome is NOT benign, so telling a graceful unregister
// apart from a genuine overflow is OnSubscriberOverflow's job, not this // apart from a genuine overflow is OnSubscriberOverflow's job, not this
// loop's. // loop's.
@@ -745,9 +751,9 @@ public sealed class SessionEventDistributor : IAsyncDisposable
} }
// Disconnect ONLY this subscriber: it is already out of the fan-out set (removed above), // 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 // so complete its channel with the overflow fault. EventStreamService consumes the channel
// throws the SessionManagerException, which EventStreamService surfaces to the client // directly, so its next WaitToReadAsync throws the SessionManagerException, which it surfaces
// exactly as the pre-epic per-RPC overflow did. The pump and every other subscriber are // 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 // untouched. This runs even when the handler above threw — the subscriber must never be
// left attached with an un-completed channel. // left attached with an un-completed channel.
subscriber.Channel.Writer.TryComplete(new SessionManagerException( subscriber.Channel.Writer.TryComplete(new SessionManagerException(
@@ -796,7 +802,7 @@ public sealed class SessionEventDistributor : IAsyncDisposable
{ {
lock (_lifecycleLock) lock (_lifecycleLock)
{ {
if (!_subscribers.TryRemove(subscriber.Id, out _)) if (!_subscribers.Remove(subscriber.Id))
{ {
return false; return false;
} }
@@ -808,7 +814,8 @@ public sealed class SessionEventDistributor : IAsyncDisposable
// Republishes the fan-out array from the current dictionary contents. MUST be called with // Republishes the fan-out array from the current dictionary contents. MUST be called with
// _lifecycleLock held — holding that lock across the dictionary mutation and this rebuild is // _lifecycleLock held — holding that lock across the dictionary mutation and this rebuild is
// what keeps the array and the dictionary from diverging. // what keeps the array and the dictionary from diverging, and it is also what makes the plain
// (non-concurrent) Dictionary safe: this enumeration never races a mutation.
private void RebuildSubscriberSnapshot() private void RebuildSubscriberSnapshot()
=> Volatile.Write(ref _subscriberSnapshot, [.. _subscribers.Values]); => Volatile.Write(ref _subscriberSnapshot, [.. _subscribers.Values]);
@@ -27,7 +27,6 @@
<PackageReference Include="Serilog.AspNetCore" Version="10.0.0" /> <PackageReference Include="Serilog.AspNetCore" Version="10.0.0" />
<PackageReference Include="Serilog.Sinks.Console" Version="6.1.1" /> <PackageReference Include="Serilog.Sinks.Console" Version="6.1.1" />
<PackageReference Include="Serilog.Sinks.File" Version="7.0.0" /> <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" /> <PackageReference Include="Microsoft.Data.Sqlite" Version="10.0.7" />
<!-- Security pin: GHSA-2m69-gcr7-jv3q. Microsoft.Data.Sqlite pulls the vulnerable <!-- 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. --> native 2.1.11; 2.1.12 patches it. Bumping Sqlite does not clear it. -->
@@ -100,7 +100,24 @@ public sealed class SecretsStorePathGuardTests
finally finally
{ {
Environment.SetEnvironmentVariable(SqlitePathVariable, original); Environment.SetEnvironmentVariable(SqlitePathVariable, original);
Directory.Delete(directory, recursive: true);
// The store runs in WAL mode with connection pooling, so a pooled handle can outlive the
// migration and keep secrets.db (plus its -wal/-shm sidecars) open. Windows refuses to
// delete a directory holding open files where Unix does not, so clear the pool first;
// the catch is belt-and-braces for a sidecar whose handle outlasts even that.
Microsoft.Data.Sqlite.SqliteConnection.ClearAllPools();
try
{
Directory.Delete(directory, recursive: true);
}
catch (IOException)
{
// Best-effort cleanup of the temp store; a locked file must not fail the test.
}
catch (UnauthorizedAccessException)
{
// Best-effort cleanup of the temp store; a locked file must not fail the test.
}
} }
} }
@@ -121,6 +121,140 @@ public sealed class DashboardEventBroadcasterTests
Assert.NotNull(sent.OnAlarmTransition.LimitValue); Assert.NotNull(sent.OnAlarmTransition.LimitValue);
} }
/// <summary>
/// An in-process subscriber gets the same redacted clone the hub group gets,
/// and the shared source event is still left untouched.
/// </summary>
[Fact]
public void Subscribe_WhenShowTagValuesFalse_DeliversRedactedCloneWithoutMutatingSource()
{
CapturingHubContext hubContext = new();
EventsHubViewerRegistry viewers = new();
DashboardEventBroadcaster broadcaster = Create(hubContext, showTagValues: false, viewers);
using IDashboardEventSubscription subscription = broadcaster.Subscribe("session-1");
MxEvent source = BuildEventWithValue();
broadcaster.Publish("session-1", source);
MxEvent received = ReadOne(subscription);
Assert.Null(received.Value);
Assert.Null(received.OnAlarmTransition.CurrentValue);
Assert.Null(received.OnAlarmTransition.LimitValue);
Assert.Equal("Tank01.Level.HiHi", received.OnAlarmTransition.AlarmFullReference);
// One clone feeds both audiences — the hub group and the in-process feed.
Assert.Same(hubContext.LastArgument, received);
// The source is shared with the gRPC stream and the replay ring.
Assert.NotSame(source, received);
Assert.NotNull(source.Value);
Assert.Equal(42.5, source.Value.DoubleValue);
Assert.NotNull(source.OnAlarmTransition.CurrentValue);
Assert.NotNull(source.OnAlarmTransition.LimitValue);
}
/// <summary>An in-process subscription opens the viewer gate the same way a hub client does.</summary>
[Fact]
public void Subscribe_OpensTheViewerGate()
{
CapturingHubContext hubContext = new();
EventsHubViewerRegistry viewers = new();
DashboardEventBroadcaster broadcaster = Create(hubContext, showTagValues: false, viewers);
broadcaster.Publish("session-1", BuildEventWithValue());
Assert.Equal(0, hubContext.SendCount);
Assert.Null(hubContext.LastArgument);
using IDashboardEventSubscription subscription = broadcaster.Subscribe("session-1");
Assert.True(viewers.HasViewers("session-1"));
broadcaster.Publish("session-1", BuildEventWithValue());
Assert.Equal(1, hubContext.SendCount);
Assert.NotNull(hubContext.LastArgument);
}
/// <summary>Disposing the last in-process subscription restores the no-viewers short-circuit.</summary>
[Fact]
public void Dispose_OfLastInProcessSubscription_RestoresTheShortCircuit()
{
CapturingHubContext hubContext = new();
EventsHubViewerRegistry viewers = new();
DashboardEventBroadcaster broadcaster = Create(hubContext, showTagValues: false, viewers);
IDashboardEventSubscription subscription = broadcaster.Subscribe("session-1");
broadcaster.Publish("session-1", BuildEventWithValue());
Assert.Equal(1, hubContext.SendCount);
Assert.Equal("session-1", ReadOne(subscription).SessionId);
subscription.Dispose();
Assert.False(viewers.HasViewers("session-1"));
broadcaster.Publish("session-1", BuildEventWithValue());
Assert.Equal(1, hubContext.SendCount);
Assert.False(subscription.Reader.TryRead(out _));
}
/// <summary>Hub viewers and in-process subscribers are audiences of their own session only.</summary>
[Fact]
public void Publish_DeliversOnlyToTheSubscribedSessionsAudience()
{
CapturingHubContext hubContext = new();
EventsHubViewerRegistry viewers = new();
viewers.AddViewer("conn-1", "session-1");
DashboardEventBroadcaster broadcaster = Create(hubContext, showTagValues: false, viewers);
using IDashboardEventSubscription subscription = broadcaster.Subscribe("session-2");
// The hub viewer's session must not spill into the in-process feed.
broadcaster.Publish("session-1", BuildEventWithValue());
Assert.Equal(1, hubContext.SendCount);
Assert.False(subscription.Reader.TryRead(out _));
broadcaster.Publish("session-2", BuildEventWithValue("session-2"));
Assert.Equal("session-2", ReadOne(subscription).SessionId);
// A session with no audience at all still short-circuits.
broadcaster.Publish("session-3", BuildEventWithValue("session-3"));
Assert.Equal(2, hubContext.SendCount);
}
/// <summary>Disposing twice is a no-op and cannot release a sibling subscription's registration.</summary>
[Fact]
public void Dispose_CalledTwice_IsSafeAndLeavesSiblingSubscriptionsAlone()
{
CapturingHubContext hubContext = new();
EventsHubViewerRegistry viewers = new();
DashboardEventBroadcaster broadcaster = Create(hubContext, showTagValues: false, viewers);
IDashboardEventSubscription first = broadcaster.Subscribe("session-1");
using IDashboardEventSubscription second = broadcaster.Subscribe("session-1");
first.Dispose();
first.Dispose();
Assert.True(viewers.HasViewers("session-1"));
broadcaster.Publish("session-1", BuildEventWithValue());
Assert.Equal(1, hubContext.SendCount);
Assert.False(first.Reader.TryRead(out _));
Assert.Equal("session-1", ReadOne(second).SessionId);
}
/// <summary>Reads exactly one event from a subscription, failing the test if none is queued.</summary>
/// <param name="subscription">The subscription to read from.</param>
/// <returns>The event that was read.</returns>
private static MxEvent ReadOne(IDashboardEventSubscription subscription)
{
Assert.True(subscription.Reader.TryRead(out MxEvent? received));
return Assert.IsType<MxEvent>(received);
}
private static DashboardEventBroadcaster Create( private static DashboardEventBroadcaster Create(
CapturingHubContext hubContext, CapturingHubContext hubContext,
bool showTagValues, bool showTagValues,
@@ -147,12 +281,15 @@ public sealed class DashboardEventBroadcasterTests
return viewers; return viewers;
} }
private static MxEvent BuildEventWithValue() /// <summary>Builds a value-bearing alarm-transition event for the given session.</summary>
/// <param name="sessionId">Session id stamped on the event.</param>
/// <returns>The event.</returns>
private static MxEvent BuildEventWithValue(string sessionId = "session-1")
{ {
return new MxEvent return new MxEvent
{ {
Family = MxEventFamily.OnAlarmTransition, Family = MxEventFamily.OnAlarmTransition,
SessionId = "session-1", SessionId = sessionId,
ServerHandle = 7, ServerHandle = 7,
ItemHandle = 11, ItemHandle = 11,
Quality = 192, Quality = 192,
@@ -27,22 +27,38 @@ public sealed class DashboardHubsRegistrationTests
endpoint.Metadata.GetMetadata<IEndpointNameMetadata>()?.EndpointName == "DashboardHubToken"); 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> /// <returns>A task that represents the asynchronous operation.</returns>
[Fact] [Fact]
public async Task Build_WhenDashboardEnabled_RegistersHubTokenServiceAndConnectionFactory() public async Task Build_WhenDashboardEnabled_RegistersHubTokenService()
{ {
await using WebApplication app = GatewayApplication.Build([]); 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>(); HubTokenService tokens = app.Services.GetRequiredService<HubTokenService>();
Assert.NotNull(tokens); Assert.NotNull(tokens);
}
using IServiceScope scope = app.Services.CreateScope(); /// <summary>
DashboardHubConnectionFactory factory = scope.ServiceProvider /// The publish and in-process subscribe faces of the event mirror must resolve to
.GetRequiredService<DashboardHubConnectionFactory>(); /// one instance: two would leave the session-details page reading a mirror the
Assert.NotNull(factory); /// session pipeline never publishes to, and the viewer gate would never open.
/// </summary>
/// <returns>A task that represents the asynchronous operation.</returns>
[Fact]
public async Task Build_WhenDashboardEnabled_ResolvesBothEventMirrorInterfacesToOneInstance()
{
await using WebApplication app = GatewayApplication.Build([]);
IDashboardEventBroadcaster broadcaster = app.Services.GetRequiredService<IDashboardEventBroadcaster>();
IDashboardSessionEventSubscriber subscriber = app.Services
.GetRequiredService<IDashboardSessionEventSubscriber>();
Assert.Same(broadcaster, subscriber);
Assert.Same(app.Services.GetRequiredService<DashboardEventBroadcaster>(), broadcaster);
} }
} }
@@ -0,0 +1,522 @@
using System.Runtime.CompilerServices;
using System.Threading.Channels;
using ZB.MOM.WW.MxGateway.Server.Dashboard;
namespace ZB.MOM.WW.MxGateway.Tests.Gateway.Dashboard;
/// <summary>
/// Covers the in-process snapshot fan-out that replaced the dashboard pages'
/// loopback <c>/hubs/snapshot</c> connections. The invariants under test are the
/// ones that make the feed cheaper than the hub hop: exactly one underlying
/// <see cref="IDashboardSnapshotService.WatchSnapshotsAsync"/> enumeration for any
/// number of viewers, nothing at all while nobody is watching, and a slow viewer
/// that can neither buffer without bound nor stall the others.
/// </summary>
public sealed class DashboardSnapshotFeedTests
{
private static readonly TimeSpan TestTimeout = TimeSpan.FromSeconds(5);
/// <summary>
/// With no page subscribed, the feed must not touch the snapshot service at
/// all — no timer, no snapshot build. This is the whole point of the idle
/// gate: an unattended gateway does no dashboard work.
/// </summary>
/// <returns>A task that represents the asynchronous operation.</returns>
[Fact]
public async Task WatchAsync_WithNoSubscribers_NeverEnumeratesUnderlyingWatch()
{
FakeSnapshotService service = new();
DashboardSnapshotFeed feed = new(service);
// Obtaining the enumerable without enumerating it must not subscribe
// either: the pump starts on the first MoveNextAsync, not before.
_ = feed.WatchAsync(CancellationToken.None);
await Task.Delay(TimeSpan.FromMilliseconds(100));
Assert.Equal(0, service.EnumerationCount);
}
/// <summary>
/// Two viewers share one underlying enumeration and both see the same
/// pushed snapshot. Before the feed, each page opened its own SignalR
/// connection and the publisher pulled its own snapshot stream.
/// </summary>
/// <returns>A task that represents the asynchronous operation.</returns>
[Fact]
public async Task WatchAsync_WithTwoSubscribers_SharesASingleUnderlyingEnumeration()
{
FakeSnapshotService service = new();
DashboardSnapshotFeed feed = new(service);
using CancellationTokenSource firstCancellation = new();
using CancellationTokenSource secondCancellation = new();
IAsyncEnumerator<DashboardSnapshot> first =
feed.WatchAsync(firstCancellation.Token).GetAsyncEnumerator(firstCancellation.Token);
Task<bool> firstMove = first.MoveNextAsync().AsTask();
await WaitUntilAsync(() => service.EnumerationCount >= 1);
IAsyncEnumerator<DashboardSnapshot> second =
feed.WatchAsync(secondCancellation.Token).GetAsyncEnumerator(secondCancellation.Token);
Task<bool> secondMove = second.MoveNextAsync().AsTask();
// Push until both have observed a snapshot: a subscriber only becomes
// visible to the pump once its MoveNextAsync has registered the channel,
// so a single push could race the second registration.
await PushUntilAsync(service, Task.WhenAll(firstMove, secondMove));
Assert.True(await firstMove.WaitAsync(TestTimeout));
Assert.True(await secondMove.WaitAsync(TestTimeout));
Assert.StartsWith("push-", first.Current.GatewayVersion, StringComparison.Ordinal);
Assert.StartsWith("push-", second.Current.GatewayVersion, StringComparison.Ordinal);
Assert.Equal(1, service.EnumerationCount);
await firstCancellation.CancelAsync();
await secondCancellation.CancelAsync();
await DrainAsync(first, firstMove);
await DrainAsync(second, secondMove);
}
/// <summary>
/// The last viewer leaving must cancel the underlying enumeration (idle
/// gate re-armed), and the next viewer must restart it — the rapid
/// unsubscribe/resubscribe path a page navigation exercises.
/// </summary>
/// <returns>A task that represents the asynchronous operation.</returns>
[Fact]
public async Task WatchAsync_WhenLastSubscriberLeaves_CancelsPumpAndRestartsForTheNext()
{
FakeSnapshotService service = new();
DashboardSnapshotFeed feed = new(service);
using CancellationTokenSource firstCancellation = new();
IAsyncEnumerator<DashboardSnapshot> first =
feed.WatchAsync(firstCancellation.Token).GetAsyncEnumerator(firstCancellation.Token);
Task<bool> firstMove = first.MoveNextAsync().AsTask();
await WaitUntilAsync(() => service.EnumerationCount >= 1);
await firstCancellation.CancelAsync();
await DrainAsync(first, firstMove);
await WaitUntilAsync(() => service.CompletedEnumerationCount >= 1);
Assert.True(service.LastEnumerationWasCancelled);
using CancellationTokenSource secondCancellation = new();
IAsyncEnumerator<DashboardSnapshot> second =
feed.WatchAsync(secondCancellation.Token).GetAsyncEnumerator(secondCancellation.Token);
Task<bool> secondMove = second.MoveNextAsync().AsTask();
await WaitUntilAsync(() => service.EnumerationCount >= 2);
Assert.Equal(2, service.EnumerationCount);
await secondCancellation.CancelAsync();
await DrainAsync(second, secondMove);
}
/// <summary>
/// A viewer that is not reading must not stall the pump or accumulate
/// snapshots: its bounded channel drops the oldest, so its next read is the
/// newest snapshot the pump has broadcast, not a backlog head. The fast
/// reader's progress is what makes the assertion deterministic — once it has
/// seen the third snapshot the pump has provably broadcast all three.
/// </summary>
/// <returns>A task that represents the asynchronous operation.</returns>
[Fact]
public async Task WatchAsync_WithASlowSubscriber_KeepsOnlyTheNewestSnapshot()
{
FakeSnapshotService service = new();
DashboardSnapshotFeed feed = new(service);
using CancellationTokenSource fastCancellation = new();
using CancellationTokenSource slowCancellation = new();
IAsyncEnumerator<DashboardSnapshot> fast =
feed.WatchAsync(fastCancellation.Token).GetAsyncEnumerator(fastCancellation.Token);
Task<bool> fastMove = fast.MoveNextAsync().AsTask();
await WaitUntilAsync(() => service.EnumerationCount >= 1);
// The slow subscriber registers but never advances until the very end.
IAsyncEnumerator<DashboardSnapshot> slow =
feed.WatchAsync(slowCancellation.Token).GetAsyncEnumerator(slowCancellation.Token);
Task<bool> slowMove = slow.MoveNextAsync().AsTask();
await PushUntilAsync(service, Task.WhenAll(fastMove, slowMove));
Assert.True(await fastMove.WaitAsync(TestTimeout));
Assert.True(await slowMove.WaitAsync(TestTimeout));
service.Push(CreateSnapshot("s1"));
service.Push(CreateSnapshot("s2"));
service.Push(CreateSnapshot("s3"));
// Drain the fast reader until it sees s3; that proves the pump broadcast
// all three to every subscriber, so the slow channel now holds exactly s3.
string fastLatest = fast.Current.GatewayVersion;
while (fastLatest != "s3")
{
Assert.True(await fast.MoveNextAsync().AsTask().WaitAsync(TestTimeout));
fastLatest = fast.Current.GatewayVersion;
}
Assert.True(await slow.MoveNextAsync().AsTask().WaitAsync(TestTimeout));
Assert.Equal("s3", slow.Current.GatewayVersion);
await fastCancellation.CancelAsync();
await slowCancellation.CancelAsync();
await DrainAsync(fast, Task.FromResult(true));
await DrainAsync(slow, Task.FromResult(true));
}
/// <summary>
/// A fault in the underlying watch is surfaced to the current viewers rather
/// than silently hanging them, and it resets the feed so the next viewer
/// starts a fresh pump instead of attaching to a dead one.
/// </summary>
/// <returns>A task that represents the asynchronous operation.</returns>
[Fact]
public async Task WatchAsync_WhenUnderlyingWatchFaults_PropagatesAndRestartsForTheNextSubscriber()
{
FakeSnapshotService service = new();
DashboardSnapshotFeed feed = new(service);
using CancellationTokenSource firstCancellation = new();
IAsyncEnumerator<DashboardSnapshot> first =
feed.WatchAsync(firstCancellation.Token).GetAsyncEnumerator(firstCancellation.Token);
Task<bool> firstMove = first.MoveNextAsync().AsTask();
await WaitUntilAsync(() => service.EnumerationCount >= 1);
service.Fault(new InvalidOperationException("simulated snapshot source failure"));
InvalidOperationException failure =
await Assert.ThrowsAsync<InvalidOperationException>(() => firstMove.WaitAsync(TestTimeout));
Assert.Equal("simulated snapshot source failure", failure.Message);
await first.DisposeAsync();
using CancellationTokenSource secondCancellation = new();
IAsyncEnumerator<DashboardSnapshot> second =
feed.WatchAsync(secondCancellation.Token).GetAsyncEnumerator(secondCancellation.Token);
Task<bool> secondMove = second.MoveNextAsync().AsTask();
await WaitUntilAsync(() => service.EnumerationCount >= 2);
await PushUntilAsync(service, secondMove);
Assert.True(await secondMove.WaitAsync(TestTimeout));
await secondCancellation.CancelAsync();
await DrainAsync(second, secondMove);
}
/// <summary>
/// The race the generation tagging exists for: a page subscribes in the window between
/// the source failing and the dying pump detaching its subscribers. Without generations
/// the newcomer joined the doomed pump, was detached with its error, and no pump ever
/// restarted (only a first subscriber started one) — that page froze for good.
/// </summary>
/// <returns>A task that represents the asynchronous operation.</returns>
[Fact]
public async Task WatchAsync_WhenASubscriberJoinsWhileAFaultedPumpUnwinds_IsServedByAFreshPump()
{
FakeSnapshotService service = new();
service.HoldDisposal();
DashboardSnapshotFeed feed = new(service);
using CancellationTokenSource firstCancellation = new();
IAsyncEnumerator<DashboardSnapshot> first =
feed.WatchAsync(firstCancellation.Token).GetAsyncEnumerator(firstCancellation.Token);
Task<bool> firstMove = first.MoveNextAsync().AsTask();
await WaitUntilAsync(() => service.EnumerationCount >= 1);
service.Fault(new InvalidOperationException("simulated snapshot source failure"));
// The pump has observed the failure and is parked disposing the enumerator — the
// exact window in which a page used to attach itself to a doomed pump.
await service.DisposalReached.WaitAsync(TestTimeout);
using CancellationTokenSource secondCancellation = new();
IAsyncEnumerator<DashboardSnapshot> second =
feed.WatchAsync(secondCancellation.Token).GetAsyncEnumerator(secondCancellation.Token);
// An async iterator body runs synchronously up to its first await, so the
// subscription is registered by the time MoveNextAsync hands back its task.
Task<bool> secondMove = second.MoveNextAsync().AsTask();
service.ReleaseDisposal();
// The subscriber that was there when the source broke still learns about it...
InvalidOperationException failure =
await Assert.ThrowsAsync<InvalidOperationException>(() => firstMove.WaitAsync(TestTimeout));
Assert.Equal("simulated snapshot source failure", failure.Message);
await first.DisposeAsync();
// ...and the one that joined mid-unwind is served by a restarted enumeration
// instead of inheriting the failure.
await WaitUntilAsync(() => service.EnumerationCount >= 2);
await PushUntilAsync(service, secondMove);
Assert.True(await secondMove.WaitAsync(TestTimeout));
Assert.StartsWith("push-", second.Current.GatewayVersion, StringComparison.Ordinal);
await secondCancellation.CancelAsync();
await DrainAsync(second, secondMove);
}
/// <summary>
/// The idle gate is per generation, not per subscriber count. A viewer that joins while a
/// pump unwinds starts a new generation, and the old generation's viewers linger in the
/// list until that pump's reset runs — so a global "is the list empty" check let the new
/// viewer leave without cancelling the generation it had just started, leaving a pump
/// enumerating the snapshot source with nobody watching it.
/// </summary>
/// <returns>A task that represents the asynchronous operation.</returns>
[Fact]
public async Task WatchAsync_WhenAJoinerLeavesWhileAnOldGenerationLingers_LeavesNoPumpRunning()
{
FakeSnapshotService service = new();
service.HoldDisposal();
DashboardSnapshotFeed feed = new(service);
using CancellationTokenSource oldCancellation = new();
IAsyncEnumerator<DashboardSnapshot> oldSubscriber =
feed.WatchAsync(oldCancellation.Token).GetAsyncEnumerator(oldCancellation.Token);
Task<bool> oldMove = oldSubscriber.MoveNextAsync().AsTask();
await WaitUntilAsync(() => service.EnumerationCount >= 1);
service.Fault(new InvalidOperationException("simulated snapshot source failure"));
await service.DisposalReached.WaitAsync(TestTimeout);
// Joins mid-unwind (starting a fresh generation) and leaves again before the dying
// pump has detached the subscriber that is still lingering in the list.
using CancellationTokenSource joinerCancellation = new();
IAsyncEnumerator<DashboardSnapshot> joiner =
feed.WatchAsync(joinerCancellation.Token).GetAsyncEnumerator(joinerCancellation.Token);
Task<bool> joinerMove = joiner.MoveNextAsync().AsTask();
await joinerCancellation.CancelAsync();
// The joiner's unwind is a continuation of its cancelled channel read; give it time to
// run its unsubscribe before the dying pump is released. Only the interleaving depends
// on this delay — the assertions below hold either way.
await Task.Delay(TimeSpan.FromMilliseconds(150));
service.ReleaseDisposal();
await Assert.ThrowsAsync<InvalidOperationException>(() => oldMove.WaitAsync(TestTimeout));
await oldSubscriber.DisposeAsync();
// Completes only once the joiner's unsubscribe has awaited its generation's pump.
await DrainAsync(joiner, joinerMove);
// Nobody is watching, so nothing may consume a snapshot: a surviving pump would drain
// this push within its first read.
service.Push(CreateSnapshot("orphan-check"));
await Task.Delay(TimeSpan.FromMilliseconds(150));
Assert.Equal(1, service.PendingPushCount);
// ...and the next viewer still starts cleanly, picking up the queued snapshot.
int enumerationsBefore = service.EnumerationCount;
using CancellationTokenSource nextCancellation = new();
IAsyncEnumerator<DashboardSnapshot> next =
feed.WatchAsync(nextCancellation.Token).GetAsyncEnumerator(nextCancellation.Token);
Task<bool> nextMove = next.MoveNextAsync().AsTask();
await WaitUntilAsync(() => service.EnumerationCount > enumerationsBefore);
Assert.True(await nextMove.WaitAsync(TestTimeout));
Assert.Equal("orphan-check", next.Current.GatewayVersion);
await nextCancellation.CancelAsync();
await DrainAsync(next, nextMove);
}
/// <summary>Builds a snapshot whose version string identifies it in assertions.</summary>
/// <param name="version">Identity marker carried in <c>GatewayVersion</c>.</param>
/// <returns>A snapshot carrying the supplied identity marker.</returns>
private static DashboardSnapshot CreateSnapshot(string version)
{
return new DashboardSnapshot(
GeneratedAt: DateTimeOffset.UnixEpoch,
GatewayStartedAt: DateTimeOffset.UnixEpoch,
GatewayUptime: TimeSpan.Zero,
GatewayStatus: "Healthy",
GatewayVersion: version,
Sessions: Array.Empty<DashboardSessionSummary>(),
Workers: Array.Empty<DashboardWorkerSummary>(),
Metrics: Array.Empty<DashboardMetricSummary>(),
Faults: Array.Empty<DashboardFaultSummary>(),
ApiKeys: Array.Empty<DashboardApiKeySummary>(),
Configuration: null!,
Galaxy: null!);
}
/// <summary>
/// Pushes snapshots until the supplied task completes, so a test never
/// depends on a single push landing after a subscriber has registered.
/// </summary>
/// <param name="service">Fake snapshot source to push through.</param>
/// <param name="until">Task whose completion stops the pushes.</param>
/// <returns>A task that represents the asynchronous operation.</returns>
private static async Task PushUntilAsync(FakeSnapshotService service, Task until)
{
using CancellationTokenSource cancellation = new(TestTimeout);
int sequence = 0;
while (!until.IsCompleted)
{
service.Push(CreateSnapshot($"push-{sequence++}"));
await Task.Delay(TimeSpan.FromMilliseconds(5), cancellation.Token);
}
}
/// <summary>
/// Observes the cancellation of a pending enumeration and disposes the
/// enumerator, mirroring how <c>await foreach</c> unwinds a cancelled watch.
/// </summary>
/// <param name="enumerator">Enumerator to unwind.</param>
/// <param name="pending">The in-flight move, if any.</param>
/// <returns>A task that represents the asynchronous operation.</returns>
private static async Task DrainAsync(IAsyncEnumerator<DashboardSnapshot> enumerator, Task pending)
{
try
{
await pending.WaitAsync(TestTimeout);
}
catch (OperationCanceledException)
{
}
try
{
await enumerator.DisposeAsync();
}
catch (OperationCanceledException)
{
}
}
private static async Task WaitUntilAsync(Func<bool> predicate)
{
using CancellationTokenSource cancellation = new(TestTimeout);
while (!predicate())
{
await Task.Delay(TimeSpan.FromMilliseconds(5), cancellation.Token);
}
}
/// <summary>
/// Snapshot source under the feed's control: counts enumerations, records how
/// each one ended, and lets the test drive snapshots (or a fault) into the
/// live enumeration.
/// </summary>
private sealed class FakeSnapshotService : IDashboardSnapshotService
{
private readonly Channel<object> _pushes = Channel.CreateUnbounded<object>();
private readonly TaskCompletionSource _disposalReached = new(TaskCreationOptions.RunContinuationsAsynchronously);
private readonly TaskCompletionSource _disposalRelease = new(TaskCreationOptions.RunContinuationsAsynchronously);
private int _enumerationCount;
private int _completedEnumerationCount;
private volatile bool _lastEnumerationWasCancelled;
private volatile bool _holdDisposal;
/// <summary>Gets the number of times the feed started enumerating this source.</summary>
public int EnumerationCount => Volatile.Read(ref _enumerationCount);
/// <summary>Gets a task that completes when a held enumerator disposal is reached.</summary>
public Task DisposalReached => _disposalReached.Task;
/// <summary>
/// Gets the number of queued snapshots no enumeration has taken yet. A live pump
/// drains this even with nobody watching, so a stable count proves the feed is idle.
/// </summary>
public int PendingPushCount => _pushes.Reader.Count;
/// <summary>Gets the number of enumerations that have finished (cancelled, faulted, or completed).</summary>
public int CompletedEnumerationCount => Volatile.Read(ref _completedEnumerationCount);
/// <summary>Gets a value indicating whether the most recently finished enumeration ended cancelled.</summary>
public bool LastEnumerationWasCancelled => _lastEnumerationWasCancelled;
/// <summary>Queues a snapshot for the live enumeration to yield.</summary>
/// <param name="snapshot">Snapshot to yield.</param>
public void Push(DashboardSnapshot snapshot) => _pushes.Writer.TryWrite(snapshot);
/// <summary>Queues a failure for the live enumeration to throw.</summary>
/// <param name="error">Exception to throw from the enumeration.</param>
public void Fault(Exception error) => _pushes.Writer.TryWrite(error);
/// <summary>
/// Parks enumerator disposal until <see cref="ReleaseDisposal"/>, which holds the pump
/// in the window between observing the source's failure and detaching its subscribers.
/// </summary>
public void HoldDisposal() => _holdDisposal = true;
/// <summary>Releases a held enumerator disposal.</summary>
public void ReleaseDisposal() => _disposalRelease.TrySetResult();
/// <inheritdoc />
public DashboardSnapshot GetSnapshot() => CreateSnapshot("current");
/// <inheritdoc />
public IAsyncEnumerable<DashboardSnapshot> WatchSnapshotsAsync(CancellationToken cancellationToken)
=> new GatedEnumerable(this);
private async Task OnDisposingAsync()
{
if (!_holdDisposal)
{
return;
}
_disposalReached.TrySetResult();
await _disposalRelease.Task.ConfigureAwait(false);
}
private async IAsyncEnumerable<DashboardSnapshot> EnumerateAsync(
[EnumeratorCancellation] CancellationToken cancellationToken)
{
Interlocked.Increment(ref _enumerationCount);
try
{
await foreach (object item in _pushes.Reader.ReadAllAsync(cancellationToken).ConfigureAwait(false))
{
if (item is Exception error)
{
throw error;
}
yield return (DashboardSnapshot)item;
}
}
finally
{
_lastEnumerationWasCancelled = cancellationToken.IsCancellationRequested;
Interlocked.Increment(ref _completedEnumerationCount);
}
}
/// <summary>
/// Wraps the iterator so disposal is a control point of its own: the feed ends a pump
/// generation when MoveNextAsync fails, which is strictly before this disposal runs.
/// </summary>
/// <param name="owner">The fake whose enumeration is being wrapped.</param>
private sealed class GatedEnumerable(FakeSnapshotService owner) : IAsyncEnumerable<DashboardSnapshot>
{
/// <summary>Creates a gated enumerator over the fake's enumeration.</summary>
/// <param name="cancellationToken">Token to observe for cancellation.</param>
/// <returns>The gated enumerator.</returns>
public IAsyncEnumerator<DashboardSnapshot> GetAsyncEnumerator(
CancellationToken cancellationToken = default)
=> new GatedEnumerator(owner, owner.EnumerateAsync(cancellationToken).GetAsyncEnumerator(cancellationToken));
}
private sealed class GatedEnumerator(
FakeSnapshotService owner,
IAsyncEnumerator<DashboardSnapshot> inner) : IAsyncEnumerator<DashboardSnapshot>
{
/// <summary>Gets the current snapshot.</summary>
public DashboardSnapshot Current => inner.Current;
/// <summary>Advances the wrapped enumeration.</summary>
/// <returns>A task that yields whether another snapshot is available.</returns>
public ValueTask<bool> MoveNextAsync() => inner.MoveNextAsync();
/// <summary>Parks while the fake holds disposal, then disposes the wrapped enumeration.</summary>
/// <returns>A task that represents the asynchronous operation.</returns>
public async ValueTask DisposeAsync()
{
await owner.OnDisposingAsync().ConfigureAwait(false);
await inner.DisposeAsync().ConfigureAwait(false);
}
}
}
}
@@ -377,6 +377,14 @@ public sealed class WorkerFrameProtocolTests
/// Verifies the writer coalesces the flush across a batch of frames drained together: four frames /// Verifies the writer coalesces the flush across a batch of frames drained together: four frames
/// queued behind an in-progress write drain in a single pass and share one FlushAsync, not four. /// queued behind an in-progress write drain in a single pass and share one FlushAsync, not four.
/// Every frame still reaches the wire intact. /// Every frame still reaches the wire intact.
/// <para>
/// The burst is all-event on purpose. The control-frame completion decoupling made the drain flush
/// at each control-to-event boundary, so a pass that mixes classes legitimately pays one flush per
/// class run; the property
/// worth pinning is that a run of same-class frames — the event hot path — still costs exactly one
/// flush no matter how many frames drain together. The mixed shape has its own count assertion in
/// <see cref="DrainPass_MixedClasses_FlushesControlRunBeforeWritingEvents"/>.
/// </para>
/// </summary> /// </summary>
/// <returns>A task that represents the asynchronous operation.</returns> /// <returns>A task that represents the asynchronous operation.</returns>
[Fact] [Fact]
@@ -387,7 +395,7 @@ public sealed class WorkerFrameProtocolTests
WorkerFrameWriter writer = new(stream, options); WorkerFrameWriter writer = new(stream, options);
// A blocked first write occupies the writer and holds the lock while more frames queue behind it. // A blocked first write occupies the writer and holds the lock while more frames queue behind it.
Task firstWrite = writer.WriteAsync(CreateGatewayHelloEnvelope(), WorkerFrameWritePriority.Control); Task firstWrite = writer.WriteAsync(CreateEventEnvelope(), WorkerFrameWritePriority.Event);
await AwaitWithTimeoutAsync(stream.FirstWriteStarted); await AwaitWithTimeoutAsync(stream.FirstWriteStarted);
Task eventWrite1 = writer.WriteAsync(CreateEventEnvelope(), WorkerFrameWritePriority.Event); Task eventWrite1 = writer.WriteAsync(CreateEventEnvelope(), WorkerFrameWritePriority.Event);
@@ -410,6 +418,179 @@ public sealed class WorkerFrameProtocolTests
} }
} }
/// <summary>
/// Control-frame completion decoupling. A control frame's delivery point must not be charged for
/// the event backlog behind it. The priority scheduler already wrote control <em>bytes</em> first,
/// but a frame counts as
/// delivered only once flushed, and the pass deferred its single flush — and every completion —
/// until after the events. The drain now flushes at the control-to-event boundary: with two control
/// frames written and the first event write blocked inside the stream, the flush that closes out the
/// control run has already run, so the heartbeat or reply is on the pipe rather than waiting behind
/// the batch. Exactly two flushes for the pass — one per class run, not one per control frame.
/// <para>
/// The assertion is on the flush, not on the queued callers' returned tasks, because those tasks are
/// still gated by the write lock they lost to the drainer (see the latency contract on
/// <c>WorkerFrameWriter.WriteAsync</c>): the completion resolves at the boundary flush, but a
/// lock-race loser observes it only once the drainer releases the lock.
/// </para>
/// </summary>
/// <returns>A task that represents the asynchronous operation.</returns>
[Fact]
public async Task DrainPass_MixedClasses_FlushesControlRunBeforeWritingEvents()
{
WorkerFrameProtocolOptions options = CreateOptions();
// Frame 1 (control) gates the pass open; frame 3 is the pass's first event write, which blocks
// so the boundary flush can be observed with the event batch still unwritten.
using GatedWriteStream stream = new(secondGateWriteIndex: 3);
WorkerFrameWriter writer = new(stream, options);
Task firstControl = writer.WriteAsync(CreateGatewayHelloEnvelope(), WorkerFrameWritePriority.Control);
await AwaitWithTimeoutAsync(stream.FirstWriteStarted);
Task secondControl = writer.WriteAsync(CreateShutdownAckEnvelope(), WorkerFrameWritePriority.Control);
Task eventWrite1 = writer.WriteAsync(CreateEventEnvelope(), WorkerFrameWritePriority.Event);
Task eventWrite2 = writer.WriteAsync(CreateEventEnvelope(), WorkerFrameWritePriority.Event);
await Task.Delay(50);
stream.ReleaseFirstWrite();
// The drain writes both control frames and is now blocked on the first event write.
await AwaitWithTimeoutAsync(stream.SecondGateWriteStarted);
// The control run was flushed before the event batch was written — not after it.
Assert.Equal(1, stream.FlushCount);
Assert.False(eventWrite1.IsCompleted);
Assert.False(eventWrite2.IsCompleted);
stream.ReleaseSecondGateWrite();
await AwaitWithTimeoutAsync(
Task.WhenAll(firstControl, secondControl, eventWrite1, eventWrite2));
// One flush per class run: the control run, then the event run at the end of the pass.
Assert.Equal(2, stream.FlushCount);
stream.Position = 0;
WorkerFrameReader reader = new(stream, options);
WorkerEnvelope frame1 = await reader.ReadAsync();
WorkerEnvelope frame2 = await reader.ReadAsync();
WorkerEnvelope frame3 = await reader.ReadAsync();
WorkerEnvelope frame4 = await reader.ReadAsync();
Assert.Equal(WorkerEnvelope.BodyOneofCase.GatewayHello, frame1.BodyCase);
Assert.Equal(WorkerEnvelope.BodyOneofCase.WorkerShutdownAck, frame2.BodyCase);
Assert.Equal(WorkerEnvelope.BodyOneofCase.WorkerEvent, frame3.BodyCase);
Assert.Equal(WorkerEnvelope.BodyOneofCase.WorkerEvent, frame4.BodyCase);
Assert.Equal(stream.Length, stream.Position);
}
/// <summary>
/// Control-frame completion decoupling, the inverse guard. An event frame's completion boundary is
/// still the end-of-pass flush: a pure-event pass takes no boundary flush, so with two events
/// already written and the third
/// blocked mid-write, nothing has been flushed and no event can have been reported delivered. Only
/// a class transition may move a flush earlier — a plain event backlog may not.
/// </summary>
/// <returns>A task that represents the asynchronous operation.</returns>
[Fact]
public async Task DrainPass_PureEventRun_DoesNotFlushBeforeThePassEnds()
{
WorkerFrameProtocolOptions options = CreateOptions();
using GatedWriteStream stream = new(secondGateWriteIndex: 3);
WorkerFrameWriter writer = new(stream, options);
Task eventWrite1 = writer.WriteAsync(CreateEventEnvelope(), WorkerFrameWritePriority.Event);
await AwaitWithTimeoutAsync(stream.FirstWriteStarted);
Task eventWrite2 = writer.WriteAsync(CreateEventEnvelope(), WorkerFrameWritePriority.Event);
Task eventWrite3 = writer.WriteAsync(CreateEventEnvelope(), WorkerFrameWritePriority.Event);
await Task.Delay(50);
stream.ReleaseFirstWrite();
await AwaitWithTimeoutAsync(stream.SecondGateWriteStarted);
// Two event frames written, none flushed: no event frame's delivery point has been reached.
Assert.Equal(0, stream.FlushCount);
Assert.False(eventWrite1.IsCompleted);
Assert.False(eventWrite2.IsCompleted);
stream.ReleaseSecondGateWrite();
await AwaitWithTimeoutAsync(Task.WhenAll(eventWrite1, eventWrite2, eventWrite3));
Assert.Equal(1, stream.FlushCount);
}
/// <summary>
/// Control-frame completion decoupling. The boundary flush is charged per class run, not per
/// control frame: a pass carrying nothing but control frames still pays exactly one flush. Flushing
/// after every control frame
/// would reinstate the syscall-per-heartbeat cost WRK-12 removed.
/// </summary>
/// <returns>A task that represents the asynchronous operation.</returns>
[Fact]
public async Task DrainPass_PureControlRun_FlushesOnce()
{
WorkerFrameProtocolOptions options = CreateOptions();
using GatedWriteStream stream = new();
WorkerFrameWriter writer = new(stream, options);
Task firstControl = writer.WriteAsync(CreateGatewayHelloEnvelope(), WorkerFrameWritePriority.Control);
await AwaitWithTimeoutAsync(stream.FirstWriteStarted);
Task secondControl = writer.WriteAsync(CreateGatewayHelloEnvelope(), WorkerFrameWritePriority.Control);
Task thirdControl = writer.WriteAsync(CreateShutdownAckEnvelope(), WorkerFrameWritePriority.Control);
await Task.Delay(50);
stream.ReleaseFirstWrite();
await AwaitWithTimeoutAsync(Task.WhenAll(firstControl, secondControl, thirdControl));
Assert.Equal(1, stream.FlushCount);
}
/// <summary>
/// Control-frame completion decoupling, the new failure window. The boundary flush is a new place
/// the pipe can break with frames written but not yet delivered, so it must fail exactly like the
/// end-of-pass flush: every written control
/// frame fails, and so do the event frame the drain had already claimed off its queue (nothing else
/// would ever complete it) and every frame still queued, so no caller waits forever on a stream that
/// will not recover. The event bytes never reach the wire — the drain stops at the fault.
/// </summary>
/// <returns>A task that represents the asynchronous operation.</returns>
[Fact]
public async Task DrainPass_WhenBoundaryFlushFails_FailsWrittenClaimedAndQueuedFrames()
{
const string faultMessage = "boundary flush failed";
WorkerFrameProtocolOptions options = CreateOptions();
using FlushFaultingGatedStream stream = new(faultMessage);
WorkerFrameWriter writer = new(stream, options);
Task firstControl = writer.WriteAsync(CreateGatewayHelloEnvelope(), WorkerFrameWritePriority.Control);
await AwaitWithTimeoutAsync(stream.FirstWriteStarted);
Task secondControl = writer.WriteAsync(CreateShutdownAckEnvelope(), WorkerFrameWritePriority.Control);
Task claimedEvent = writer.WriteAsync(CreateEventEnvelope(), WorkerFrameWritePriority.Event);
Task queuedEvent = writer.WriteAsync(CreateEventEnvelope(), WorkerFrameWritePriority.Event);
await Task.Delay(50);
// The drain writes both control frames, claims the first event, and faults on the boundary flush.
stream.ReleaseFirstWrite();
// AwaitWithTimeoutAsync turns a frame nobody ever completes into a TimeoutException — a failed
// assertion rather than a hung test run.
foreach (Task write in new[] { firstControl, secondControl, claimedEvent, queuedEvent })
{
IOException failure = await Assert.ThrowsAsync<IOException>(
async () => await AwaitWithTimeoutAsync(write));
Assert.Equal(faultMessage, failure.Message);
}
// Only the control frames reached the wire; the claimed event was never written.
stream.Position = 0;
WorkerFrameReader reader = new(stream, options);
WorkerEnvelope frame1 = await reader.ReadAsync();
WorkerEnvelope frame2 = await reader.ReadAsync();
Assert.Equal(WorkerEnvelope.BodyOneofCase.GatewayHello, frame1.BodyCase);
Assert.Equal(WorkerEnvelope.BodyOneofCase.WorkerShutdownAck, frame2.BodyCase);
Assert.Equal(stream.Length, stream.Position);
}
/// <summary> /// <summary>
/// Verifies a per-frame rejection does not burn a sequence number (WRK-23). The sequence is a /// Verifies a per-frame rejection does not burn a sequence number (WRK-23). The sequence is a
/// diagnostic counter, so a gap breaks nothing functionally — but an operator correlating a pipe /// diagnostic counter, so a gap breaks nothing functionally — but an operator correlating a pipe
@@ -877,24 +1058,108 @@ public sealed class WorkerFrameProtocolTests
} }
// A MemoryStream whose first WriteAsync blocks until released, so a test can queue additional frames // A MemoryStream whose first WriteAsync blocks until released, so a test can queue additional frames
// behind an in-progress write and observe the writer's priority ordering. // behind an in-progress write and observe the writer's priority ordering. A second, optional gate on
// a chosen write index lets a test stop a drain pass mid-flight — at a class boundary, say — and
// sample what the writer has already flushed while the rest of the pass is still unwritten.
private sealed class GatedWriteStream : MemoryStream private sealed class GatedWriteStream : MemoryStream
{ {
private readonly SemaphoreSlim _release = new SemaphoreSlim(0); private readonly SemaphoreSlim _release = new SemaphoreSlim(0);
private readonly SemaphoreSlim _secondGateRelease = new SemaphoreSlim(0);
private readonly TaskCompletionSource<bool> _firstWriteStarted = private readonly TaskCompletionSource<bool> _firstWriteStarted =
new TaskCompletionSource<bool>(TaskCreationOptions.RunContinuationsAsynchronously); new TaskCompletionSource<bool>(TaskCreationOptions.RunContinuationsAsynchronously);
private readonly TaskCompletionSource<bool> _secondGateWriteStarted =
new TaskCompletionSource<bool>(TaskCreationOptions.RunContinuationsAsynchronously);
private readonly int _secondGateWriteIndex;
private int _writeCount; private int _writeCount;
private int _flushCount; private int _flushCount;
/// <summary>Initializes a new instance of the GatedWriteStream class.</summary>
/// <param name="secondGateWriteIndex">
/// One-based index of a later write to block as well, or 0 (the default) to gate only the first
/// write. Write indexes start at 1, so 0 never matches.
/// </param>
public GatedWriteStream(int secondGateWriteIndex = 0)
{
_secondGateWriteIndex = secondGateWriteIndex;
}
/// <summary>Gets a task that completes once the first <see cref="WriteAsync"/> call has started blocking.</summary> /// <summary>Gets a task that completes once the first <see cref="WriteAsync"/> call has started blocking.</summary>
public Task FirstWriteStarted => _firstWriteStarted.Task; public Task FirstWriteStarted => _firstWriteStarted.Task;
/// <summary>Gets a task that completes once the second gated <see cref="WriteAsync"/> call has started blocking.</summary>
public Task SecondGateWriteStarted => _secondGateWriteStarted.Task;
/// <summary>Gets the number of <see cref="FlushAsync"/> calls observed so far.</summary> /// <summary>Gets the number of <see cref="FlushAsync"/> calls observed so far.</summary>
public int FlushCount => Volatile.Read(ref _flushCount); public int FlushCount => Volatile.Read(ref _flushCount);
/// <summary>Releases the first blocked write so it can complete.</summary> /// <summary>Releases the first blocked write so it can complete.</summary>
public void ReleaseFirstWrite() => _release.Release(); public void ReleaseFirstWrite() => _release.Release();
/// <summary>Releases the second gated write so it can complete.</summary>
public void ReleaseSecondGateWrite() => _secondGateRelease.Release();
/// <inheritdoc />
public override async Task WriteAsync(byte[] buffer, int offset, int count, CancellationToken cancellationToken)
{
int writeIndex = Interlocked.Increment(ref _writeCount);
if (writeIndex == 1)
{
_firstWriteStarted.TrySetResult(true);
await _release.WaitAsync(cancellationToken);
}
else if (writeIndex == _secondGateWriteIndex)
{
_secondGateWriteStarted.TrySetResult(true);
await _secondGateRelease.WaitAsync(cancellationToken);
}
await base.WriteAsync(buffer, offset, count, cancellationToken);
}
/// <inheritdoc />
public override Task FlushAsync(CancellationToken cancellationToken)
{
Interlocked.Increment(ref _flushCount);
return base.FlushAsync(cancellationToken);
}
/// <inheritdoc />
protected override void Dispose(bool disposing)
{
if (disposing)
{
_release.Dispose();
_secondGateRelease.Dispose();
}
base.Dispose(disposing);
}
}
// A MemoryStream whose first write blocks until released and whose every FlushAsync throws, so a test
// can fault the class-boundary flush with control frames already written and an event frame already
// claimed off its queue.
private sealed class FlushFaultingGatedStream : MemoryStream
{
private readonly SemaphoreSlim _release = new SemaphoreSlim(0);
private readonly TaskCompletionSource<bool> _firstWriteStarted =
new TaskCompletionSource<bool>(TaskCreationOptions.RunContinuationsAsynchronously);
private readonly string _faultMessage;
private int _writeCount;
/// <summary>Initializes a new instance of the FlushFaultingGatedStream class.</summary>
/// <param name="faultMessage">Message carried by the <see cref="IOException"/> every flush throws.</param>
public FlushFaultingGatedStream(string faultMessage)
{
_faultMessage = faultMessage;
}
/// <summary>Gets a task that completes once the first <see cref="WriteAsync"/> call has started blocking.</summary>
public Task FirstWriteStarted => _firstWriteStarted.Task;
/// <summary>Releases the first blocked write so it can complete.</summary>
public void ReleaseFirstWrite() => _release.Release();
/// <inheritdoc /> /// <inheritdoc />
public override async Task WriteAsync(byte[] buffer, int offset, int count, CancellationToken cancellationToken) public override async Task WriteAsync(byte[] buffer, int offset, int count, CancellationToken cancellationToken)
{ {
@@ -910,8 +1175,7 @@ public sealed class WorkerFrameProtocolTests
/// <inheritdoc /> /// <inheritdoc />
public override Task FlushAsync(CancellationToken cancellationToken) public override Task FlushAsync(CancellationToken cancellationToken)
{ {
Interlocked.Increment(ref _flushCount); return Task.FromException(new IOException(_faultMessage));
return base.FlushAsync(cancellationToken);
} }
/// <inheritdoc /> /// <inheritdoc />
@@ -912,6 +912,86 @@ public sealed class WorkerPipeSessionTests
await Assert.ThrowsAsync<InvalidOperationException>(async () => await runTask); await Assert.ThrowsAsync<InvalidOperationException>(async () => await runTask);
} }
/// <summary>
/// WRK-31, the other side of the invariant. The graceful path leaves the message loop
/// through its <c>return</c> after the shutdown ack, with that iteration's read already
/// awaited — so there is no abandoned read for teardown to account for. Pinning this keeps
/// the new disposal-and-observe step a pure no-op on the path production takes every time a
/// session closes normally: no "PipeRead" observation, and therefore no chance of paying
/// the observation timeout on a healthy shutdown.
/// </summary>
/// <returns>A task that represents the asynchronous operation.</returns>
[Fact]
public async Task RunAsync_GracefulShutdown_LeavesNoPendingPipeReadToObserve()
{
using CancellationTokenSource cancellation = new(TimeSpan.FromSeconds(10));
using PipePair pipePair = await PipePair.CreateAsync(cancellation.Token);
FakeRuntimeSession runtime = new();
RecordingWorkerLogger logger = new();
WorkerPipeSession session = CreatePipeSession(
pipePair.WorkerStream,
runtime,
new WorkerPipeSessionOptions
{
HeartbeatInterval = TimeSpan.FromMilliseconds(100),
HeartbeatGrace = TimeSpan.FromSeconds(5),
},
logger);
Task runTask = session.RunAsync(cancellation.Token);
await CompleteGatewayHandshakeAsync(pipePair, cancellation.Token);
// Reads the ack and bounds RunAsync's completion at 5s.
await SendShutdownAndWaitAsync(pipePair, runTask, cancellation.Token);
Assert.True(runtime.Disposed, "Graceful shutdown must dispose the runtime session.");
Assert.DoesNotContain(
logger.Events,
entry => (entry.EventName == "WorkerPipeSessionBackgroundTaskStopFailed"
|| entry.EventName == "WorkerPipeSessionBackgroundTaskStopTimedOut")
&& entry.Fields.TryGetValue("task", out object? task)
&& (task as string) == "PipeRead");
}
/// <summary>
/// WRK-31. The session now owns and closes the transport rather than leaving it to
/// <c>WorkerPipeClient</c>'s <c>using</c>, because the read it unblocks has to be observed
/// while the session still holds it. This asserts the closure actually happens on the
/// session's own timeline: the gateway end of the pipe must see disconnection while
/// <see cref="PipePair"/> is still undisposed, so nothing but the worker side of the
/// session can have closed it.
/// </summary>
/// <returns>A task that represents the asynchronous operation.</returns>
[Fact]
public async Task RunAsync_WhenSessionEnds_ClosesTransportBeforeTheHarnessDoes()
{
using CancellationTokenSource cancellation = new(TimeSpan.FromSeconds(10));
using PipePair pipePair = await PipePair.CreateAsync(cancellation.Token);
FakeRuntimeSession runtime = new();
WorkerPipeSession session = CreatePipeSession(pipePair.WorkerStream, runtime);
Task runTask = session.RunAsync(cancellation.Token);
await CompleteGatewayHandshakeAsync(pipePair, cancellation.Token);
await SendShutdownAndWaitAsync(pipePair, runTask, cancellation.Token);
// PipePair.Dispose has not run — its `using` is still in scope — so the only thing that can
// have closed the worker end is the session. A gateway-side read is uncancellable on net48
// exactly as the worker's is, so a session that left the pipe open would park this read
// until the harness disposes; the bound below is what catches that.
Task<Exception> disconnectTask = ReadUntilDisconnectedAsync(pipePair.GatewayReader);
Task completedTask = await Task.WhenAny(
disconnectTask,
Task.Delay(TimeSpan.FromSeconds(5), cancellation.Token));
Assert.Same(disconnectTask, completedTask);
Exception disconnect = await disconnectTask;
Assert.True(
disconnect is IOException
|| disconnect is ObjectDisposedException
|| (disconnect is WorkerFrameProtocolException frameException
&& frameException.ErrorCode == WorkerFrameProtocolErrorCode.EndOfStream),
$"Expected the gateway read to observe a pipe disconnection, got {disconnect}.");
}
/// <summary> /// <summary>
/// Verifies that ShutdownWorker returns its OK reply BEFORE the graceful /// Verifies that ShutdownWorker returns its OK reply BEFORE the graceful
/// shutdown runs and disposes the runtime session, and that the message /// shutdown runs and disposes the runtime session, and that the message
@@ -1881,7 +1961,11 @@ public sealed class WorkerPipeSessionTests
() => 1234, () => 1234,
sessionOptions, sessionOptions,
() => runtime, () => runtime,
logger); logger,
// Hand the session the same ownership the production WorkerPipeClient path gives it, so
// these tests exercise the real teardown: the session closes the transport itself and
// then observes the read that closure unblocks.
transportStream: stream);
} }
private static WorkerFrameProtocolOptions CreateOptions() private static WorkerFrameProtocolOptions CreateOptions()
@@ -2149,6 +2233,32 @@ public sealed class WorkerPipeSessionTests
} }
} }
/// <summary>
/// Reads a pipe end until it stops producing frames, returning whatever ended it. Frames
/// still buffered from before the peer closed its handle — a trailing heartbeat, say — are
/// drained first, because Windows named pipes hand over buffered bytes ahead of the
/// broken-pipe signal.
/// </summary>
/// <param name="reader">Frame reader over the end being watched.</param>
/// <returns>The exception that ended the read.</returns>
private static async Task<Exception> ReadUntilDisconnectedAsync(WorkerFrameReader reader)
{
while (true)
{
try
{
await reader.ReadAsync(CancellationToken.None).ConfigureAwait(false);
}
catch (Exception exception) when (
exception is IOException
|| exception is ObjectDisposedException
|| exception is WorkerFrameProtocolException)
{
return exception;
}
}
}
private static WorkerEnvelope[] ReadWrittenFrames( private static WorkerEnvelope[] ReadWrittenFrames(
MemoryStream stream, MemoryStream stream,
WorkerFrameProtocolOptions options) WorkerFrameProtocolOptions options)
@@ -2165,6 +2275,123 @@ public sealed class WorkerPipeSessionTests
return envelopes.ToArray(); return envelopes.ToArray();
} }
/// <summary>
/// The one teardown test that has to arm <see cref="TaskScheduler.UnobservedTaskException"/>,
/// which is process-global: a task faulting in any concurrently running test class can be
/// finalized inside this test's window and read as its result. It therefore lives in its own
/// non-parallel collection (see <see cref="WorkerPipeSessionNonParallelCollection"/>) rather
/// than alongside its siblings. Nested so it can still reach
/// <see cref="WorkerPipeSessionTests"/>'s private harness — <c>PipePair</c>,
/// <c>CreatePipeSession</c>, <c>RecordingWorkerLogger</c> — without widening any of it.
/// </summary>
[Collection(WorkerPipeSessionNonParallelCollection.Name)]
public sealed class AbandonedPipeReadTeardownTests
{
/// <summary>
/// WRK-31. A fault exit unwinds the message loop while its frame read is still pending,
/// and on net48 nothing can cancel that read — <c>NamedPipeClientStream.ReadAsync</c>
/// ignores the token, so only closing the handle ends it. The session must therefore
/// dispose the transport itself and account for the read that disposal unblocks: the
/// worker installs no <c>TaskScheduler.UnobservedTaskException</c> handler, so before
/// this the read faulted on a task nobody held — still carrying the reader's reused
/// prefix buffer and its pooled payload buffer — and surfaced only at finalization.
/// </summary>
/// <returns>A task that represents the asynchronous operation.</returns>
[Fact]
public async Task RunAsync_WhenFaultEndsSession_ObservesTheAbandonedPipeRead()
{
const uint tinyMaxFrameBytes = 4096;
object unobservedGate = new();
List<Exception> unobservedPipeExceptions = new();
EventHandler<UnobservedTaskExceptionEventArgs> unobservedHandler = (_, args) =>
{
// Narrowed to a pipe stream's own teardown exception even though the collection is
// non-parallel, because the handler stays armed across this test's own async
// machinery. SetObserved is deliberately NOT called — the default policy already
// swallows these, and observing them here would mask a regression rather than
// report it.
foreach (Exception inner in args.Exception.Flatten().InnerExceptions)
{
if ((inner is ObjectDisposedException || inner is IOException)
&& inner.Message.IndexOf("pipe", StringComparison.OrdinalIgnoreCase) >= 0)
{
lock (unobservedGate)
{
unobservedPipeExceptions.Add(inner);
}
}
}
};
RecordingWorkerLogger logger = new();
TaskScheduler.UnobservedTaskException += unobservedHandler;
try
{
using CancellationTokenSource cancellation = new(TimeSpan.FromSeconds(15));
using PipePair pipePair = await PipePair.CreateAsync(cancellation.Token);
FakeRuntimeSession runtime = new();
WorkerPipeSession session = CreatePipeSession(
pipePair.WorkerStream,
runtime,
new WorkerPipeSessionOptions
{
HeartbeatInterval = TimeSpan.FromMilliseconds(100),
HeartbeatGrace = TimeSpan.FromSeconds(5),
},
logger);
runtime.EnqueueEvent(CreateOversizedWorkerEvent(sequence: 91, payloadBytes: 16 * 1024));
Task runTask = session.RunAsync(cancellation.Token);
await CompleteGatewayHandshakeAsync(pipePair, tinyMaxFrameBytes, cancellation.Token);
await ReadUntilAsync(
pipePair.GatewayReader,
WorkerEnvelope.BodyOneofCase.WorkerFault,
cancellation.Token);
// The same 5s bound the sibling oversized-event test uses: teardown must not stall
// on the read it abandoned.
Task completedTask = await Task.WhenAny(
runTask,
Task.Delay(TimeSpan.FromSeconds(5), cancellation.Token));
Assert.Same(runTask, completedTask);
await Assert.ThrowsAsync<InvalidOperationException>(async () => await runTask);
// Evidence the read was abandoned and that teardown took responsibility for it: the
// shared observe-with-timeout helper records it under the "PipeRead" tag either way.
// Which of the two entries lands is a timing detail, not a contract. The fault
// normally arrives at once (StopFailed), but Windows owes no deadline for a
// completion torn off a closed handle, so on a loaded box it can arrive after
// BackgroundTaskStopTimeout (StopTimedOut). Both are correct, because observation is
// unconditional — the helper attaches a fault-observing continuation when it gives
// up waiting — and the unobserved-exception assertion below is what actually pins
// that. That the transport really is closed is pinned deterministically by
// RunAsync_WhenSessionEnds_ClosesTransportBeforeTheHarnessDoes, so insisting on
// "StopFailed within 1s" here would buy nothing but a flake at the windev gate.
Assert.Contains(
logger.Events,
entry => (entry.EventName == "WorkerPipeSessionBackgroundTaskStopFailed"
|| entry.EventName == "WorkerPipeSessionBackgroundTaskStopTimedOut")
&& entry.Fields.TryGetValue("task", out object? task)
&& (task as string) == "PipeRead");
// Drive any task that faulted without an awaiter through its finalizer, which is
// what raises UnobservedTaskException. Nothing from the pipe read may surface.
GC.Collect();
GC.WaitForPendingFinalizers();
GC.Collect();
}
finally
{
TaskScheduler.UnobservedTaskException -= unobservedHandler;
}
lock (unobservedGate)
{
Assert.Empty(unobservedPipeExceptions);
}
}
}
private sealed class RecordingWorkerLogger : ZB.MOM.WW.MxGateway.Worker.Bootstrap.IWorkerLogger private sealed class RecordingWorkerLogger : ZB.MOM.WW.MxGateway.Worker.Bootstrap.IWorkerLogger
{ {
private readonly object gate = new(); private readonly object gate = new();
@@ -2484,3 +2711,19 @@ public sealed class WorkerPipeSessionTests
} }
} }
} }
/// <summary>
/// Collection for tests that observe process-global state and so cannot share the runner with
/// anything else. Its only member today is
/// <see cref="WorkerPipeSessionTests.AbandonedPipeReadTeardownTests"/>, which arms
/// <see cref="TaskScheduler.UnobservedTaskException"/> and forces a GC: a task faulting in any
/// concurrently running test class would be finalized inside that window and misread as this
/// session's orphaned pipe read. Keep membership minimal — every test added here is a test the
/// rest of the suite has to wait for.
/// </summary>
[CollectionDefinition(Name, DisableParallelization = true)]
public sealed class WorkerPipeSessionNonParallelCollection
{
/// <summary>Collection name referenced by <see cref="CollectionAttribute"/>.</summary>
public const string Name = "WorkerPipeSessionNonParallel";
}
@@ -669,6 +669,107 @@ public sealed class MxAccessCommandExecutorTests
Assert.Contains("RemoveItem:90:900", fakeComObject.OperationNames); Assert.Contains("RemoveItem:90:900", fakeComObject.OperationNames);
} }
/// <summary>
/// Verifies ReadBulk's cached fast path end to end — the
/// <c>was_cached = true</c> half of the command, which nothing else in
/// the worker suite exercises. With the tag already added AND advised
/// and a value in the per-session cache, the executor answers from the
/// cache: the result is successful and flagged cached, and no COM call
/// is made for the read at all, so the subscription the caller did not
/// create is left exactly as it was.
///
/// It also pins the borrow contract through the whole read path: the
/// <c>Value</c>, <c>SourceTimestamp</c>, and status row on the
/// <c>BulkReadResult</c> are the cached event's own instances, not
/// copies (see <see cref="MxAccessValueCache.Set"/>). The worker only
/// serializes them onto the IPC pipe, so the alias never escapes the
/// process.
///
/// Driven through <see cref="MxAccessCommandExecutor"/> directly rather
/// than <see cref="MxAccessStaSession"/> because seeding the cache needs
/// a handle on it: <see cref="MxAccessSession.Create"/> only shares the
/// sink's cache for the production <c>MxAccessBaseEventSink</c>, which
/// casts the COM object to <c>LMXProxyServerClass</c> and so cannot take
/// a fake, while <c>CreateForTesting</c> accepts the cache directly. The
/// cached path neither waits nor pumps, so it needs no STA.
/// </summary>
[Fact]
public void Execute_ReadBulk_WhenTagIsAdvisedAndCached_ServesTheCachedInstancesWithoutTouchingTheSubscription()
{
FakeMxAccessComObject fakeComObject = new(
registerHandle: 92,
addItemHandle: 920);
MxAccessValueCache valueCache = new();
using MxAccessSession session = MxAccessSession.CreateForTesting(
mxAccessServer: fakeComObject,
eventSink: new NoopEventSink(),
valueCache: valueCache);
MxAccessCommandExecutor executor = new(
session,
new ZB.MOM.WW.MxGateway.Worker.Conversion.VariantConverter());
// Registry half of the fast path: the tag must resolve to a live item
// handle on this server AND carry an advice, or TryGetCachedReadFor
// falls through to the AddItem/Advise snapshot lifecycle.
MxCommandReply registerReply = executor.Execute(
CreateRegisterCommand("register-before-cached-read", "client-a"));
MxCommandReply addItemReply = executor.Execute(
CreateAddItemCommand("add-before-cached-read", 92, "Galaxy.Tag.Value"));
MxCommandReply adviseReply = executor.Execute(
CreateAdviseCommand("advise-before-cached-read", 92, 920));
Assert.Equal(ProtocolStatusCode.Ok, registerReply.ProtocolStatus.Code);
Assert.Equal(ProtocolStatusCode.Ok, addItemReply.ProtocolStatus.Code);
Assert.Equal(ProtocolStatusCode.Ok, adviseReply.ProtocolStatus.Code);
// Cache half: stand in for the event sink's post-publish hook, which
// records the event it just enqueued.
MxEvent cachedEvent = new()
{
Family = MxEventFamily.OnDataChange,
ServerHandle = 92,
ItemHandle = 920,
Quality = 192,
SourceTimestamp = Timestamp.FromDateTime(new(2026, 8, 15, 10, 0, 0, DateTimeKind.Utc)),
Value = new MxValue
{
DataType = MxDataType.Integer,
VariantType = "VT_I4",
Int32Value = 7788,
},
OnDataChange = new OnDataChangeEvent(),
};
cachedEvent.Statuses.Add(new MxStatusProxy { Category = MxStatusCategory.Ok });
valueCache.Set(92, 920, cachedEvent);
MxCommandReply reply = executor.Execute(CreateReadBulkCommand(
"read-bulk-cached",
serverHandle: 92,
tagAddresses: new[] { "Galaxy.Tag.Value" },
timeoutMs: 80));
Assert.Equal(ProtocolStatusCode.Ok, reply.ProtocolStatus.Code);
Assert.Equal(MxCommandKind.ReadBulk, reply.Kind);
BulkReadResult result = Assert.Single(reply.ReadBulk.Results);
Assert.True(result.WasSuccessful);
Assert.True(result.WasCached);
Assert.Equal("Galaxy.Tag.Value", result.TagAddress);
Assert.Equal(920, result.ItemHandle);
Assert.Equal(192, result.Quality);
Assert.Equal(7788, result.Value.Int32Value);
// Borrowed, not copied — all the way from the event handed to
// MxAccessValueCache.Set out to the reply the worker serializes.
Assert.Same(cachedEvent.Value, result.Value);
Assert.Same(cachedEvent.SourceTimestamp, result.SourceTimestamp);
Assert.Same(cachedEvent.Statuses[0], Assert.Single(result.Statuses));
// No second AddItem, and above all no UnAdvise/RemoveItem: only the
// three setup calls ever reached MXAccess.
Assert.Equal(
new[] { "Register:client-a", "AddItem:92:Galaxy.Tag.Value", "Advise:92:920" },
fakeComObject.OperationNames);
}
/// <summary>Verifies that ReadBulk with no payload returns an invalid request error.</summary> /// <summary>Verifies that ReadBulk with no payload returns an invalid request error.</summary>
/// <returns>A task that represents the asynchronous operation.</returns> /// <returns>A task that represents the asynchronous operation.</returns>
[Fact] [Fact]
@@ -48,14 +48,28 @@ public sealed class MxAccessValueCacheTests
} }
/// <summary> /// <summary>
/// Verifies that Set stores an independent deep-copied snapshot: mutating /// Pins the ownership contract: <c>Set</c> borrows the event's own
/// the source event's protobuf sub-messages after caching does not alter /// protobuf sub-messages instead of deep-copying them, so <c>TryGet</c>
/// the cached value. WRK-11 stopped the event sink cloning before enqueue, /// hands back the very <c>Value</c>, <c>SourceTimestamp</c>, and
/// so the same MxEvent instance now flows to the outbound queue; the cache /// <c>MxStatusProxy</c> instances the caller passed in.
/// must own its own copy so the two never share mutable state. ///
/// This is safe only because the event is write-once by the time
/// <c>Set</c> runs: the sink enqueues it (which stamps the worker
/// sequence and timestamp) and only then post-publishes it here, and
/// <see cref="MxAccessEventQueue"/>'s ownership invariant forbids
/// mutating an enqueued event. This test therefore asserts reference
/// identity and deliberately does NOT mutate the event afterwards —
/// doing so is exactly what the contract forbids, and it would also
/// invalidate the serialized size the queue memoized at enqueue.
///
/// It replaced a test asserting the opposite (an independent deep-copied
/// snapshot). The three clones that test pinned — the MxValue, the
/// Timestamp, and the RepeatedField plus every status row in it — ran on
/// every OnDataChange and bought nothing: the read path already aliased
/// the cache's instances into every BulkReadResult.
/// </summary> /// </summary>
[Fact] [Fact]
public void Set_StoresIndependentSnapshot_UnaffectedByLaterEventMutation() public void Set_BorrowsTheEventsOwnInstances_ByOwnershipContract()
{ {
MxAccessValueCache cache = new(); MxAccessValueCache cache = new();
Timestamp sourceTimestamp = Timestamp.FromDateTime(new(2026, 5, 19, 9, 0, 0, DateTimeKind.Utc)); Timestamp sourceTimestamp = Timestamp.FromDateTime(new(2026, 5, 19, 9, 0, 0, DateTimeKind.Utc));
@@ -63,19 +77,12 @@ public sealed class MxAccessValueCacheTests
cache.Set(7, 21, mxEvent); cache.Set(7, 21, mxEvent);
// Mutate the event in place after it was cached — as if it kept flowing
// through the (unrelated) outbound path. None of this must reach the cache.
mxEvent.Value.Int32Value = 999;
mxEvent.Quality = 0;
mxEvent.SourceTimestamp = Timestamp.FromDateTime(new(2030, 1, 1, 0, 0, 0, DateTimeKind.Utc));
mxEvent.Statuses[0].Category = MxStatusCategory.SecurityError;
Assert.True(cache.TryGet(7, 21, out MxAccessValueCache.CachedValue cached)); Assert.True(cache.TryGet(7, 21, out MxAccessValueCache.CachedValue cached));
Assert.Equal(100, cached.Value.Int32Value); Assert.Same(mxEvent.Value, cached.Value);
Assert.Same(mxEvent.SourceTimestamp, cached.SourceTimestamp);
Assert.Same(mxEvent.Statuses, cached.Statuses);
Assert.Same(mxEvent.Statuses[0], Assert.Single(cached.Statuses));
Assert.Equal(192, cached.Quality); Assert.Equal(192, cached.Quality);
Assert.Equal(sourceTimestamp, cached.SourceTimestamp);
Assert.Single(cached.Statuses);
Assert.Equal(MxStatusCategory.Ok, cached.Statuses[0].Category);
} }
/// <summary>Verifies that TryGet returns false for unknown handles.</summary> /// <summary>Verifies that TryGet returns false for unknown handles.</summary>
@@ -22,6 +22,14 @@ public sealed class WorkerFrameReader
// Reused across frames rather than allocated per read (GWC-30). Safe because ReadAsync is // Reused across frames rather than allocated per read (GWC-30). Safe because ReadAsync is
// single-consumer by construction; the prefix is fully overwritten by every read. // single-consumer by construction; the prefix is fully overwritten by every read.
//
// The single-consumer invariant has to survive teardown as well as steady state, because a read
// abandoned by WorkerPipeSession's message loop still owns this buffer (and its rented payload
// buffer) until it faults. Nothing may call ReadAsync again after that point: a second read
// would race the abandoned one for the prefix, and could hand a pooled payload buffer back to
// ArrayPool twice. The loop guarantees it structurally — it only ever issues a read after
// awaiting the previous one, and it never re-enters after unwinding — and teardown only awaits
// the abandoned read, never reissues it (WorkerPipeSession.ObserveAbandonedPipeReadAsync).
private readonly byte[] _lengthPrefix = new byte[sizeof(uint)]; private readonly byte[] _lengthPrefix = new byte[sizeof(uint)];
/// <summary>Initializes the reader with a stream and protocol options.</summary> /// <summary>Initializes the reader with a stream and protocol options.</summary>
@@ -106,6 +114,12 @@ public sealed class WorkerFrameReader
int offset = 0; int offset = 0;
while (offset < count) while (offset < count)
{ {
// The token is forwarded but is NOT a bound on a pipe read: on .NET Framework 4.8
// NamedPipeClientStream.ReadAsync accepts a CancellationToken and never wires it to the
// overlapped I/O, so a read waiting on gateway bytes ignores cancellation entirely. Only
// closing the handle ends it (WorkerPipeSession disposes the transport at teardown for
// exactly this reason). It is still passed because non-pipe streams — the
// MemoryStream-backed unit tests, and any future transport — do honor it.
int bytesRead = await _stream int bytesRead = await _stream
.ReadAsync(buffer, offset, count - offset, cancellationToken) .ReadAsync(buffer, offset, count - offset, cancellationToken)
.ConfigureAwait(false); .ConfigureAwait(false);
@@ -14,9 +14,11 @@ namespace ZB.MOM.WW.MxGateway.Worker.Ipc;
/// Writes worker frames to a stream with length-prefixed protobuf serialization. Callers enqueue a /// Writes worker frames to a stream with length-prefixed protobuf serialization. Callers enqueue a
/// frame at a <see cref="WorkerFrameWritePriority"/> and then contend for a single write lock; whoever /// frame at a <see cref="WorkerFrameWritePriority"/> and then contend for a single write lock; whoever
/// holds the lock drains every queued frame, control frames first, so a reply, fault, or heartbeat is /// holds the lock drains every queued frame, control frames first, so a reply, fault, or heartbeat is
/// never delayed behind an event backlog. The envelope <c>Sequence</c> is stamped by the /// never delayed behind an event backlog — neither in the bytes it writes nor in the flush that
/// draining lock-holder at the moment of writing, so the on-wire order and the stamped sequence always /// delivers them, because the drain flushes at every control-to-event boundary rather than only at the
/// agree even under concurrent callers and priority reordering. /// end of the pass. The envelope <c>Sequence</c> is stamped by the draining lock-holder at the moment
/// of writing, so the on-wire order and the stamped sequence always agree even under concurrent callers
/// and priority reordering.
/// </summary> /// </summary>
public sealed class WorkerFrameWriter public sealed class WorkerFrameWriter
{ {
@@ -24,15 +26,25 @@ public sealed class WorkerFrameWriter
{ {
/// <summary>Initializes a new instance of the PendingFrame class.</summary> /// <summary>Initializes a new instance of the PendingFrame class.</summary>
/// <param name="envelope">Worker envelope awaiting write.</param> /// <param name="envelope">Worker envelope awaiting write.</param>
public PendingFrame(WorkerEnvelope envelope) /// <param name="priority">Priority class the frame was queued at.</param>
public PendingFrame(WorkerEnvelope envelope, WorkerFrameWritePriority priority)
{ {
Envelope = envelope; Envelope = envelope;
IsControl = priority != WorkerFrameWritePriority.Event;
Completion = new TaskCompletionSource<bool>(TaskCreationOptions.RunContinuationsAsynchronously); Completion = new TaskCompletionSource<bool>(TaskCreationOptions.RunContinuationsAsynchronously);
} }
/// <summary>Gets the worker envelope awaiting write.</summary> /// <summary>Gets the worker envelope awaiting write.</summary>
public WorkerEnvelope Envelope { get; } public WorkerEnvelope Envelope { get; }
/// <summary>
/// Gets a value indicating whether this frame was queued as control-plane traffic. Recorded at
/// construction from the same expression that picks the queue, so the class the drain sees can
/// never disagree with the queue the frame sits in. The drain uses it to flush and complete
/// written control frames at the moment it turns to events (see <see cref="DrainQueuedFramesAsync"/>).
/// </summary>
public bool IsControl { get; }
/// <summary>Gets the completion source signaled once the frame has been written or has failed.</summary> /// <summary>Gets the completion source signaled once the frame has been written or has failed.</summary>
public TaskCompletionSource<bool> Completion { get; } public TaskCompletionSource<bool> Completion { get; }
@@ -95,6 +107,15 @@ public sealed class WorkerFrameWriter
/// the canceller behind the very write it is abandoning would defeat the point of cancellation. /// the canceller behind the very write it is abandoning would defeat the point of cancellation.
/// The abandoned frame's completion gets a fault-observing continuation so a write failure after /// The abandoned frame's completion gets a fault-observing continuation so a write failure after
/// the caller unwinds never raises an unobserved-task exception (NEXT-04). /// the caller unwinds never raises an unobserved-task exception (NEXT-04).
/// <para>
/// Latency contract: a control frame's bytes are written, flushed, and its completion
/// resolved before the events a drain pass writes after it — the delivery point of a heartbeat,
/// reply, fault, or shutdown ack is never charged for the event backlog behind it. The returned
/// task can still be later than that instant for a caller that lost the write-lock race: it only
/// observes its completion after the winning drainer releases the lock, so its own return remains
/// bounded by that pass. That parking is deliberate — the alternative is to race the lock wait
/// against the completion, which buys nothing for the frame's delivery.
/// </para>
/// </remarks> /// </remarks>
public async Task WriteAsync( public async Task WriteAsync(
WorkerEnvelope envelope, WorkerEnvelope envelope,
@@ -106,7 +127,7 @@ public sealed class WorkerFrameWriter
throw new ArgumentNullException(nameof(envelope)); throw new ArgumentNullException(nameof(envelope));
} }
PendingFrame frame = new PendingFrame(envelope); PendingFrame frame = new PendingFrame(envelope, priority);
lock (_gate) lock (_gate)
{ {
if (priority == WorkerFrameWritePriority.Event) if (priority == WorkerFrameWritePriority.Event)
@@ -153,8 +174,14 @@ public sealed class WorkerFrameWriter
/// rather than one per frame (WRK-25, realizing the WRK-12 coalescing on the path it was built /// rather than one per frame (WRK-25, realizing the WRK-12 coalescing on the path it was built
/// for). Intra-batch order is preserved because the enqueue is atomic under <c>_gate</c> and each /// for). Intra-batch order is preserved because the enqueue is atomic under <c>_gate</c> and each
/// class queue is FIFO; the control-before-event guarantee still holds because any concurrently /// class queue is FIFO; the control-before-event guarantee still holds because any concurrently
/// queued control frame is drained ahead of this batch by <see cref="DequeueNext"/>. Every frame's /// queued control frame is drained ahead of this batch by <see cref="DequeueNext"/>, and — since
/// "written and flushed before completion" contract is unchanged. /// the control-frame completion decoupling — is also flushed and completed before this batch's
/// 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. 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> /// </summary>
/// <param name="envelopes">Envelopes to write, in order.</param> /// <param name="envelopes">Envelopes to write, in order.</param>
/// <param name="priority">Scheduling priority for the whole batch.</param> /// <param name="priority">Scheduling priority for the whole batch.</param>
@@ -189,7 +216,7 @@ public sealed class WorkerFrameWriter
{ {
WorkerEnvelope envelope = envelopes[index] WorkerEnvelope envelope = envelopes[index]
?? throw new ArgumentException("Batch envelopes must not contain null.", nameof(envelopes)); ?? throw new ArgumentException("Batch envelopes must not contain null.", nameof(envelopes));
frames[index] = new PendingFrame(envelope); frames[index] = new PendingFrame(envelope, priority);
} }
lock (_gate) lock (_gate)
@@ -296,14 +323,24 @@ public sealed class WorkerFrameWriter
// The stream write itself is not cancellable: a frame is written atomically or fails, never left // The stream write itself is not cancellable: a frame is written atomically or fails, never left
// half-written on the pipe because a caller gave up waiting. // half-written on the pipe because a caller gave up waiting.
// //
// Flushes are coalesced across the whole drained batch (WRK-12 / IPC-15): each frame is written to // Flushes are coalesced within a priority class rather than blindly across the whole pass (WRK-12 /
// the stream but not flushed individually; a single FlushAsync runs after the batch, then every // IPC-15, narrowed by the control-frame completion decoupling): each frame is written to the stream
// successfully-written frame is completed. A caller's Completion therefore still signals only after // but not flushed individually, and one FlushAsync runs at the end of the pass — plus one at each
// its bytes have been written AND flushed, so the "written and flushed" contract is unchanged — but // control-to-event boundary, which flushes and completes the control frames written so far before
// a burst of N events now costs one flush syscall instead of N. // the event backlog behind them is written, instead of after it. Without that boundary flush the
// priority scheduler only got control *bytes* out early: their delivery point, and every waiting
// caller's completion, still sat behind up to a full event batch.
//
// A caller's Completion therefore still signals only after its bytes have been written AND flushed —
// the contract is unchanged, the moment it is reached simply stops being pinned to the end of the
// pass. Cost is bounded: a pure-event pass (the event hot path) still pays exactly one flush, a burst
// of control frames still pays one for the whole burst, and only a pass that actually mixes both
// classes pays a second — never one flush per control frame, which is the syscall-per-heartbeat cost
// WRK-12 removed.
private async Task DrainQueuedFramesAsync() private async Task DrainQueuedFramesAsync()
{ {
List<PendingFrame> written = new List<PendingFrame>(); List<PendingFrame> written = new List<PendingFrame>();
bool writtenHoldsControl = false;
while (true) while (true)
{ {
PendingFrame? frame = DequeueNext(); PendingFrame? frame = DequeueNext();
@@ -312,10 +349,40 @@ public sealed class WorkerFrameWriter
break; break;
} }
if (writtenHoldsControl && !frame.IsControl)
{
// Class transition: the frames written so far include at least one control frame whose
// caller is waiting on delivery. Flush and complete them here rather than parking them
// behind the events this pass is about to write. Charged once per transition, not once
// per control frame. Event frames already in the list ride along — they too are written
// and now flushed, so completing them early is the same contract, earlier.
try
{
await _stream.FlushAsync(CancellationToken.None).ConfigureAwait(false);
}
catch (Exception exception)
{
// Same shape as the end-of-pass flush failure: the bytes reached the stream but the
// flush that guarantees delivery failed, so the pipe is broken. Fail the frame just
// claimed (it is out of its queue and nothing else will ever complete it), every
// written-but-unflushed frame, and everything still queued, then stop draining.
frame.Completion.TrySetException(exception);
FailFrames(written, exception);
FailAllQueued(exception);
return;
}
// Completed frames leave the list, so a later failure in this pass cannot fail them.
CompleteFrames(written);
written.Clear();
writtenHoldsControl = false;
}
try try
{ {
await WriteFrameAsync(frame.Envelope).ConfigureAwait(false); await WriteFrameAsync(frame.Envelope).ConfigureAwait(false);
written.Add(frame); written.Add(frame);
writtenHoldsControl |= frame.IsControl;
} }
catch (WorkerFrameProtocolException exception) when (IsPerFrameRejection(exception)) catch (WorkerFrameProtocolException exception) when (IsPerFrameRejection(exception))
{ {
@@ -348,13 +415,19 @@ public sealed class WorkerFrameWriter
catch (Exception exception) catch (Exception exception)
{ {
// The batch reached the stream but the flush that guarantees delivery failed: the pipe is // The batch reached the stream but the flush that guarantees delivery failed: the pipe is
// broken. Fail every frame in the batch (the queue was already drained) so no caller treats // broken. Fail every frame still in the batch (the queue was already drained) so no caller
// an unflushed write as delivered. // treats an unflushed write as delivered. Frames a boundary flush already completed are not
// in the list — their bytes were flushed, so this later failure does not reach back to them.
FailFrames(written, exception); FailFrames(written, exception);
return; return;
} }
foreach (PendingFrame frame in written) CompleteFrames(written);
}
private static void CompleteFrames(List<PendingFrame> frames)
{
foreach (PendingFrame frame in frames)
{ {
frame.Completion.TrySetResult(true); frame.Completion.TrySetResult(true);
} }
@@ -151,6 +151,13 @@ public sealed class WorkerPipeClient : IWorkerPipeClient
WorkerFrameProtocolOptions frameOptions = new(options); WorkerFrameProtocolOptions frameOptions = new(options);
// The session disposes this pipe itself as its last teardown step — that disposal is what
// unblocks a net48 pipe read the message loop abandoned, and it has to happen while the
// session still holds the read task so the resulting fault is observed rather than orphaned
// (see WorkerPipeSession.RunAsync). The `using` stays as the backstop for the paths the
// session never reaches: a session factory that throws, or a RunAsync that never gets past
// its own construction. Disposal is idempotent, so the second Dispose is a no-op — do not
// "clean up" this `using` on the assumption that it is now redundant.
using NamedPipeClientStream pipe = await ConnectWithRetryAsync(options.PipeName, cancellationToken) using NamedPipeClientStream pipe = await ConnectWithRetryAsync(options.PipeName, cancellationToken)
.ConfigureAwait(false); .ConfigureAwait(false);
@@ -39,8 +39,21 @@ public sealed class WorkerPipeSession
private readonly WorkerFrameWriter _writer; private readonly WorkerFrameWriter _writer;
private readonly object _commandTaskGate = new(); private readonly object _commandTaskGate = new();
private readonly HashSet<Task> _activeCommandTasks = new(); private readonly HashSet<Task> _activeCommandTasks = new();
// The transport the reader and writer share, when this session was handed one to own. Null for
// the reader/writer constructors, whose callers keep ownership of whatever streams they built the
// pair over. Owning it is what lets teardown close the handle (WRK-31): on net48 a pipe read
// parked in the kernel cannot be cancelled, only unblocked by disposal.
private readonly Stream? _transportStream;
private IWorkerRuntimeSession? _runtimeSession; private IWorkerRuntimeSession? _runtimeSession;
// The one outstanding, not-yet-awaited frame read, or null when no read is in flight. Written
// only by the message loop (which sets it at each read issue and clears it before awaiting that
// read itself) and read only by RunAsync's finally — which runs after the loop's task has
// completed, so the await supplies the happens-before edge and no interlock is needed.
private Task<WorkerEnvelope>? _pendingReadTask;
// Mutated from the message loop, command tasks, the heartbeat loop and the // Mutated from the message loop, command tasks, the heartbeat loop and the
// shutdown path; volatile so cross-thread reads observe the latest state // shutdown path; volatile so cross-thread reads observe the latest state
// without tearing (WorkerState is an int-backed protobuf enum). // without tearing (WorkerState is an int-backed protobuf enum).
@@ -63,7 +76,8 @@ public sealed class WorkerPipeSession
() => Process.GetCurrentProcess().Id, () => Process.GetCurrentProcess().Id,
new WorkerPipeSessionOptions(), new WorkerPipeSessionOptions(),
() => new MxAccessStaSession((eq, affinity, comFactory) => new AlarmCommandHandler(eq, () => new WnWrapAlarmConsumer(), affinity, comFactory, standbyFactory: null)), () => new MxAccessStaSession((eq, affinity, comFactory) => new AlarmCommandHandler(eq, () => new WnWrapAlarmConsumer(), affinity, comFactory, standbyFactory: null)),
logger) logger,
stream)
{ {
} }
@@ -96,6 +110,12 @@ public sealed class WorkerPipeSession
/// <param name="sessionOptions">Session-specific options.</param> /// <param name="sessionOptions">Session-specific options.</param>
/// <param name="runtimeSessionFactory">Factory creating the MXAccess runtime session.</param> /// <param name="runtimeSessionFactory">Factory creating the MXAccess runtime session.</param>
/// <param name="logger">Optional logger for diagnostic output.</param> /// <param name="logger">Optional logger for diagnostic output.</param>
/// <param name="transportStream">
/// Stream the reader and writer share, when this session is to own it. Supplying it makes
/// <see cref="RunAsync"/> dispose the transport as its last teardown step, which is the only
/// way to unblock a pending net48 pipe read (see <see cref="RunMessageLoopAsync"/>). Null
/// leaves ownership — and the disposal — with the caller.
/// </param>
public WorkerPipeSession( public WorkerPipeSession(
WorkerFrameReader reader, WorkerFrameReader reader,
WorkerFrameWriter writer, WorkerFrameWriter writer,
@@ -103,7 +123,8 @@ public sealed class WorkerPipeSession
Func<int> processIdProvider, Func<int> processIdProvider,
WorkerPipeSessionOptions sessionOptions, WorkerPipeSessionOptions sessionOptions,
Func<IWorkerRuntimeSession> runtimeSessionFactory, Func<IWorkerRuntimeSession> runtimeSessionFactory,
IWorkerLogger? logger = null) IWorkerLogger? logger = null,
Stream? transportStream = null)
{ {
_reader = reader ?? throw new ArgumentNullException(nameof(reader)); _reader = reader ?? throw new ArgumentNullException(nameof(reader));
_writer = writer ?? throw new ArgumentNullException(nameof(writer)); _writer = writer ?? throw new ArgumentNullException(nameof(writer));
@@ -112,6 +133,7 @@ public sealed class WorkerPipeSession
_sessionOptions = sessionOptions ?? throw new ArgumentNullException(nameof(sessionOptions)); _sessionOptions = sessionOptions ?? throw new ArgumentNullException(nameof(sessionOptions));
_runtimeSessionFactory = runtimeSessionFactory ?? throw new ArgumentNullException(nameof(runtimeSessionFactory)); _runtimeSessionFactory = runtimeSessionFactory ?? throw new ArgumentNullException(nameof(runtimeSessionFactory));
_logger = logger; _logger = logger;
_transportStream = transportStream;
_sessionOptions.Validate(); _sessionOptions.Validate();
} }
@@ -142,9 +164,117 @@ public sealed class WorkerPipeSession
_runtimeSession?.Dispose(); _runtimeSession?.Dispose();
_runtimeSession = null; _runtimeSession = null;
_state = WorkerState.Stopped; _state = WorkerState.Stopped;
// Closing the transport is what actually ends a pipe read parked in the kernel: on net48
// NamedPipeClientStream.ReadAsync ignores its CancellationToken, so the message loop's
// cancellation can never reach one (WRK-31). It is deliberately the LAST teardown step,
// because in the ordinary case every frame this session will ever write has completed by
// the time control reaches here: WorkerFrameWriter.WriteAsync signals only after the
// frame is written AND flushed, and each exit path awaits its final write before
// unwinding — the shutdown ack and shutdown-timeout fault inside the loop's dispatch, the
// event-drain and oversized-event faults inside the drain task the loop awaits, the
// watchdog fault inside the heartbeat task the loop awaits, and the handshake fault
// inside CompleteStartupHandshakeAsync's catch.
//
// "Ordinary" is the honest word, not "always": the loop's wait on the heartbeat and
// drain tasks is budgeted (BackgroundTaskStopTimeout), and a stream write is genuinely
// uncancellable, so a write that overran the budget can still be in flight against the
// stream being disposed here. In-flight command replies are in the same position, and
// they raced the identical disposal before this change (WorkerPipeClient's `using` fired
// on the very next statement after RunAsync). That is precisely why disposal below is
// exception-tolerant and why every abandoned task gets a fault-observing continuation
// from ObserveBackgroundTaskStopAsync — a write losing its stream mid-flight must be a
// logged non-event, not a lost terminal exception or an unobserved task.
//
// Owning the disposal here — rather than leaving it to WorkerPipeClient's `using` — is
// what makes the abandoned read observable: the fault it takes on disposal lands on a
// task this session still holds, so ObserveAbandonedPipeReadAsync can await it instead of
// leaving it for a TaskScheduler.UnobservedTaskException handler the worker does not have.
DisposeTransportStream();
await ObserveAbandonedPipeReadAsync().ConfigureAwait(false);
} }
} }
/// <summary>
/// Disposes the transport this session owns, if it was handed one. Dispose-time failures are
/// logged and swallowed: this runs inside <see cref="RunAsync"/>'s finally, where letting a
/// failure escape would replace the exception that actually ended the session (a shutdown
/// timeout, a protocol violation, an event too large to frame) with a far less actionable
/// one. Disposal is idempotent, so <c>WorkerPipeClient</c>'s outer <c>using</c> re-disposing
/// the same stream immediately afterwards is a no-op.
/// </summary>
/// <remarks>
/// The catch is deliberately total rather than the <see cref="IOException"/> /
/// <see cref="ObjectDisposedException"/> pair the fault-write paths use, matching
/// <see cref="ObserveBackgroundTaskStopAsync"/>'s shape. Narrowing it to the expected types
/// would let an unexpected one — a <c>Win32Exception</c> surfaced by the handle close, say —
/// do the exact harm this guard exists to prevent. The rule is about the position in the
/// code, not about which exceptions are plausible: nothing thrown while releasing a handle
/// is more actionable than the session's terminal exception, so nothing may displace it.
/// </remarks>
private void DisposeTransportStream()
{
if (_transportStream is null)
{
return;
}
try
{
_transportStream.Dispose();
}
catch (Exception exception)
{
_logger?.Error(
"WorkerPipeSessionTransportDisposeFailed",
new Dictionary<string, object?>
{
["session_id"] = _options.SessionId,
["exception"] = exception.ToString(),
});
}
}
/// <summary>
/// Awaits the frame read the message loop walked away from, if there is one.
/// <see cref="RunMessageLoopAsync"/> races an uncancellable read against the heartbeat and
/// event-drain loops, so every fault exit (event-drain fault, oversized event, heartbeat
/// write failure) leaves a read outstanding on a task the loop never awaits again. Once
/// <see cref="DisposeTransportStream"/> has closed the handle that read faults with
/// <see cref="ObjectDisposedException"/>, <see cref="IOException"/>, or a zero-byte read
/// mapped to <c>EndOfStream</c>, and that fault has to be observed — the worker installs no
/// <c>TaskScheduler.UnobservedTaskException</c> handler.
/// </summary>
/// <remarks>
/// <para>
/// The fault is observed <em>unconditionally</em>; it is only <em>logged</em> within
/// <see cref="BackgroundTaskStopTimeout"/>. Windows is under no obligation to deliver
/// the abandoned read's completion inside that budget, so a bounded await alone would
/// reopen the orphaning window it was added to close. <see cref="ObserveBackgroundTaskStopAsync"/>
/// therefore hands the task a fault-observing continuation when it gives up waiting,
/// which makes the budget a diagnostics decision rather than a correctness one.
/// </para>
/// <para>
/// No second read can follow this one. The message loop only ever issues a read after
/// awaiting the previous one, and it never re-enters after unwinding, so
/// <see cref="WorkerFrameReader"/>'s single-consumer invariant — and with it the safety
/// of its reused length-prefix buffer and its pooled payload buffer, which the abandoned
/// read still owns until it faults — holds through teardown.
/// </para>
/// </remarks>
/// <returns>A task that represents the asynchronous operation.</returns>
private async Task ObserveAbandonedPipeReadAsync()
{
Task<WorkerEnvelope>? readTask = _pendingReadTask;
_pendingReadTask = null;
if (readTask is null)
{
return;
}
await ObserveBackgroundTaskStopAsync(readTask, "PipeRead").ConfigureAwait(false);
}
/// <summary>Completes the gateway startup handshake using default MXAccess initialization.</summary> /// <summary>Completes the gateway startup handshake using default MXAccess initialization.</summary>
/// <param name="cancellationToken">Token to cancel the asynchronous operation.</param> /// <param name="cancellationToken">Token to cancel the asynchronous operation.</param>
/// <returns>A task that represents the asynchronous operation.</returns> /// <returns>A task that represents the asynchronous operation.</returns>
@@ -264,6 +394,34 @@ public sealed class WorkerPipeSession
return _writer.WriteAsync(CreateEnvelope(ready), cancellationToken); return _writer.WriteAsync(CreateEnvelope(ready), cancellationToken);
} }
/// <summary>
/// Runs the post-handshake message loop, racing one outstanding frame read against the
/// heartbeat and event-drain loops.
/// </summary>
/// <remarks>
/// <para>
/// <c>loopCancellation</c> does NOT bound the read. On .NET Framework 4.8
/// <c>NamedPipeClientStream.ReadAsync</c> accepts a <see cref="CancellationToken"/> and
/// then ignores it — the token never reaches the overlapped I/O, so a read parked
/// waiting for gateway bytes stays parked no matter what is cancelled. The token is
/// still passed because the reader's contract takes one and a non-pipe stream (the
/// MemoryStream-backed unit tests) does honor it. What actually ends a parked read is
/// closing the handle, which <see cref="RunAsync"/> does at the end of teardown.
/// </para>
/// <para>
/// That asymmetry is why the loop records its outstanding read in
/// <c>_pendingReadTask</c>. Every fault exit — an event-drain fault, an event too large
/// to frame, a failed heartbeat write — unwinds through <c>Task.WhenAny</c> while the
/// read is still pending, and the fault that read eventually takes has to be observed by
/// somebody (see <see cref="ObserveAbandonedPipeReadAsync"/>). The field is cleared
/// before the loop awaits a read itself, so it is non-null exactly when a read is
/// outstanding and unobserved. Graceful exits — the <c>return</c> below, after a
/// <c>WorkerShutdown</c> envelope or a <c>ShutdownWorker</c> command — leave no pending
/// read at all, so teardown's observation is a no-op there.
/// </para>
/// </remarks>
/// <param name="cancellationToken">Token to cancel the asynchronous operation.</param>
/// <returns>A task that represents the asynchronous operation.</returns>
private async Task RunMessageLoopAsync(CancellationToken cancellationToken) private async Task RunMessageLoopAsync(CancellationToken cancellationToken)
{ {
using CancellationTokenSource loopCancellation = CancellationTokenSource using CancellationTokenSource loopCancellation = CancellationTokenSource
@@ -272,7 +430,11 @@ public sealed class WorkerPipeSession
.CreateLinkedTokenSource(cancellationToken); .CreateLinkedTokenSource(cancellationToken);
Task heartbeatTask = RunHeartbeatLoopAsync(heartbeatCancellation.Token); Task heartbeatTask = RunHeartbeatLoopAsync(heartbeatCancellation.Token);
Task eventDrainTask = RunEventDrainLoopAsync(heartbeatCancellation.Token); Task eventDrainTask = RunEventDrainLoopAsync(heartbeatCancellation.Token);
Debug.Assert(
_pendingReadTask is null,
"A frame read must never be issued while another is outstanding: WorkerFrameReader is single-consumer, and a second read would race the abandoned one for the reused prefix buffer and could return a pooled payload buffer twice.");
Task<WorkerEnvelope> readTask = _reader.ReadAsync(loopCancellation.Token); Task<WorkerEnvelope> readTask = _reader.ReadAsync(loopCancellation.Token);
_pendingReadTask = readTask;
try try
{ {
@@ -281,6 +443,9 @@ public sealed class WorkerPipeSession
Task completedTask = await Task.WhenAny(readTask, heartbeatTask, eventDrainTask).ConfigureAwait(false); Task completedTask = await Task.WhenAny(readTask, heartbeatTask, eventDrainTask).ConfigureAwait(false);
if (completedTask == readTask) if (completedTask == readTask)
{ {
// The loop observes this read itself, whether it yields an envelope or throws,
// so it is no longer the abandoned one teardown has to account for.
_pendingReadTask = null;
WorkerEnvelope envelope = await readTask.ConfigureAwait(false); WorkerEnvelope envelope = await readTask.ConfigureAwait(false);
bool keepReading = await DispatchGatewayEnvelopeAsync(envelope, cancellationToken).ConfigureAwait(false); bool keepReading = await DispatchGatewayEnvelopeAsync(envelope, cancellationToken).ConfigureAwait(false);
if (!keepReading) if (!keepReading)
@@ -288,7 +453,11 @@ public sealed class WorkerPipeSession
return; return;
} }
Debug.Assert(
_pendingReadTask is null,
"The previous read must have been awaited before the next is issued: WorkerFrameReader is single-consumer.");
readTask = _reader.ReadAsync(loopCancellation.Token); readTask = _reader.ReadAsync(loopCancellation.Token);
_pendingReadTask = readTask;
} }
else if (completedTask == heartbeatTask) else if (completedTask == heartbeatTask)
{ {
@@ -309,6 +478,32 @@ public sealed class WorkerPipeSession
} }
} }
/// <summary>
/// Waits a bounded time for a background task to stop and records what happened.
/// </summary>
/// <remarks>
/// <para>
/// The budget bounds the <em>logging</em>, never the observation. Every task this is
/// asked to observe is uncancellable at the point that matters — a net48 pipe read
/// ignores its token outright, and <c>WorkerFrameWriter.WriteFrameAsync</c> issues the
/// stream write under <c>CancellationToken.None</c> so a frame is never left
/// half-written on the wire — so any of them can outlive the budget and only then fault,
/// typically against a transport <see cref="RunAsync"/> has since disposed. Overrunning
/// the budget and returning is therefore not enough: the task would be left with nobody
/// holding it, which is the exact orphaning this method exists to prevent.
/// </para>
/// <para>
/// So the timeout path hands the task a fault-observing continuation before returning.
/// The fault is then observed unconditionally, whenever it arrives; the budget only
/// decides whether it also gets logged here or is swallowed silently by the
/// continuation. That distinction matters because the worker installs no
/// <c>TaskScheduler.UnobservedTaskException</c> handler, so an unheld faulted task
/// surfaces only at finalization.
/// </para>
/// </remarks>
/// <param name="task">Background task being stopped.</param>
/// <param name="taskName">Name recorded in the diagnostic logs.</param>
/// <returns>A task that represents the asynchronous operation.</returns>
private async Task ObserveBackgroundTaskStopAsync( private async Task ObserveBackgroundTaskStopAsync(
Task task, Task task,
string taskName) string taskName)
@@ -318,6 +513,7 @@ public sealed class WorkerPipeSession
.ConfigureAwait(false); .ConfigureAwait(false);
if (completedTask != task) if (completedTask != task)
{ {
ObserveFaultWhenever(task);
_logger?.Error( _logger?.Error(
"WorkerPipeSessionBackgroundTaskStopTimedOut", "WorkerPipeSessionBackgroundTaskStopTimedOut",
new Dictionary<string, object?> new Dictionary<string, object?>
@@ -347,6 +543,24 @@ public sealed class WorkerPipeSession
} }
} }
/// <summary>
/// Attaches a continuation that observes <paramref name="task"/>'s exception whenever the
/// task eventually faults, so a task nobody is awaiting any more can never reach the
/// finalizer with an unobserved exception. Mirrors the shape
/// <c>WorkerFrameWriter.ObserveAbandonedFault</c> uses for frames a cancelled caller stops
/// awaiting (NEXT-04). Faulting is the only outcome that runs the continuation, and the
/// continuation is scheduled inline, so this costs nothing on the ordinary path.
/// </summary>
/// <param name="task">Task that may fault after its awaiter has walked away.</param>
private static void ObserveFaultWhenever(Task task)
{
_ = task.ContinueWith(
static faultedTask => _ = faultedTask.Exception,
CancellationToken.None,
TaskContinuationOptions.OnlyOnFaulted | TaskContinuationOptions.ExecuteSynchronously,
TaskScheduler.Default);
}
private async Task RunEventDrainLoopAsync(CancellationToken cancellationToken) private async Task RunEventDrainLoopAsync(CancellationToken cancellationToken)
{ {
while (!cancellationToken.IsCancellationRequested) while (!cancellationToken.IsCancellationRequested)
@@ -20,8 +20,11 @@ namespace ZB.MOM.WW.MxGateway.Worker.MxAccess;
/// <see cref="MxEvent"/> that it does not retain, reuse, or mutate after the /// <see cref="MxEvent"/> that it does not retain, reuse, or mutate after the
/// call returns. All production callers (MxAccessBaseEventSink, /// call returns. All production callers (MxAccessBaseEventSink,
/// MxAccessAlarmEventSink, AlarmCommandHandler) build a new event per /// MxAccessAlarmEventSink, AlarmCommandHandler) build a new event per
/// Enqueue via the mapper and satisfy this; the value cache stores its own /// Enqueue via the mapper and satisfy this. "Does not mutate" is the load-
/// independent snapshot (see <see cref="MxAccessValueCache.Set"/>). /// bearing half for the post-publish value cache, which deliberately borrows
/// the enqueued event's own value/timestamp/status instances rather than
/// copying them (see <see cref="MxAccessValueCache.Set"/>) — it reads and
/// serializes them, and this invariant is what keeps that safe.
/// <para> /// <para>
/// The byte-budgeted <see cref="Drain(uint, int)"/> relies on the same invariant: each /// The byte-budgeted <see cref="Drain(uint, int)"/> relies on the same invariant: each
/// event's serialized size is measured once at enqueue and stored beside it, which is /// event's serialized size is measured once at enqueue and stored beside it, which is
@@ -57,7 +57,12 @@ public sealed class MxAccessValueCache
/// <summary>Records a fresh OnDataChange payload for the given handle pair.</summary> /// <summary>Records a fresh OnDataChange payload for the given handle pair.</summary>
/// <param name="serverHandle">MXAccess server handle.</param> /// <param name="serverHandle">MXAccess server handle.</param>
/// <param name="itemHandle">MXAccess item handle.</param> /// <param name="itemHandle">MXAccess item handle.</param>
/// <param name="mxEvent">The protobuf MxEvent created by the event mapper.</param> /// <param name="mxEvent">
/// The protobuf MxEvent created by the event mapper, already handed to
/// the outbound queue. The cache borrows this event's
/// <c>Value</c>/<c>SourceTimestamp</c>/<c>Statuses</c> instances rather
/// than copying them — see the ownership contract in the method body.
/// </param>
public void Set( public void Set(
int serverHandle, int serverHandle,
int itemHandle, int itemHandle,
@@ -68,20 +73,39 @@ public sealed class MxAccessValueCache
throw new ArgumentNullException(nameof(mxEvent)); throw new ArgumentNullException(nameof(mxEvent));
} }
// WRK-11: the event sink no longer clones before enqueue, so the passed // Ownership contract: the cache BORROWS, it does not copy. CachedValue
// mxEvent is the very instance handed to the outbound queue. Deep-copy // retains the event's own Value, SourceTimestamp, and Statuses
// the value/timestamp/statuses payload we retain here so the cache's // instances.
// snapshot stays independent of the enqueued (and later serialized) //
// event — the two must never share mutable protobuf sub-messages. // Sound because the event is write-once by the time this runs.
// Value is always set for OnDataChange; SourceTimestamp may be unset when // MxAccessBaseEventSink.EnqueueEvent calls eventQueue.Enqueue first —
// the source timestamp could not be parsed, so both are cloned only when // which stamps WorkerSequence/WorkerTimestamp inside the queue lock —
// present. The null-forgiving result matches CachedValue's non-null- // and only then runs the postPublish hook that lands here; the queue's
// annotated parameters, which already accepted a runtime-null value or // ownership invariant (see MxAccessEventQueue's class remarks) forbids
// timestamp before WRK-11 (the ternary keeps the compiler's null-state // mutating an event after it is enqueued. The producer side never
// from poisoning to maybe-null, which a plain null check would do). // reuses instances either: the mapper builds a fresh MxEvent, and the
MxValue cachedValue = mxEvent.Value is null ? null! : mxEvent.Value.Clone(); // VariantConverter a fresh MxValue, per COM callback. The three deep
Timestamp cachedTimestamp = mxEvent.SourceTimestamp is null ? null! : mxEvent.SourceTimestamp.Clone(); // copies this replaced (the MxValue — recursive for an MxArray — the
// Timestamp, and the RepeatedField container plus every MxStatusProxy
// in it) therefore bought nothing but garbage on the worker's hottest
// path.
//
// Consumers of TryGet / TryWaitForUpdate may read and serialize what
// they get back; they must never mutate it. ReadBulk already depends on
// that: MxAccessSession.SucceededRead puts these very instances on the
// BulkReadResult it returns, which the worker only serializes onto the
// IPC pipe — worker↔gateway is a process boundary, so no gateway-side
// consumer can alias them. Mutating one would corrupt the event still
// queued for the outbound stream AND invalidate QueuedEvent.Size, the
// serialized size memoized at enqueue that the byte-budgeted Drain
// charges against its budget: a message grown after enqueue could
// overshoot the negotiated frame max and fault the session with
// MessageTooLarge (WorkerPipeSession.FaultOnOversizedEventAsync).
//
// Value is always set for OnDataChange; SourceTimestamp can be null when
// the source timestamp could not be parsed. CachedValue's parameters are
// annotated non-null but have always accepted a runtime null for both,
// and the read side null-checks accordingly.
long key = CreateItemKey(serverHandle, itemHandle); long key = CreateItemKey(serverHandle, itemHandle);
lock (syncRoot) lock (syncRoot)
{ {
@@ -91,10 +115,10 @@ public sealed class MxAccessValueCache
entries[key] = new CachedValue( entries[key] = new CachedValue(
nextVersion, nextVersion,
cachedValue, mxEvent.Value,
mxEvent.Quality, mxEvent.Quality,
cachedTimestamp, mxEvent.SourceTimestamp,
mxEvent.Statuses.Clone()); mxEvent.Statuses);
} }
// Signaled outside the lock: the waiter re-takes syncRoot (through // Signaled outside the lock: the waiter re-takes syncRoot (through
@@ -228,23 +252,35 @@ public sealed class MxAccessValueCache
} }
/// <summary> /// <summary>
/// Snapshot of the most recent OnDataChange payload for a handle pair. /// The most recent OnDataChange payload for a handle pair.
/// <see cref="Version"/> increments by one on every <see cref="Set"/> /// <see cref="Version"/> increments by one on every <see cref="Set"/>
/// call so the bulk read executor can detect "a new value arrived /// call so the bulk read executor can detect "a new value arrived
/// since I started waiting". /// since I started waiting".
/// </summary> /// </summary>
/// <remarks> /// <remarks>
/// Plain readonly struct (not a record) so this compiles under the /// <para>
/// worker's net48 target, which lacks <c>IsExternalInit</c>. /// Borrowed references, not copies. <see cref="Value"/>,
/// <see cref="SourceTimestamp"/>, and <see cref="Statuses"/> are the
/// very instances hanging off the MxEvent that was enqueued for the
/// outbound stream — <see cref="Set"/> carries the full ownership
/// contract. Read them and serialize them; never mutate them and
/// never hand them to something that will. That event is write-once
/// from the moment it is enqueued, and its serialized size is
/// memoized at that point for the byte-budgeted drain.
/// </para>
/// <para>
/// Plain readonly struct (not a record) so this compiles under the
/// worker's net48 target, which lacks <c>IsExternalInit</c>.
/// </para>
/// </remarks> /// </remarks>
public readonly struct CachedValue public readonly struct CachedValue
{ {
/// <summary>Initializes a new cached value snapshot.</summary> /// <summary>Initializes a new cached value snapshot.</summary>
/// <param name="version">Version counter incremented on each update.</param> /// <param name="version">Version counter incremented on each update.</param>
/// <param name="value">The MXAccess value.</param> /// <param name="value">The MXAccess value, borrowed from the enqueued event.</param>
/// <param name="quality">The MXAccess quality code.</param> /// <param name="quality">The MXAccess quality code.</param>
/// <param name="sourceTimestamp">The source timestamp of the value.</param> /// <param name="sourceTimestamp">The source timestamp of the value, borrowed from the enqueued event.</param>
/// <param name="statuses">The MXAccess status codes.</param> /// <param name="statuses">The MXAccess status codes, borrowed from the enqueued event.</param>
public CachedValue( public CachedValue(
ulong version, ulong version,
MxValue value, MxValue value,
@@ -262,16 +298,16 @@ public sealed class MxAccessValueCache
/// <summary>Monotonic per-handle version counter.</summary> /// <summary>Monotonic per-handle version counter.</summary>
public ulong Version { get; } public ulong Version { get; }
/// <summary>The cached MxValue payload.</summary> /// <summary>The OnDataChange event's own MxValue payload. Read-only to consumers.</summary>
public MxValue Value { get; } public MxValue Value { get; }
/// <summary>Quality code from the OnDataChange event.</summary> /// <summary>Quality code from the OnDataChange event.</summary>
public int Quality { get; } public int Quality { get; }
/// <summary>Source timestamp from the OnDataChange event.</summary> /// <summary>The OnDataChange event's own source timestamp. Read-only to consumers.</summary>
public Timestamp SourceTimestamp { get; } public Timestamp SourceTimestamp { get; }
/// <summary>MxStatusProxy entries from the OnDataChange event.</summary> /// <summary>The OnDataChange event's own MxStatusProxy collection. Read-only to consumers.</summary>
public RepeatedField<MxStatusProxy> Statuses { get; } public RepeatedField<MxStatusProxy> Statuses { get; }
} }
} }
@@ -73,6 +73,19 @@ public sealed class MxAccessWriteCompletionCache
ulong version = entries.TryGetValue(key, out CompletionEntry existing) ulong version = entries.TryGetValue(key, out CompletionEntry existing)
? existing.Version + 1 ? existing.Version + 1
: 1UL; : 1UL;
// Still a defensive copy, deliberately — this is NOT an oversight
// left behind by the borrow that MxAccessValueCache.Set adopted (see
// the ownership contract there). The value cache is handed the whole
// enqueued MxEvent, so the queue's write-once ownership invariant
// covers everything it retains. This method is handed a bare
// RepeatedField whose provenance its signature cannot constrain:
// the production sink does pass an enqueued event's Statuses, but
// callers that build and keep their own rows are equally valid
// against this API, and a borrowed alias would then let a later
// mutation rewrite an already-recorded completion. The write path is
// command-rate, not the per-OnDataChange streaming hot path, so the
// clone costs nothing worth reclaiming.
entries[key] = new CompletionEntry(version, statuses.Clone()); entries[key] = new CompletionEntry(version, statuses.Clone());
} }