Files
lmxopcua/docs/reviews/2026-10-21/06-drivers-session.md
T

39 KiB
Raw Blame History

Review 06 — Drivers (session slice): Galaxy, OpcUaClient, Mqtt, MTConnect, Sql

Repo: /Users/dohertj2/Desktop/OtOpcUa @ d51d35bd (master). Production code only (src/Drivers/...). Read-only audit; every finding was verified by reading the cited lines (fork-sourced MQTT/MTConnect/SQL findings were spot-checked by the lead against the cited lines). Line numbers are as of this commit. Prefix key: G Galaxy, O OpcUaClient, M MQTT, T MTConnect, S Sql.

Host context that shapes several findings (verified outside the slice):

  • The runtime never disposes a driver instance. DriverInstanceActor.PostStop calls only _driver.ShutdownAsync() (src/Server/ZB.MOM.WW.OtOpcUa.Runtime/Drivers/DriverInstanceActor.cs:1550); DriverHostActor.StopChild only stops the actor (DriverHostActor.cs:3759-3775); grep -rn "is IAsyncDisposable\|as IDisposable" src/Server/ZB.MOM.WW.OtOpcUa.Runtime is empty. The /raw browse picker (src/Core/ZB.MOM.WW.OtOpcUa.Commons/Browsing/DiscoveryDriverBrowser.cs:128-150) builds a throwaway driver per capture and also only calls ShutdownAsync. ShutdownAsync is therefore the only teardown a driver ever gets.
  • The comms-loss watchdog's recovery is ShutdownAsync → InitializeAsync on the SAME driver object, retried every 10 s while Reconnecting (DriverInstanceActor.cs:808-855).

1. Memory / resource leaks

G-1. GalaxyDriver.ShutdownAsync is a no-op — the whole gRPC runtime outlives the driver on every respawn, removal and /raw browse

  • Severity: Critical · Confidence: High
  • Location: src/Drivers/ZB.MOM.WW.OtOpcUa.Driver.Galaxy/GalaxyDriver.cs:735-741 (ShutdownAsync); :1517-1568 (DisposeAsync, where teardown actually lives).
  • Evidence:
    public Task ShutdownAsync(CancellationToken cancellationToken)
    {
        if (_disposed) return Task.CompletedTask;
        _logger.LogInformation("GalaxyDriver {InstanceId} shutting down", _driverInstanceId);
        _health = new DriverHealth(DriverState.Unknown, _health.LastSuccessfulRead, null);
        return Task.CompletedTask;
    }
    
    Everything BuildProductionRuntimeAsync (:390-437) creates — _ownedMxClient (gRPC channel), _ownedMxSession (a SECOND MxGatewayClient + MX session + Register handle, Runtime/GalaxyMxSession.cs:85-87), the EventPump (two background tasks draining StreamEvents), GatewayGalaxyAlarmFeed (background StreamAlarms loop that reconnects forever, Runtime/GatewayGalaxyAlarmFeed.cs:102-148), ReconnectSupervisor, PerPlatformProbeWatcher, DeployWatcher (WatchDeployEvents stream loop) and _ownedRepositoryClient — is released only in DisposeAsync, which nothing in the host calls.
  • Failure scenario: (a) a config change is a respawn (new GalaxyDriver from the factory; the old child only gets ShutdownAsync) → the old instance keeps its MX session registered on mxaccessgw, keeps draining StreamEvents/StreamAlarms, keeps the deploy-watch stream open and its supervisor loop alive — permanently, one set per deploy. (b) Every AdminUI /raw browse of a Galaxy device (DiscoveryDriverBrowser.CaptureAsync → InitializeAsync → DiscoverAsync → ShutdownAsync) leaks a full runtime: two gRPC channels, an MX session, both streams, a 50k-slot bounded channel. The gateway accumulates live sessions and STA-pumped advises; OtOpcUa accumulates threads and channels.
  • Suggested fix: make ShutdownAsync do the device-state half of DisposeAsync (stop pump/alarm feed/deploy watcher/probe watcher/supervisor, dispose session + clients, null the seams so a later InitializeAsync rebuilds) and keep DisposeAsync as the idempotent final teardown. The #630 contract already puts device state under InitializeAsync/ShutdownAsync for the other eleven drivers.

G-2. Every InitializeAsync on a live GalaxyDriver creates a new DeployWatcher without stopping the previous one

  • Severity: High · Confidence: High
  • Location: GalaxyDriver.cs:631-654 (StartDeployWatcherAsync), called unconditionally from InitializeAsync :377.
  • Evidence:
    var source = new GatewayGalaxyDeployWatchSource(_ownedRepositoryClient);
    _deployWatcher = new DeployWatcher(source, _logger);          // previous watcher dropped, still running
    _deployWatcher.OnRediscoveryNeeded += (_, args) => OnRediscoveryNeeded?.Invoke(this, args);
    var startTask = _deployWatcher.StartAsync(CancellationToken.None);
    
    InitializeAsync (:357-382) skips BuildProductionRuntimeAsync on a second call (seams non-null) but always runs this; DeployWatcher.RunLoopAsync (Browse/DeployWatcher.cs:133-235) runs until cancelled.
  • Failure scenario: the watchdog's recovery is ShutdownAsync (no-op, G-1) + InitializeAsync. While mxaccessgw is down the supervisor reports Degraded with a frozen LastSuccessfulRead (G-11), so the host trips every ~90 s and each trip spawns another watch-stream loop with its own 1–30 s backoff. When the gateway returns, N watchers each raise OnRediscoveryNeeded per real deploy and N streams stay open for the process lifetime.
  • Suggested fix: stop/dispose the existing watcher at the top of StartDeployWatcherAsync (or early-return when one exists), and stop it in ShutdownAsync (G-1).

