ad2b5c4044
Claude-Session: https://claude.ai/code/session_01Ub9hGmYLQvn9XETzw7CMsT
39 KiB
39 KiB
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.PostStopcalls only_driver.ShutdownAsync()(src/Server/ZB.MOM.WW.OtOpcUa.Runtime/Drivers/DriverInstanceActor.cs:1550);DriverHostActor.StopChildonly stops the actor (DriverHostActor.cs:3759-3775);grep -rn "is IAsyncDisposable\|as IDisposable" src/Server/ZB.MOM.WW.OtOpcUa.Runtimeis empty. The/rawbrowse picker (src/Core/ZB.MOM.WW.OtOpcUa.Commons/Browsing/DiscoveryDriverBrowser.cs:128-150) builds a throwaway driver per capture and also only callsShutdownAsync.ShutdownAsyncis therefore the only teardown a driver ever gets. - The comms-loss watchdog's recovery is
ShutdownAsync→InitializeAsyncon the SAME driver object, retried every 10 s whileReconnecting(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:
Everything
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; }BuildProductionRuntimeAsync(:390-437) creates —_ownedMxClient(gRPC channel),_ownedMxSession(a SECONDMxGatewayClient+ MX session +Registerhandle,Runtime/GalaxyMxSession.cs:85-87), theEventPump(two background tasks drainingStreamEvents),GatewayGalaxyAlarmFeed(backgroundStreamAlarmsloop that reconnects forever,Runtime/GatewayGalaxyAlarmFeed.cs:102-148),ReconnectSupervisor,PerPlatformProbeWatcher,DeployWatcher(WatchDeployEventsstream loop) and_ownedRepositoryClient— is released only inDisposeAsync, which nothing in the host calls. - Failure scenario: (a) a config change is a respawn (new
GalaxyDriverfrom the factory; the old child only getsShutdownAsync) → the old instance keeps its MX session registered on mxaccessgw, keeps drainingStreamEvents/StreamAlarms, keeps the deploy-watch stream open and its supervisor loop alive — permanently, one set per deploy. (b) Every AdminUI/rawbrowse 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
ShutdownAsyncdo the device-state half ofDisposeAsync(stop pump/alarm feed/deploy watcher/probe watcher/supervisor, dispose session + clients, null the seams so a laterInitializeAsyncrebuilds) and keepDisposeAsyncas the idempotent final teardown. The#630contract already puts device state underInitializeAsync/ShutdownAsyncfor 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 fromInitializeAsync: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) skipsBuildProductionRuntimeAsyncon 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 reportsDegradedwith a frozenLastSuccessfulRead(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 raiseOnRediscoveryNeededper 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 inShutdownAsync(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:
No
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);catch: on a throw the SDKSubscriptionstays insession.Subscriptions(created server-side ifCreateAsyncsucceeded) with everyMonitoredItem.Notificationclosure still attached, untracked by_subscriptions, so neitherUnsubscribeAsyncnorShutdownAsyncdeletes 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
CreateSubscriptionbut rejectsCreateMonitoredItems(e.g.BadTooManyMonitoredItems, transient timeout) makesDriverInstanceActor'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-1824intry/catchmirroring the alarm path — detach handlers, boundedsession.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-valueSELECT … 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=TagNameon a 50M-row samples table, orwhereColumn=Lineon an events table) pulls every matching row into memory every poll (default 5 s). Memory spikes to the match-set size, GC churn, andcommandTimeoutis 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+1so 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(SubscribeAsyncreturns the handle regardless),:489-499(OnReconnectedAsyncre-subscribes_subscribedTopicsonly). - Failure scenario (plain mode): the SUBSCRIBE overruns its deadline or the broker transiently refuses a filter. The topic leaves
_subscribedTopics, its tags goBadCommunicationError, butSubscribeAsyncstill returns a handle soDriverInstanceActorconsiders the subscription live and never re-subscribes; the next reconnect re-SUBSCRIBEs only_subscribedTopics, which no longer holds it. Connection state staysConnected/Healthy andLastMessageUtcadvances from other topics, so the watchdog never trips. Dark until a redeploy. - Suggested fix: keep failed filters in a pending set that
OnReconnectedAsync/the nextSubscribeAsyncretry, or throw fromSubscribeAsyncon 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(pumpfinally { PublishAgentCursor(instanceId, cursor); }, comment "dropped if a lifecycle change has already installed a newer session"),:2491-2508(PublishAgentCursoronly early-returns onsession is nullor 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
_supersededPumpsor abandoned afterTeardownWaitfinishes afterStartCoreAsyncinstalled the new session; the next pump start reads the staleInstanceId, 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
_sessionGenerationand havePublishAgentCursorrefuse when it differs (or compareReferenceEquals(session.Client, client)).
Low-severity leak nits (all High confidence)
- G-3
ReconnectSupervisor.ReportTransportFailure(Runtime/ReconnectSupervisor.cs:127-141) allocates a new_loopCtsper recovery cycle and disposes only the last one inDispose(). Bounded by outage count. - O-2
OpcUaClientDriver.InitializeAsync(:521-568): the catch closesSession(the property, still null) rather than the localsession, 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 anILoggerFactoryper driver construction, released only inDispose/DisposeAsync(:3365,:3395) — which the host never calls;_gateand_sessionWorkCtslikewise. 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 andRegisternever prunesValues; bounded by the union of every RawPath authored across redeploys. - M-3 A failed initial connect leaves the new
MqttConnection/IMqttClientundisposed (MqttDriver.cs:967-986, catch at:1029rethrows without disposing; contrastEstablishAsyncat:1011-1019). Only one stale connection is held at a time (_transportis replaced each attempt), no socket. - S-2 A timed-out SQL group keeps its connection +
maxConcurrentGroupsslot 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._gateis never disposed and a new reader is built perApplyBinding(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 quietlySubscriptionRegistry.Registeris called only fromReadViaSubscribeOnceAsync(:959) andSubscribeAsync(:1110) — never for the watcher's handles.OnPumpDataChange(:1480-1488), the only caller ofOnProbeValueChanged, is reached only via the pump's fan-out, so a*.ScanStateevent never gets there. The only test (tests/.../Health/PerPlatformProbeWatcherTests.cs) drivesOnProbeValueChangeddirectly. - Also: after a reconnect (
ReopenAsync→GalaxyMxSession.RecreateAsync) the watcher's handles are dead and nothing re-subscribes them —SyncPlatformsAsyncis called only fromDiscoverAsync(:800);ReplayAsyncrebinds only_subscriptions. - Failure scenario:
IHostConnectivityProbe.GetHostStatuses()for Galaxy only ever carries the synthetic transport entry; per-$WinPlatform/$AppEngine Running/Stopped on/hostsnever updates. Each discovery also leaves NScanStateitems advised on the gateway for nothing (released only byDispose, G-1). - Suggested fix: register the probe handles in the registry under a reserved subscription id (and
Rebindthem inResubscribeAllAsync), or route rawMxEvents 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 srcfinds no reader outside Contracts and the AdminUI form (MqttDriverForm.razor:222,MqttDriverFormModel.cs:168,274,376);MqttDriverBrowser.OpenBudgetusesConnectTimeoutSecondsclamped 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
UntilStableloop and the browse window byConnectTimeoutSeconds. - 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), AdminUIGalaxyDriverForm.razor:94. - Suggested fix: thread it into
DiscoverHierarchyAsyncor 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.ProbeTimeoutSecondshas 0 references in driver, browser, session or any host project. Both probes take the caller'stimeoutargument 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
/rawbrowse-commit viaDiscoveryDriverBrowserdoes 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-392lists the leaf withIsArray: true. - Impact: an operator can commit a tag the ingestor never feeds — warned once, then
BadWaitingForInitialDataforever. - Suggested fix: mark array leaves not-bindable in
DescribeMetricasmapped is nullalready 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(ReadAsyncignorescurrent.InstanceId),:2061-2081(onlychunk.InstanceId != instanceIdcheck),:1687-1711(AnnounceAgentRestart). - Impact: a driver with no active subscription (browse-only) keeps a cached
/probemodel across an Agent restart andOnRediscoveryNeedednever 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(noIWritable). - 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.SampleRateparsed per observation (MTConnectStreamsParser.cs:424-428) and never read;MTConnectTagDefinition.MtType/MtSubType/Unitscarried but onlyMtCategoryis read (MTConnectDriver.cs:3493). - S-5
IS_NULLABLEprojected by every dialect'sListColumnsSql(SqlServerDialect.cs:107-109,PostgresDialect.cs:141-144,OracleDialect.cs:184-187) and never read bySqlBrowseSession.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(ToFrozenDictionaryper 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
ObservationIndexwhile_pumpTaskis live and!_streamDegraded, keep/currentas the fallback; or prebuildbinding.IndexTagsas a FrozenDictionary onTagBinding.
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/intervalmakes 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 adaptscount. - Suggested fix: counter/Warning when chunks repeatedly carry exactly
SampleCountobservations orexpected - chunk.FirstSequenceshrinks toward zero; document the cap next toSampleCount.
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) thenawait source.GetHierarchyAsync(cancellationToken)again just to select platforms. - Failure scenario: two full
DiscoverHierarchyAsyncsweeps (every object + attribute) per browse/deploy on a large galaxy — and the second feeds the dead probe path (G-4). - Suggested fix: have
GalaxyDiscovererexpose 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 acrossBrowseRecursiveAsync+ 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 throwawayDiscoveryDriverBrowserinstance). - 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(TimedOutwarns every poll per group),:411-419(QueryAsyncwarns with the fullplan.SqlTextevery poll per failed group); only contract warnings go throughShouldWarnAboutContract()(: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 perReadAsync),:206(SqlGroupPlanner.Planper poll) vsSqlQueryPlan.cs:68-70("safe to cache and reuse across polls" — nothing caches). - Suggested fix: cache plans per subscription reference list, invalidated on
ApplyCatalogGateAsyncswap.
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
WriteAsyncpublishes serially, one awaited QoS 1 round-trip per request bounded byConnectTimeoutSeconds(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 severalid=valuepairs.
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
TryDecodeUtf8allocatesnew UTF8Encoding(false, true)per Raw/Scalar delivery (MqttSubscriptionManager.cs:1374);MqttBrowseSession.cs:150already uses a static. - T-7 The MTConnect connectivity probe parses the full
/probedevice 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
OnDataChangeis invoked under the driver-wide_deliveryLock(OpcUaClientDriver.cs:368-380,:1888-1912) — deliberate; acceptable only while every subscriber justTells.
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+InitializeAsyncneither tears down nor reconnects — it sets_health = Healthy(:381, masked byGetHealth()preferring_supervisor.IsDegraded) and spawns another DeployWatcher (G-2). If the supervisor loop ends with a dead session (e.g.ReopenAsyncreturning early on_ownedMxSession is null,:448, thenReplaying→Healthy), the host has no lever short of a respawn. - Suggested fix: as G-1 — make
ShutdownAsyncrelease 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:Acatch (Exception ex) { _logger.LogWarning(ex, "...AcknowledgeAsync failed; acknowledgements may not have been applied."); }ServiceResultExceptionfromsession.CallAsync(faulted session, closed channel) returns normally; the local condition is acked while upstream still shows it unacknowledged. The nextConditionRefreshdedup will re-report it as unacked, but the operator received success. - Suggested fix: rethrow (or return per-ack failures) so the OPC UA
Acknowledgecall 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 fromOnPumpDataChange/ReadViaSubscribeOnceAsync. - Failure scenario: the watchdog's conjunction degenerates to "non-Healthy for 90 s" for Galaxy, so any supervisor
Degradedlasting 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'sRecordLivenessEvidencedoes.
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:
DispatchLoopAsynccatches any exception, logs "events past this point will be lost until restart" and exits;RunAsynckeeps draining into the bounded channel until it fills, then only counts drops. Nothing callsNotifyStreamFault;GetHealth()staysHealthyand (G-11) the stamp never moves anyway.Dispatchisolates handler exceptions, so the remaining throw sites areStatusCodeMap.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; contrastMTConnectDriverProbe.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
StartCoreAsyncand 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}") vsSqlDriverProbe.cs:84-95,SqlDriverBrowser.cs:292-303(message withheld "because the malformed text may carry a connection string"). The deployed config should carry onlyconnectionStringRef, so this bites only on a pasted literal — but the message then reachesInitializeFailed→ healthLastError→/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-effortDeleteAsyncper 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/DisposeAsyncfall back to an ungatedTeardownAsyncafterConnectTimeoutSeconds(MqttDriver.cs:402-409,:929-935); the post-connect disposed re-check (:993-997) coversDisposeAsynconly, so a wedgedInitializeCoreAsynccompleting after a timed-outShutdownAsynccan publish one stray live connection — the actor's own Shutdown→Initialize sequence tears it down next time (bounded to one). - M-11
MqttSubscriptionManager.EstablishAsyncwith no transport records topics as subscribed and returns success (:646-665); unreachable today (AttachToalways precedes connect,MqttDriver.cs:972-979) but a silent "subscribed, receives nothing" if the ordering ever changes. - O-10
ConnectionLostraised before the host processedInitializeSucceededis 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
ObservePollOutcomeduring the nextInitializeAsync;UpdateHealthUnlessFaultedguards onlyFaulted, so a late Good read can briefly flipInitializing→Healthy(cosmetic; nextWriteHealthoverrides). - S-10 The SQL catalog gate rejects rather than reloads on schema drift (
SqlDriver.cs:929-972,:898-901): a column added after init staysBadNodeIdUnknownuntil 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 infinally; internal 30 s deadline coversCancellationToken.None; aSubscribeBulkcompleting after the deadline is unsubscribed by continuation.SubscriptionRegistryprunes the reverse map onRemove/Rebind; nothing grows past the live subscription set.EventPumpchannel is bounded (50 000 default, configurable) with explicitTryWrite+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.SubscribeAlarmsAsynccollapses to a single handle (:1310-1322); the per-reconnect handle growth it describes is closed.GatewayGalaxyDataWriterhandle/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 onDisposeAsync/finally; open path disposes on failure.
OpcUaClient
ReleaseSessionAsync(:1055-1095) detaches keep-alive + diagnostics handlers, boundsCloseAsyncby 5 s, skips the wire close when the session is known lost, and re-arms every outage latch;InitializeAsynccalls it first, so a re-init cannot orphan the previous session.- Comms-loss contract (#527/#670/#676): bad keep-alive →
Degradedwith the stamp preserved; good keep-alives/notifications advance it only while no subscription is latched lost; definitive statuses raiseConnectionLostonce per session;PublishCommsLossfansBadNoCommunicationwith value retained; recovery dedup is(value, status)(IsReconcileRepair). _lastValues/_lastGoodSourceTicks/_deliverySeq/_awaitingRecovery/_replayGuardare keyed by authored RawPath and cleared inShutdownAsync; bounded even thoughUnsubscribeAsyncdoes not prune them.CancelSessionWorkdeliberately 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/MaxBrowseDepthbound the browse. Browser session/OpcUaClientDriverBrowser/probe dispose on every path.
MQTT
- Teardown-before-rebuild still holds:
InitializeCoreAsynccalls the idempotentTeardownAsync()first (MqttDriver.cs:955), which reaches all three disposable fields —_primaryHost,_hostProbeCts,_connection(:1381-1405); no new disposable field escaped it. MqttConnectionreconnect loop does not leak per attempt: oneIMqttClientcreated once (:718-724), per-attempt CTSsusing-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,_metadataRebirthspruned to authored nodes onRegister(:421-454) and cleared on session reset; NDEATH/DDEATH evict (:1076,:1091);_birthSignaturespruned to_authoredScopes(MqttDriver.cs:1221-1227);_warnedcapped at 1024 (:1912-1922);_pendingRebirthsself-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 fromMqttConnection.StateandLastSuccessfulReadfromLastMessageUtc(MqttDriver.cs:421-440);ConnectionLostfires on theConnected→Reconnectingedge (MqttConnection.cs:976-992); both ingest paths stale toBadNoCommunication(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
DisposeAsyncand 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)preservingLastSuccessfulRead(:3152-3159); every chunk incl. heartbeats →PublishHealthy(now)(:2102-2103); a quiet enumeration end under a live token is reported asMTConnectStreamEndedException/Transient, never idle (:2106-2117,MTConnectAgentClient.cs:237-243) — no clean-termination evasion;/currentand/sampleown 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):
InitializeAsyncstops pump + probe and disposes the old client before building (:565-574); a client built in a failed attempt is disposed (:2783).MTConnectAgentClientdisposed on every teardown path;HttpResponseMessage/streamsusing/await using(MTConnectAgentClient.cs:175-191, 208-214); heartbeat watchdog bounds every body read, buffer capped at 32 MiB, DTD prohibited. _supersededPumpsis pruned (:1742-1754); index/tracker/told-record maps are bounded by the Agent's DataItem count. Streams parser is forward-onlyXmlReaderwith a single-string fast path; no JSON on the hot path.
SQL
- Every
DbConnection/DbCommand/DbDataReaderpath usesawait 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.Detachobserves faults. - Connection string never logged: only
Endpointis 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) thenQuoteIdentifierwith doubling + control-char rejection in all three dialects; OracleBindByNameset 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-BadTimeoutfreezes it → watchdog →ShutdownAsync(_poll.DisposeAsync, 5 s cap) →InitializeAsync→ re-subscribe; a query-failure-only outage isDegradedwith the clock advancing, so no reconnect loop. Groups run in parallel underTask.WhenAllbounded bymaxConcurrentGroups; key-value groups chunk at dialect bind caps;CommandTimeoutfloored at 1 s; factory enforcesoperationTimeout > commandTimeout.