From ab5d0752d8341308d520321ece87f3c3bdcbc8bb Mon Sep 17 00:00:00 2001 From: Joseph Doherty Date: Wed, 10 Jun 2026 19:45:24 -0400 Subject: [PATCH] fix(scripted-alarms): atomic alarm-condition lookup under Lock (T15 review) --- .../OtOpcUaNodeManager.cs | 17 ++++++++--------- 1 file changed, 8 insertions(+), 9 deletions(-) diff --git a/src/Server/ZB.MOM.WW.OtOpcUa.OpcUaServer/OtOpcUaNodeManager.cs b/src/Server/ZB.MOM.WW.OtOpcUa.OpcUaServer/OtOpcUaNodeManager.cs index 2d0a6573..cb2dbfa1 100644 --- a/src/Server/ZB.MOM.WW.OtOpcUa.OpcUaServer/OtOpcUaNodeManager.cs +++ b/src/Server/ZB.MOM.WW.OtOpcUa.OpcUaServer/OtOpcUaNodeManager.cs @@ -107,9 +107,11 @@ public sealed class OtOpcUaNodeManager : CustomNodeManager2 ArgumentException.ThrowIfNullOrEmpty(alarmNodeId); ArgumentNullException.ThrowIfNull(state); - if (_alarmConditions.TryGetValue(alarmNodeId, out var condition)) + // Look up + project under a SINGLE Lock so a concurrent RebuildAddressSpace can't clear + // _alarmConditions / detach the condition node between the lookup and the Set* calls. + lock (Lock) { - lock (Lock) + if (_alarmConditions.TryGetValue(alarmNodeId, out var condition)) { // EnabledState / AckedState / ActiveState are mandatory children — always present after // Create. Confirm + Shelving are optional Part 9 children: T14's real-server finding is @@ -147,15 +149,12 @@ public sealed class OtOpcUaNodeManager : CustomNodeManager2 // NO ReportEvent here — T16 owns event firing. ClearChangeMasks just notifies any // attribute (not event) subscribers watching the condition's children directly. condition.ClearChangeMasks(SystemContext, includeChildren: true); + return; } - return; - } - // Fallback: alarm not materialised as a real condition — keep the legacy bool[2] variable so - // un-materialised callers (and the existing unit tests) keep working. - lock (Lock) - { - // CreateVariable mutates the SDK address space, so it MUST run under Lock (see WriteValue). + // Fallback: alarm not materialised as a real condition — keep the legacy bool[2] variable so + // un-materialised callers (and the existing unit tests) keep working. CreateVariable mutates + // the SDK address space, so it MUST run under Lock (see WriteValue). if (!_variables.TryGetValue(alarmNodeId, out var variable)) { variable = CreateVariable(alarmNodeId);