O-1. OpcUaClient SubscribeAsync leaves a half-created subscription on the session when CreateItemsAsync (or CreateAsync) throws — one per host retry

  • Severity: Medium · Confidence: High
  • Location: src/Drivers/ZB.MOM.WW.OtOpcUa.Driver.OpcUaClient/OpcUaClientDriver.cs:1781-1826.
  • Evidence:
    session.AddSubscription(subscription);
    await subscription.CreateAsync(cancellationToken).ConfigureAwait(false);
    ...
    item.Notification += notifHandler;
    ...
    await subscription.CreateItemsAsync(cancellationToken).ConfigureAwait(false);
    _subscriptions[id] = new RemoteSubscription(subscription, handle, itemHandlers);
    
    No catch: on a throw the SDK Subscription stays in session.Subscriptions (created server-side if CreateAsync succeeded) with every MonitoredItem.Notification closure still attached, untracked by _subscriptions, so neither UnsubscribeAsync nor ShutdownAsync deletes it. The alarm path has exactly this cleanup (:2204-2226: "a half-built subscription left on the session would accumulate one per retry"); the data path does not.
  • Failure scenario: a server that accepts CreateSubscription but rejects CreateMonitoredItems (e.g. BadTooManyMonitoredItems, transient timeout) makes DriverInstanceActor's health-tick reconcile retry every tick; each retry adds a live, publishing subscription on the upstream server until the next re-initialise disposes the session — and servers with per-session caps then refuse the real subscribe.
  • Suggested fix: wrap :1789-1824 in try/catch mirroring the alarm path — detach handlers, bounded session.RemoveSubscriptionAsync(subscription), rethrow.

S-1. SQL key-value and where-pair plans have no row cap; an ambiguous selector materialises every matching row on every poll

  • Severity: Medium · Confidence: High
  • Location: src/Drivers/ZB.MOM.WW.OtOpcUa.Driver.Sql/SqlGroupPlanner.cs:233-238 (key-value SELECT … WHERE key IN (…), no limit), :271-281 (where-pair … WHERE col = @w, no limit); src/Drivers/ZB.MOM.WW.OtOpcUa.Driver.Sql/SqlPollReader.cs:445-452:
    var rows = new List<object?[]>();
    while (await reader.ReadAsync(cancellationToken).ConfigureAwait(false)) { ... rows.Add(row); }
    
    SelectWideRow (:552-560) explicitly accepts that "an ambiguous selector materialises every matching row for one poll".
  • Failure scenario: a tag authored against a log/history table (keyColumn=TagName on a 50M-row samples table, or whereColumn=Line on an events table) pulls every matching row into memory every poll (default 5 s). Memory spikes to the match-set size, GC churn, and commandTimeout is the only brake; nothing refuses this at deploy time and the only signal is a once-per-minute contract warning.
  • Suggested fix: cap the read loop (stop after N rows, publish a configuration-error status and warn when the cap is hit), or emit dialect TOP/LIMIT N+1 so ambiguity is still observable without unbounded materialisation.

M-1. A failed MQTT SUBSCRIBE (no connection drop) returns a live handle and drops the topic from the re-subscribe set — tags stay dark behind a Healthy connection

  • Severity: Medium · Confidence: High (state-kept-wrong after failure; same family as a leak)
  • Location: src/Drivers/ZB.MOM.WW.OtOpcUa.Driver.Mqtt/MqttSubscriptionManager.cs:667-684 (transport throw → DegradeTopic → return 0), :719-721 (_subscribedTopics.TryRemove(topic, out _)), :417-418 (SubscribeAsync returns the handle regardless), :489-499 (OnReconnectedAsync re-subscribes _subscribedTopics only).
  • Failure scenario (plain mode): the SUBSCRIBE overruns its deadline or the broker transiently refuses a filter. The topic leaves _subscribedTopics, its tags go BadCommunicationError, but SubscribeAsync still returns a handle so DriverInstanceActor considers the subscription live and never re-subscribes; the next reconnect re-SUBSCRIBEs only _subscribedTopics, which no longer holds it. Connection state stays Connected/Healthy and LastMessageUtc advances from other topics, so the watchdog never trips. Dark until a redeploy.
  • Suggested fix: keep failed filters in a pending set that OnReconnectedAsync/the next SubscribeAsync retry, or throw from SubscribeAsync on a transport failure so the actor's reconcile re-subscribes.

