Code-review follow-ups on the poll-loop collapse: (1) RetireAsync is fire-and-
forget and does NOT guarantee zero overlap — the retired loop runs until its
in-flight read+tick finish and it observes cancellation, so a device transition
landing in that one-tick window can fire once on both loops (at most ONE
duplicate raise/clear per reconnect, transient + self-correcting; upstream Part
9 conditions dedupe on ConditionId). Documented in both RetireAsync XML docs so
it isn't mistaken for a zero-overlap guarantee. (2) wrap Cts.Dispose() so the
fire-and-forget task has no theoretical unobserved-exception path.
The owning DriverInstanceActor re-subscribes alarms on every Connected
entry (DetachAlarmSource nulls its cached handle on Connected->Reconnecting
without calling UnsubscribeAlarmsAsync), and the driver object + its alarm
projection are reused across every in-place reconnect. Each SubscribeAsync
started a fresh, never-cancelled Task.Run poll loop and added it to _subs,
so N reconnects leaked N concurrent loops all polling the device and all
firing the same raise/clear transitions => duplicate alarm events + CPU/mem
growth.
Mirrors the Galaxy #399 fix (Clear-before-Add) but for live poll loops the
collapse must also CANCEL the superseded loops, not just drop references.
SubscribeAsync now snapshots existing subs under _subsLock, clears _subs,
adds the new sub, starts its loop, then retires each stale sub out-of-band
(RetireAsync: Cancel + await loop + Dispose CTS, fire-and-forget so the new
subscription's return isn't blocked on a poll interval). Snapshot+clear under
the same lock DisposeAsync uses guarantees no double-own / double-dispose.
There is exactly one consumer per driver instance (factory-per-actor), so
retiring all prior subscriptions before starting the new one is faithful.
Regression tests (TDD, fail->pass): subscribe twice then drive one device
raise; assert OnAlarmEvent fires exactly once (was twice with two leaked
loops).
GalaxyDriver's StreamAlarms feed is session-less and survives an in-place
reconnect, so DriverInstanceActor re-subscribed on every Connected re-entry
(after dropping its own cached handle without an Unsubscribe — sync teardown).
The re-subscribe was additive: _alarmSubscriptions.Add grew the list by one
untracked handle per reconnect cycle — a slow unbounded leak. Functionally
harmless (the gate is Count>0 and OnAlarmFeedTransition only reads [0], firing
once regardless), but it accumulated forever.
Fix: SubscribeAlarmsAsync clears the set before adding, collapsing to a single
live handle (under the existing _alarmHandlersLock, atomic w.r.t. the fan-out
reader). There is exactly one consumer per driver instance (factory-per-actor
lifecycle), so replacing the set with the latest handle is faithful. Chosen
over making the actor's sync DetachAlarmSource call UnsubscribeAlarmsAsync
async/fire-and-forget — disproportionate for a minor leak.
Regression test Re_subscribe_collapses_to_a_single_handle_no_accumulation
(TDD-verified: FAILS without the Clear — releasing the latest handle leaves
the feed open because stale handles remain; PASSES with the fix). Galaxy tests
263 pass / 3 skip; Runtime native-alarm 24 pass. Code-reviewed (approved).
Native-alarm delivery through OnAlarmFeedTransition was a black box — there was no way
to answer 'is the gateway feed delivering / is a subscription un-gating it', which is
partly why the missing-SubscribeAlarmsAsync wiring shipped undetected. Add a single
per-transition Debug line (kind, ref, live subscription count, fanout flag). Debug so a
flapping galaxy doesn't flood prod, but available on demand.
Add IGalaxyDataWriter.InvalidateHandleCaches() and call it in
GalaxyDriver.ReopenAsync after RecreateAsync succeeds. Prior to this
fix, GatewayGalaxyDataWriter's _itemHandles and _supervisedHandles
dictionaries survived across reconnects, causing the next write to
skip AddItem and AdviseSupervisory against already-dead handles.
Equipment tags resolved at runtime via FocasEquipmentTagParser were not
seeded in _parsedAddressesByTagName so both ReadAsync and WriteAsync
re-parsed the raw TagConfig JSON address string on every hot-path call.
Promoted the field to ConcurrentDictionary (read + write thread safety)
and introduced ResolveParsedAddress(GetOrAdd) so the first call stores
the parse result and all subsequent calls are a cache hit. Authored tags
seeded at InitializeAsync compile and work unchanged.
A plain MXAccess Write runs with no user login (WriteUserId is typically 0),
and MXAccess only COMMITS such a write when the item is advised in supervisory
mode. Without it the gateway's Write call doesn't throw (the reply looks OK) but
the value never reaches the galaxy. GatewayGalaxyDataWriter now issues
AdviseSupervisory (once per item handle) before each raw Write; SecuredWrite/
VerifiedWrite tags keep their own user-identity path. Live-verified end-to-end:
an authorized write to a Galaxy equipment tag commits and PERSISTS across a
fresh re-subscribe; an anonymous write is denied.
(The sister ScadaBridge driver commits writes the other way — a configured
non-zero WriteUserId + regular Advise; we have no galaxy login, so we use the
supervisory context.)
The net48 sidecar's TcpFrameServer.RunOneConnectionAsync registered the
cancellation token to Stop() only the listener (to unblock a parked
AcceptTcpClientAsync), but never closed the active client. On net48
NetworkStream.ReadAsync ignores the CancellationToken, so while the frame
loop is parked reading an idle connected client, cancelling the token cannot
unblock it — only closing the socket can. RunAsync therefore never returned
on Ctrl-C/service-stop while a connection was open (Program.Main's
RunAsync().GetAwaiter().GetResult() would hang until NSSM force-killed).
Register the cancel to Close() the active client, and convert the resulting
cancel-time read/handshake exception to OperationCanceledException so RunAsync
unwinds cleanly without logging it as a connection failure or counting it
toward MaxConsecutiveFailures.
Caught by the first-ever net48 execution of TcpRoundTripTests on the Windows
VM (these only compile on macOS): SingleActive_SecondClientHelloCompletesOnly
AfterFirstCloses deadlocked in teardown. Full net48 historian suite now green
(122 passed, 0 failed, 2 skipped); all 6 TcpRoundTrip tests pass.
Live verification on a Windows VM surfaced a crash loop: TcpFrameServer.EnsureListening
assigned _listener = new TcpListener(...) BEFORE calling Start(). When Start() throws —
e.g. the port is in a Windows excluded/reserved range (WSAEACCES) or already in use — the
field was left non-null-but-unstarted, so the `if (_listener is not null) return` guard
permanently skipped re-Start() and every subsequent AcceptTcpClientAsync() threw the
misleading InvalidOperationException "Not listening" → 20 failures → exit 2 → NSSM restart
→ loop. Now _listener is assigned only after Start() succeeds, so a transient bind failure
is retried and a permanent one surfaces the real bind error each iteration. Adds a
regression test that forces a bind conflict and asserts the SocketException persists.
All five suppressed advisories are now resolved at baseline/resolved versions,
so every NuGetAuditSuppress is removed repo-wide:
- System.Security.Cryptography.Xml (GHSA-37gx-xxp4-5rgx / GHSA-w3x6-4m5h-cxqf)
-> fixed by the .NET 10 baseline (10.0.6)
- OPCFoundation Opc.Ua.Core (GHSA-h958-fxgg-g7w3) -> fixed at resolved 1.5.378.106
Two were still live and are now patched via direct security pins:
- OpenTelemetry.Api 1.9.0 -> 1.15.3 (GHSA-g94r-2vxg-569j) pinned in Cluster;
Runtime/ControlPlane/AdminUI + tests inherit via project reference
- Tmds.DBus.Protocol 0.20.0 -> 0.21.3 (GHSA-xrw6-gwf8-vvr9) pinned in Client.UI
Also correct the Historian sidecar runtime comments (x86 -> x64, matching the
csproj PlatformTarget). Solution audit: 0 vulnerable packages; full build clean.
The driver/factory/seed use 'GalaxyMxGateway' (legacy 'Galaxy' was retired),
but the AdminUI editor router, GalaxyDriverPage, address picker, identity
dropdown, the Galaxy browser/probe, and DraftValidator still keyed on 'Galaxy'.
Result: the seeded GalaxyMxGateway driver couldn't be edited ('no editor
registered'), UI-created Galaxy drivers wrote a type with no factory, and a
SystemPlatform-bound GalaxyMxGateway driver failed publish validation.
Align all stragglers to GalaxyMxGateway (+ failing-test-first DraftValidator
coverage). ShouldStub's 'Galaxy' legacy safety-net left intact.
Resolves the 12 reported build errors (7 CS0535 sink fakes + 5 CLI CS1587).
Runtime.Tests green (74). NOTE: OpcUaServer.Tests still has pre-existing CS7036
errors from the in-progress Galaxy-tag workstream (Phase7Plan/Phase7CompositionResult
new required params) — separate, test-only, not addressed here.
Imports the freshly-rebuilt ZB.MOM.WW.MxGateway.Client + ZB.MOM.WW.MxGateway.Contracts
nupkgs (0.1.0) from /tmp/mxgw-dist. Replaces the vendored libs/ DLLs and the
pre-restructure MxGateway.* namespaces across the runtime Galaxy driver,
Galaxy.Browser, and their tests.
Key changes:
- nuget-packages/ added as a local feed via NuGet.config; .gitignore exempts it
from the *.nupkg rule so the packages are tracked
- Directory.Packages.props pins both packages at 0.1.0
- 4 csprojs swap <Reference HintPath="libs/...dll"/> for <PackageReference/>
- 36 .cs files renamed `using MxGateway.*` -> `using ZB.MOM.WW.MxGateway.*`
- libs/ removed (vendored DLLs + README.md)
GalaxyBrowseSession rewritten around the new lazy API:
- RootAsync calls GalaxyRepositoryClient.BrowseAsync (returns LazyBrowseNodes)
and caches them by TagName instead of bulk-fetching the whole hierarchy
- ExpandAsync looks up the cached LazyBrowseNode and calls its ExpandAsync,
giving true one-wire-call-per-click instead of in-memory parent/child scan
- _byGobjectId + _hasChildrenSet dropped (LazyBrowseNode carries HasChildrenHint)
- AttributesAsync unchanged (already uses DiscoverHierarchyAsync MaxDepth=0)
Tests: Galaxy.Tests 245/245, Galaxy.Browser.Tests 10/10, AdminUI.Tests 66/66.
Pre-existing 12 solution errors unchanged (test sinks + Cli XML comments).
GalaxyDriverBrowser opens an ad-hoc GalaxyRepositoryClient from the
AdminUI's persisted Galaxy options and hands it to a GalaxyBrowseSession
for the address picker. Mirrors GalaxyDriver.BuildClientOptions field-
for-field so the gateway sees an identical option shape, with API-key
resolution inlined (env:/file:/dev: prefixes) so the Browser project
needn't take a hard reference on Driver.Galaxy.
Connect phase runs under a 30s budget linked to the caller's CT and
includes a TestConnectionAsync call so auth/TLS/DNS failures surface
inside the budget instead of waiting for the first DiscoverHierarchy
round-trip. On any post-Create exception the client is disposed before
the throw propagates.
Refactored GalaxyBrowseSession to take only GalaxyRepositoryClient —
browse never needs MxGatewaySession (that's only for live subscribe/
write paths), and constructing one outside the runtime driver isn't
straightforward. The session now disposes _client in DisposeAsync; the
_session field/parameter is gone.
Browser project (Phase 3) needs to share namespace-stable address encoding
with the runtime driver. Move keeps the same namespace, so existing usages
in OpcUaClientDriver compile unchanged.
ModbusDriverProbe.DriverType was "Modbus" but the AdminUI's
ModbusDriverPage persists DriverInstance.DriverType = "ModbusTcp".
GalaxyDriverProbe used the runtime DriverTypeName constant
("GalaxyMxGateway") but the AdminUI saves "Galaxy". The probe DI
lookup is case-insensitive but not name-insensitive, so Test
Connect would fail to find a probe for these two drivers.
Cheap-and-fast probe: open TCP socket to the configured endpoint,
close immediately. Surfaces SocketError on failure, latency on
success, "timed out" on caller cancel. Sufficient for the AdminUI
Test Connect "can we reach the host?" question. Richer protocol-
level probes (OPC UA session open, FOCAS handshake, gRPC ping)
are a documented follow-up. Each probe registered as
AddSingleton<IDriverProbe, X> in DriverFactoryBootstrap so they
flow through DI into AdminOperationsActor.
Historian.Wonderware returns a clean "TCP probe not applicable"
result because it communicates over a Windows named pipe, not TCP.
Also adds OpcUaClient + Historian.Wonderware.Client project
references to Host.csproj (both were missing from the driver
ItemGroup).
Move WonderwareHistorianClientOptions to a new
Driver.Historian.Wonderware.Client.Contracts sibling project. The record
had no using directives and uses only primitive types (string, TimeSpan)
so the contracts project is dependency-free.
Convert one doc-comment reference:
<see cref="WonderwareHistorianClient"/> → <c>WonderwareHistorianClient</c>
per the approved decision — no compilable usings were present.
The runtime Driver.Historian.Wonderware.Client project gains a
ProjectReference to .Contracts; the .slnx is updated accordingly.
Move GalaxyDriverOptions (and nested records GalaxyGatewayOptions,
GalaxyMxAccessOptions, GalaxyRepositoryOptions, GalaxyReconnectOptions)
from Config/GalaxyDriverOptions.cs into a new Driver.Galaxy.Contracts
sibling project at the contracts root (no Config/ subdirectory). The
existing namespace ZB.MOM.WW.OtOpcUa.Driver.Galaxy.Config is preserved
unchanged — it is a runtime ABI concern and all consumers already import
it via the namespace qualifier.
No doc-comment substitutions required — the only cref in the file
(<see cref="ApiKeySecretRef"/>) is an intra-type parameter reference
that resolves within the contracts project itself.
The options file had no using directives and no NuGet type surface;
the contracts project is dependency-free. The runtime Driver.Galaxy
project gains a ProjectReference to .Contracts; the .slnx is updated
accordingly.
Joseph Doherty