T-1. MTConnect: a late (superseded/abandoned) pump publishes its stale cursor onto the NEW session — the doc says it is dropped, the code has no generation guard

  • Severity: Low · Confidence: Medium
  • Location: src/Drivers/ZB.MOM.WW.OtOpcUa.Driver.MTConnect/MTConnectDriver.cs:2011-2020 (pump finally { PublishAgentCursor(instanceId, cursor); }, comment "dropped if a lifecycle change has already installed a newer session"), :2491-2508 (PublishAgentCursor only early-returns on session is null or an identical (InstanceId, NextSequence); it CAS-writes onto whatever session is installed — nothing checks _sessionGeneration), :1789-1811, :2914-2922.
  • Failure scenario: a pump retired into _supersededPumps or abandoned after TeardownWait finishes after StartCoreAsync installed the new session; the next pump start reads the stale InstanceId, treats the first chunk as an Agent restart → ObservationIndex.Clear() + Bad flicker on every subscribed tag + a needless re-baseline (:2061-2081). Bounded (one flicker), no leak.
  • Suggested fix: stamp the pump with _sessionGeneration and have PublishAgentCursor refuse when it differs (or compare ReferenceEquals(session.Client, client)).

Low-severity leak nits (all High confidence)

  • G-3 ReconnectSupervisor.ReportTransportFailure (Runtime/ReconnectSupervisor.cs:127-141) allocates a new _loopCts per recovery cycle and disposes only the last one in Dispose(). Bounded by outage count.
  • O-2 OpcUaClientDriver.InitializeAsync (:521-568): the catch closes Session (the property, still null) rather than the local session, so a throw in :544-556 (AttachDiagnostics/NamespaceMap.FromSession/RecordSkippedTags) orphans an activated session with its keep-alive handler until GC. Rare.
  • O-3 SdkTelemetry.ForLogger (OpcUaClientDriver.cs:85, SdkTelemetry.cs:28-40) creates an ILoggerFactory per driver construction, released only in Dispose/DisposeAsync (:3365, :3395) — which the host never calls; _gate and _sessionWorkCts likewise. One small object set per respawn, unbounded count.
  • M-2 LastValueCache (MqttSubscriptionManager.cs:398, Sparkplug/SparkplugIngestor.cs:553, LastValueCache.cs:42-46) gains an entry per non-authored reference ever subscribed and Register never prunes Values; bounded by the union of every RawPath authored across redeploys.
  • M-3 A failed initial connect leaves the new MqttConnection/IMqttClient undisposed (MqttDriver.cs:967-986, catch at :1029 rethrows without disposing; contrast EstablishAsync at :1011-1019). Only one stale connection is held at a time (_transport is replaced each attempt), no socket.
  • S-2 A timed-out SQL group keeps its connection + maxConcurrentGroups slot until the provider returns (SqlPollReader.cs:275-335, :345-357); documented and bounded (default 4), but the bound is per driver, not per poll — pool sizing should know.
  • S-3 SqlPollReader._gate is never disposed and a new reader is built per ApplyBinding (SqlPollReader.cs:53-59, SqlDriver.cs:264-273); managed-only today.

2. Unimplemented / stubbed

G-4. Galaxy per-platform ScanState host probing is structurally dead: the probe watcher's handles are never registered in SubscriptionRegistry, so EventPump drops every probe event

  • Severity: High · Confidence: High
  • Location: Health/PerPlatformProbeWatcher.cs:125-150 (subscribes via _subscriber.SubscribeBulkAsync, stores handles only in its own _itemHandlesByPlatform), Runtime/EventPump.cs:314-317, GalaxyDriver.cs:1465-1489.
  • Evidence:
    // EventPump.DispatchDataChange
    var subscribers = _registry.ResolveSubscribers(ev.ItemHandle);
    if (subscribers.Count == 0) return; // stale event after unsubscribe — drop quietly
    
    SubscriptionRegistry.Register is called only from ReadViaSubscribeOnceAsync (:959) and SubscribeAsync (:1110) — never for the watcher's handles. OnPumpDataChange (:1480-1488), the only caller of OnProbeValueChanged, is reached only via the pump's fan-out, so a *.ScanState event never gets there. The only test (tests/.../Health/PerPlatformProbeWatcherTests.cs) drives OnProbeValueChanged directly.
  • Also: after a reconnect (ReopenAsync → GalaxyMxSession.RecreateAsync) the watcher's handles are dead and nothing re-subscribes them — SyncPlatformsAsync is called only from DiscoverAsync (:800); ReplayAsync rebinds only _subscriptions.
  • Failure scenario: IHostConnectivityProbe.GetHostStatuses() for Galaxy only ever carries the synthetic transport entry; per-$WinPlatform/$AppEngine Running/Stopped on /hosts never updates. Each discovery also leaves N ScanState items advised on the gateway for nothing (released only by Dispose, G-1).
  • Suggested fix: register the probe handles in the registry under a reserved subscription id (and Rebind them in ResubscribeAllAsync), or route raw MxEvents to the watcher by item handle before the registry lookup.

M-4. sparkplug.birthObservationWindowSeconds is authored, range-checked and persisted by the AdminUI but consumed by nothing

  • Severity: Medium · Confidence: High
  • Location: src/Drivers/ZB.MOM.WW.OtOpcUa.Driver.Mqtt.Contracts/MqttDriverOptions.cs:222-227 (BirthObservationWindowSeconds = 15, documented as the browse/discovery birth-collection window). grep -rn BirthObservationWindowSeconds src finds no reader outside Contracts and the AdminUI form (MqttDriverForm.razor:222, MqttDriverFormModel.cs:168,274,376); MqttDriverBrowser.OpenBudget uses ConnectTimeoutSeconds clamped 5–30 s (MqttDriverBrowser.cs:188-189).
  • Failure scenario: an operator raising the window for slow births sees nothing change; discovery stability is governed by the host's UntilStable loop and the browse window by ConnectTimeoutSeconds.
  • Suggested fix: feed it into MqttDriverBrowser.OpenBudget/the UntilStable cadence, or remove it from the contract and form.

G-5. Repository.DiscoverPageSize has an AdminUI field and is parsed by the factory, but nothing consumes it

  • Severity: Low · Confidence: High
  • Location: GalaxyDriverFactoryExtensions.cs:94, Contracts/GalaxyDriverOptions.cs:114, Browse/GatewayGalaxyHierarchySource.cs:24-25 (_client.DiscoverHierarchyAsync(cancellationToken) — no page size), AdminUI GalaxyDriverForm.razor:94.
  • Suggested fix: thread it into DiscoverHierarchyAsync or drop the field.

G-6 / O-4. ProbeTimeoutSeconds exists on both GalaxyDriverOptions and OpcUaClientDriverOptions and is consumed nowhere

  • Severity: Low · Confidence: High
  • Location: GalaxyDriver.cs:732 (only use: equality compare); OpcUaClientDriverOptions.ProbeTimeoutSeconds has 0 references in driver, browser, session or any host project. Both probes take the caller's timeout argument instead.

O-5. OpcUaClient AdminUI browse session's AttributesAsync is a stub; Certificate auth is refused "in v1"

  • Severity: Low · Confidence: High
  • Location: OpcUaClient.Browser/OpcUaClientBrowseSession.cs:56-61 (Array.Empty<AttributeInfo>()), OpcUaClientDriverBrowser.cs:61-63.
  • Impact: no DataType/access info in the live picker side-panel (the /raw browse-commit via DiscoveryDriverBrowser does enrich); a certificate-authenticated upstream cannot be browsed from the AdminUI.

M-5. Sparkplug array metrics are refused ("not supported in v1") yet the browser still offers them as bindable

  • Severity: Low · Confidence: High (documented decision)
  • Location: SparkplugIngestor.cs:1274-1285 (if (binding.DataType.IsSparkplugArray()) { WarnOnce(...); return; }; DataSet/Template/PropertySet at :1287-1302); MqttBrowseSession.cs:387-392 lists the leaf with IsArray: true.
  • Impact: an operator can commit a tag the ingestor never feeds — warned once, then BadWaitingForInitialData forever.
  • Suggested fix: mark array leaves not-bindable in DescribeMetric as mapped is null already is.

T-2. MTConnect Agent-restart detection lives only on the /sample pump; ReadAsync discards instanceId

  • Severity: Low · Confidence: High
  • Location: MTConnectDriver.cs:716-738 (ReadAsync ignores current.InstanceId), :2061-2081 (only chunk.InstanceId != instanceId check), :1687-1711 (AnnounceAgentRestart).
  • Impact: a driver with no active subscription (browse-only) keeps a cached /probe model across an Agent restart and OnRediscoveryNeeded never fires. Production always subscribes, so browse-only edge.

S-4. SQL: SqlTagModel.Query is on the enum but unsupported end-to-end; MySql/Odbc providers are reserved; allowWrites is parsed and ignored

  • Severity: Low · Confidence: High (all documented deferrals that fail loudly or warn once)
  • Location: Driver.Sql.Contracts/SqlEquipmentTagParser.cs:75-76, SqlGroupPlanner.cs:336-339; SqlDriverFactoryExtensions.cs:214-225, SqlDriverProbe.cs:196-211, SqlDriverBrowser.cs:135-141; SqlDriverFactoryExtensions.cs:151-161 + SqlDriver.cs:30-31 (no IWritable).
  • Note: a model: "Query" tag is silently skipped (ledger + BadNodeIdUnknown) rather than refused at the deploy gate.

Dead parsed surface (Low, High confidence)

  • T-3 MTConnectObservation.SampleRate parsed per observation (MTConnectStreamsParser.cs:424-428) and never read; MTConnectTagDefinition.MtType/MtSubType/Units carried but only MtCategory is read (MTConnectDriver.cs:3493).
  • S-5 IS_NULLABLE projected by every dialect's ListColumnsSql (SqlServerDialect.cs:107-109, PostgresDialect.cs:141-144, OracleDialect.cs:184-187) and never read by SqlBrowseSession.cs:359-371 / SqlCatalogLoader.cs:117-124.

3. Performance limiters

T-4. Every MTConnect ReadAsync batch fetches + parses the whole device /current and builds a fresh FrozenDictionary index, even while the pump holds a live one

  • Severity: Medium · Confidence: High
  • Location: MTConnectDriver.cs:718-732; MTConnectObservationIndex.cs:149-164 (ToFrozenDictionary per construction).
  • Evidence: var current = await BoundedAsync(client.CurrentAsync, …); var snapshot = new MTConnectObservationIndex(binding.IndexTags); snapshot.Apply(current, baseline: true);
  • Failure scenario: OPC UA clients polling with Read (no subscription) against a 2–5k DataItem Agent cost one full XML transfer + parse + index build per Read call regardless of batch size; a few such clients make the Agent the bottleneck. The single-writer rationale (:654-671) forbids writing the shared index, not reading it.
  • Suggested fix: read through ObservationIndex while _pumpTask is live and !_streamDegraded, keep /current as the fallback; or prebuild binding.IndexTags as a FrozenDictionary on TagBinding.

T-5. MTConnect subscription throughput is hard-capped at SampleCount / SampleIntervalMs (default 1000 obs/s) with no lag detection or metric

  • Severity: Medium · Confidence: Medium
  • Location: MTConnectAgentClient.cs:154-156 (&interval=…&count=…), MTConnectDriver.cs:2083-2091 (gap → re-baseline), :1961-1978.
  • Failure scenario: a machine producing more observations/s than count/interval makes the cursor fall steadily behind the Agent ring buffer; every buffer/rate seconds the driver warns, re-baselines via /current (losing intermediate transitions) and reopens — forever, every delivered value backlog-stale. Nothing surfaces the lag and nothing adapts count.
  • Suggested fix: counter/Warning when chunks repeatedly carry exactly SampleCount observations or expected - chunk.FirstSequence shrinks toward zero; document the cap next to SampleCount.

G-7. GalaxyDriver.DiscoverAsync downloads the full Galaxy hierarchy twice per discovery

  • Severity: Medium · Confidence: High
  • Location: GalaxyDriver.cs:787-800: discoverer.DiscoverAsync(...) (which calls _source.GetHierarchyAsync, Browse/GalaxyDiscoverer.cs:68) then await source.GetHierarchyAsync(cancellationToken) again just to select platforms.
  • Failure scenario: two full DiscoverHierarchyAsync sweeps (every object + attribute) per browse/deploy on a large galaxy — and the second feeds the dead probe path (G-4).
  • Suggested fix: have GalaxyDiscoverer expose the hierarchy it fetched.

O-6. One SemaphoreSlim _gate serialises OpcUaClient reads, writes, subscribes, acks AND the full recursive discovery browse

  • Severity: Medium · Confidence: High
  • Location: OpcUaClientDriver.cs:1213 (Read), :1302 (Write), :1436-1466 (DiscoverAsync holds the gate across BrowseRecursiveAsync + enrichment), :1781, :2195, :2301.
  • Failure scenario: a stalled write (up to _options.Timeout) blocks every read and subscription create on that driver; recovery/reconcile reads queue behind it. Discovery on the runtime instance would block the data plane for the browse duration (latent today — discovery runs on the throwaway DiscoveryDriverBrowser instance).
  • Suggested fix: separate read/write gating from subscription management; don't hold the gate across the whole recursive browse.

S-6. SQL per-poll warning spam on a persistently failing or frozen group is not rate-limited

  • Severity: Low · Confidence: High
  • Location: SqlPollReader.cs:345-357 (TimedOut warns every poll per group), :411-419 (QueryAsync warns with the full plan.SqlText every poll per failed group); only contract warnings go through ShouldWarnAboutContract() (:714-721).
  • Failure scenario: a dropped table or paused server with 20 groups at 1 s = 20 warnings/s with SQL text, indefinitely.
  • Suggested fix: reuse the rate-limiter per plan, or log once per state transition.

S-7. SQL query plans are re-planned and the slot dictionary re-allocated on every poll

  • Severity: Low · Confidence: Medium
  • Location: SqlPollReader.cs:167-182 (Dictionary<SqlTagDefinition, List<int>> keyed by a 12-field positional record, rebuilt per ReadAsync), :206 (SqlGroupPlanner.Plan per poll) vs SqlQueryPlan.cs:68-70 ("safe to cache and reuse across polls" — nothing caches).
  • Suggested fix: cache plans per subscription reference list, invalidated on ApplyCatalogGateAsync swap.

Serial write paths (Low, High confidence)

  • G-8 Galaxy writes are strictly serial, three round-trips on first touch (Runtime/GatewayGalaxyDataWriter.cs:124-131, :152-183: AddItem → AdviseSupervisory → Write); acks serial by design (GalaxyDriver.cs:1364-1381).
  • M-8 MQTT WriteAsync publishes serially, one awaited QoS 1 round-trip per request bounded by ConnectTimeoutSeconds (MqttDriver.cs:695-749, MqttConnection.cs:1172) — 50 writes × up to 15 s on a stalled broker.
  • T-6 MTConnect writes are one POST per slot (MTConnectDriver.cs:804-878) though the Agent PUT surface accepts several id=value pairs.

Other Low perf items (High confidence)

  • M-6 Wildcard-authored MQTT topics are matched linearly against every inbound message on MQTTnet's dispatcher thread (MqttSubscriptionManager.cs:586-592); concrete topics are a dictionary lookup. Fine for hundreds of wildcard tags, not thousands.
  • M-7 TryDecodeUtf8 allocates new UTF8Encoding(false, true) per Raw/Scalar delivery (MqttSubscriptionManager.cs:1374); MqttBrowseSession.cs:150 already uses a static.
  • T-7 The MTConnect connectivity probe parses the full /probe device model every 5 s and discards it (MTConnectDriver.cs:1496-1504, MTConnectAgentClient.cs:169-180); documented trade-off (:1456-1462). ResponseHeadersRead + status check would do.
  • O-7 OnDataChange is invoked under the driver-wide _deliveryLock (OpcUaClientDriver.cs:368-380, :1888-1912) — deliberate; acceptable only while every subscriber just Tells.

4. Other hazards

G-9. The comms-loss watchdog's re-init does nothing for Galaxy (no teardown, no rebuild); recovery rests entirely on the in-driver supervisor

  • Severity: Medium · Confidence: High
  • Location: GalaxyDriver.cs:366-369 (if (_subscriber is null && _dataWriter is null && _hierarchySource is null) BuildProductionRuntimeAsync), :735-741.
  • Evidence: after the first init the seams are non-null, so a watchdog-driven ShutdownAsync+InitializeAsync neither tears down nor reconnects — it sets _health = Healthy (:381, masked by GetHealth() preferring _supervisor.IsDegraded) and spawns another DeployWatcher (G-2). If the supervisor loop ends with a dead session (e.g. ReopenAsync returning early on _ownedMxSession is null, :448, then Replaying→Healthy), the host has no lever short of a respawn.
  • Suggested fix: as G-1 — make ShutdownAsync release the runtime so the host's re-init genuinely rebuilds.

O-8. OpcUaClient AcknowledgeAsync swallows non-cancellation transport failures, so the host reports the Part 9 ack as applied

  • Severity: Medium · Confidence: High
  • Location: OpcUaClientDriver.cs:2384-2391:
    catch (Exception ex)
    {
        _logger.LogWarning(ex, "...AcknowledgeAsync failed; acknowledgements may not have been applied.");
    }
    
    A ServiceResultException from session.CallAsync (faulted session, closed channel) returns normally; the local condition is acked while upstream still shows it unacknowledged. The next ConditionRefresh dedup will re-report it as unacked, but the operator received success.
  • Suggested fix: rethrow (or return per-ack failures) so the OPC UA Acknowledge call fails with the transport status.

G-11. Galaxy LastSuccessfulRead never advances on data — stamped only at init and on supervisor Healthy

  • Severity: Low · Confidence: High
  • Location: GalaxyDriver.cs:381, 593-597, 661, 739; no write from OnPumpDataChange/ReadViaSubscribeOnceAsync.
  • Failure scenario: the watchdog's conjunction degenerates to "non-Healthy for 90 s" for Galaxy, so any supervisor Degraded lasting 90 s trips the host re-init even while values still flow on the old stream; with G-2 that is one leaked DeployWatcher per 90 s.
  • Suggested fix: stamp (throttled) from OnPumpDataChange, as OpcUaClient's RecordLivenessEvidence does.

G-10. EventPump dispatch loop dying leaves the read loop alive and health Healthy — clean termination evading the watchdog

  • Severity: Low · Confidence: Medium
  • Location: Runtime/EventPump.cs:276-295.
  • Evidence: DispatchLoopAsync catches any exception, logs "events past this point will be lost until restart" and exits; RunAsync keeps draining into the bounded channel until it fills, then only counts drops. Nothing calls NotifyStreamFault; GetHealth() stays Healthy and (G-11) the stamp never moves anyway. Dispatch isolates handler exceptions, so the remaining throw sites are StatusCodeMap.FromMxStatus/ToSnapshot — hence Medium confidence on reachability.
  • Suggested fix: treat a dispatch-loop exit while not cancelled as a stream fault, as the read loop does for a server-completed stream (:173-188).

T-8. MTConnect logs and surfaces the raw AgentUri (which may carry user:pass@) while its own probe deliberately redacts it

  • Severity: Low · Confidence: Medium
  • Location: MTConnectDriver.cs:1396 (HostName => _options.AgentUri), log lines at :852, 1215, 2064, 2086, 2136, 2757, 2790; contrast MTConnectDriverProbe.cs:227-243, 251-256 (RedactedDisplay, "ex.Message is an open door for a credential leak").
  • Suggested fix: compute a redacted display URI once at StartCoreAsync and log/expose that.

S-8. SQL factory quotes the JSON parser message on a malformed config; the probe and browser withhold it for credential hygiene

  • Severity: Low · Confidence: Medium
  • Location: SqlDriverFactoryExtensions.cs:1002-1006 ($"... is not valid JSON: {ex.Message}") vs SqlDriverProbe.cs:84-95, SqlDriverBrowser.cs:292-303 (message withheld "because the malformed text may carry a connection string"). The deployed config should carry only connectionStringRef, so this bites only on a pasted literal — but the message then reaches InitializeFailed → health LastError → /hosts.

O-9. OpcUaClient ShutdownAsync can take N×5 s against a half-dead upstream and blocks the actor's PostStop

  • Severity: Low · Confidence: High
  • Location: OpcUaClientDriver.cs:1121-1135 (serial best-effort DeleteAsync per subscription, each bounded by 5 s, skipped only when _sessionLost == 1), DriverInstanceActor.cs:1550 (ShutdownAsync(...).GetAwaiter().GetResult()).
  • Suggested fix: one bounded budget for the whole teardown, or delete concurrently.

Low hazards, documented / bounded (High confidence)

  • M-10 MqttDriver.ShutdownAsync/DisposeAsync fall back to an ungated TeardownAsync after ConnectTimeoutSeconds (MqttDriver.cs:402-409, :929-935); the post-connect disposed re-check (:993-997) covers DisposeAsync only, so a wedged InitializeCoreAsync completing after a timed-out ShutdownAsync can publish one stray live connection — the actor's own Shutdown→Initialize sequence tears it down next time (bounded to one).
  • M-11 MqttSubscriptionManager.EstablishAsync with no transport records topics as subscribed and returns success (:646-665); unreachable today (AttachTo always precedes connect, MqttDriver.cs:972-979) but a silent "subscribed, receives nothing" if the ordering ever changes.
  • O-10 ConnectionLost raised before the host processed InitializeSucceeded is swallowed — documented known gap (OpcUaClientDriver.cs:140-145).
  • S-9 A stale SQL poll outliving the 5 s dispose wait (operationTimeout 15 s) can call ObservePollOutcome during the next InitializeAsync; UpdateHealthUnlessFaulted guards only Faulted, so a late Good read can briefly flip Initializing→Healthy (cosmetic; next WriteHealth overrides).
  • S-10 The SQL catalog gate rejects rather than reloads on schema drift (SqlDriver.cs:929-972, :898-901): a column added after init stays BadNodeIdUnknown until a reinit. Deliberate (#523 no-I/O-on-healthy rule); listed so operators know the surface is the skip ledger.

5. Checked and fine

Galaxy

  • ReadViaSubscribeOnceAsync (GalaxyDriver.cs:856-1009): subscribe-once reads are the only MxAccess read primitive; handles unsubscribed in finally; internal 30 s deadline covers CancellationToken.None; a SubscribeBulk completing after the deadline is unsubscribed by continuation.
  • SubscriptionRegistry prunes the reverse map on Remove/Rebind; nothing grows past the live subscription set.
  • EventPump channel is bounded (50 000 default, configurable) with explicit TryWrite + EventsDropped; a server-completed stream is treated as a fault (EventPump.cs:173-188). Replay-gap handling is unreachable by design (afterWorkerSequence: 0, GatewayGalaxySubscriber.cs:161) and documented.
  • SubscribeAlarmsAsync collapses to a single handle (:1310-1322); the per-reconnect handle growth it describes is closed.
  • GatewayGalaxyDataWriter handle/advise caches are cleared on reconnect (InvalidateHandleCaches, GalaxyDriver.cs:453) and bounded by the written tag set; the fail-closed advise path forgets a failed handle.
  • GalaxyBrowseSession/GalaxyDriverBrowser, GalaxyDriverProbe: client ownership transfers into the session and is disposed on DisposeAsync/finally; open path disposes on failure.

OpcUaClient

  • ReleaseSessionAsync (:1055-1095) detaches keep-alive + diagnostics handlers, bounds CloseAsync by 5 s, skips the wire close when the session is known lost, and re-arms every outage latch; InitializeAsync calls it first, so a re-init cannot orphan the previous session.
  • Comms-loss contract (#527/#670/#676): bad keep-alive → Degraded with the stamp preserved; good keep-alives/notifications advance it only while no subscription is latched lost; definitive statuses raise ConnectionLost once per session; PublishCommsLoss fans BadNoCommunication with value retained; recovery dedup is (value, status) (IsReconcileRepair).
  • _lastValues/_lastGoodSourceTicks/_deliverySeq/_awaitingRecovery/_replayGuard are keyed by authored RawPath and cleared in ShutdownAsync; bounded even though UnsubscribeAsync does not prune them.
  • CancelSessionWork deliberately does not dispose the old CTS (documented race with the keep-alive thread); no timer attached.
  • Discovery releases continuation points on cancel (:1533-1550); MaxDiscoveredNodes/MaxBrowseDepth bound the browse. Browser session/OpcUaClientDriverBrowser/probe dispose on every path.

MQTT

  • Teardown-before-rebuild still holds: InitializeCoreAsync calls the idempotent TeardownAsync() first (MqttDriver.cs:955), which reaches all three disposable fields — _primaryHost, _hostProbeCts, _connection (:1381-1405); no new disposable field escaped it.
  • MqttConnection reconnect loop does not leak per attempt: one IMqttClient created once (:718-724), per-attempt CTSs using-scoped (:689-693), single supervisor task (??=, :909) awaited bounded on dispose (:498-509).
  • Sparkplug caches are bounded by configuration, not wire: unauthored edge nodes dropped before decode (SparkplugIngestor.cs:736-745); Births, _trackers, _lastRebirthTicks, _nodeBirthSeen, _metadataRebirths pruned to authored nodes on Register (:421-454) and cleared on session reset; NDEATH/DDEATH evict (:1076, :1091); _birthSignatures pruned to _authoredScopes (MqttDriver.cs:1221-1227); _warned capped at 1024 (:1912-1922); _pendingRebirths self-prunes (:1803-1810). Rebirth storms gated (10 s per-node debounce, 3-attempt metadata cap, signature gate on rediscovery).
  • Comms-loss contract: GetHealth() derives state from MqttConnection.State and LastSuccessfulRead from LastMessageUtc (MqttDriver.cs:421-440); ConnectionLost fires on the Connected→Reconnecting edge (MqttConnection.cs:976-992); both ingest paths stale to BadNoCommunication (MqttSubscriptionManager.cs:269-282, SparkplugIngestor.cs:366-379); plain-mode suppression keyed on (status, value) (:824-828); every (re)subscribe re-pushes cached snapshots.
  • Inbound handling runs synchronously on MQTTnet's dispatcher with one JSON parse per message shared across matched tags; back-pressure is MQTTnet's per-client serialisation; the only handler Tells.
  • Browse session: atomic node-cap reservation, bounded payload inspection (512 B / 1 MiB birth), client disposed on DisposeAsync and on open failure. Primary host: separate client id, Will + explicit OFFLINE, claim backoff 2→30 s, connection disposed when ONLINE publish fails.

MTConnect

  • Comms-loss contract: stream failure → Degrade(error) preserving LastSuccessfulRead (:3152-3159); every chunk incl. heartbeats → PublishHealthy(now) (:2102-2103); a quiet enumeration end under a live token is reported as MTConnectStreamEndedException/Transient, never idle (:2106-2117, MTConnectAgentClient.cs:237-243) — no clean-termination evasion; /current and /sample own separate degraded flags.
  • UNAVAILABLE → BadNoCommunication, value null (MTConnectObservationIndex.cs:421-425); conditions stay active with quality-only Bad (MTConnectConditionAlarmMapper.cs:90-97), as the rules file specifies.
  • instanceId/sequence: restart checked before gap (:2055-2081); gap and OUT_OF_RANGE both re-baseline (:2127-2141); backoff resets only when the stream delivered (:1975-1990).
  • Re-init without teardown (#630): InitializeAsync stops pump + probe and disposes the old client before building (:565-574); a client built in a failed attempt is disposed (:2783). MTConnectAgentClient disposed on every teardown path; HttpResponseMessage/streams using/await using (MTConnectAgentClient.cs:175-191, 208-214); heartbeat watchdog bounds every body read, buffer capped at 32 MiB, DTD prohibited.
  • _supersededPumps is pruned (:1742-1754); index/tracker/told-record maps are bounded by the Agent's DataItem count. Streams parser is forward-only XmlReader with a single-string fast path; no JSON on the hot path.

SQL

  • Every DbConnection/DbCommand/DbDataReader path uses await using (SqlPollReader.cs:371-405, SqlDriver.cs:982-989, :1086-1098, SqlCatalogLoader.cs:153-203, SqlBrowseSession.cs:396-404); the browser disposes on open failure and hands ownership to the session otherwise.
  • CTS/slot ownership on timeout paths is transferred to the work task and disposed in its finally (SqlPollReader.cs:282-316, SqlDriver.cs:1042-1060); AbandonedTask.Detach observes faults.
  • Connection string never logged: only Endpoint is rendered (SqlDriver.cs:1114-1128); health/host messages use exception type names; browser redacts and drops inner exceptions.
  • SQL injection surface closed: values bound everywhere; identifiers rewritten to catalog spellings by the allow-list gate before quoting (SqlCatalogGate.cs:142-151) then QuoteIdentifier with doubling + control-char rejection in all three dialects; Oracle BindByName set at every site; >2-part names rejected.
  • Comms-loss contract: ObservePollOutcome (SqlDriver.cs:606-675) advances the stamp on any "database answered" code and degrades on timeouts; all-BadTimeout freezes it → watchdog → ShutdownAsync (_poll.DisposeAsync, 5 s cap) → InitializeAsync → re-subscribe; a query-failure-only outage is Degraded with the clock advancing, so no reconnect loop. Groups run in parallel under Task.WhenAll bounded by maxConcurrentGroups; key-value groups chunk at dialect bind caps; CommandTimeout floored at 1 s; factory enforces operationTimeout > commandTimeout.