dohertj2/lmxopcua
1
0
Fork 0
Code Issues 2 Pull Requests 1 Actions Packages Projects Releases Wiki Activity

fix(scripted-alarms): lock CreateVariable + RemoveRootNotifier on rebuild (T14 review)

Browse Source
This commit is contained in:
Joseph Doherty
2026-06-10 19:26:01 -04:00
parent 60d48a2a0a
commit b31d7cb03f
1 changed files with 29 additions and 6 deletions
Download Patch File Download Diff File Expand all files Collapse all files
src/Server/ZB.MOM.WW.OtOpcUa.OpcUaServer/OtOpcUaNodeManager.cs
+29 -6
View File
@@ -30,8 +30,10 @@ public sealed class OtOpcUaNodeManager : CustomNodeManager2
private readonly ConcurrentDictionary<string, FolderState> _folders = new(StringComparer.Ordinal);
private readonly ConcurrentDictionary<string, AlarmConditionState> _alarmConditions = new(StringComparer.Ordinal);
/// <summary>Folders we have already promoted to event-notifiers + registered as root notifiers,
/// so repeated <see cref="MaterialiseAlarmCondition"/> calls don't double-add (idempotent guard).</summary>
private readonly HashSet<NodeId> _notifierFolders = new();
/// so repeated <see cref="MaterialiseAlarmCondition"/> calls don't double-add (idempotent guard).
/// Keyed by NodeId → the actual <see cref="FolderState"/> so <see cref="RebuildAddressSpace"/> can
/// pass the folder to <c>RemoveRootNotifier</c> on teardown.</summary>
private readonly Dictionary<NodeId, FolderState> _notifierFolders = new();
private FolderState? _root;
/// <summary>Initializes a new instance of the <see cref="OtOpcUaNodeManager"/> class with the OPC UA server and configuration.</summary>
@@ -69,10 +71,17 @@ public sealed class OtOpcUaNodeManager : CustomNodeManager2
public void WriteValue(string nodeId, object? value, OpcUaQuality quality, DateTime sourceTimestampUtc)
{
ArgumentException.ThrowIfNullOrEmpty(nodeId);
var variable = _variables.GetOrAdd(nodeId, CreateVariable);
lock (Lock)
{
// CreateVariable mutates the SDK address space (_root.AddChild + AddPredefinedNode),
// so it MUST run under Lock — the SDK's subscription/ConditionRefresh threads take it too.
if (!_variables.TryGetValue(nodeId, out var variable))
{
variable = CreateVariable(nodeId);
_variables[nodeId] = variable;
}
variable.Value = value;
variable.StatusCode = StatusFromQuality(quality);
variable.Timestamp = sourceTimestampUtc;
@@ -117,9 +126,15 @@ public sealed class OtOpcUaNodeManager : CustomNodeManager2
}
// Fallback: alarm not materialised as a real condition — keep the legacy bool[2] variable.
var variable = _variables.GetOrAdd(alarmNodeId, CreateVariable);
lock (Lock)
{
// CreateVariable mutates the SDK address space, so it MUST run under Lock (see WriteValue).
if (!_variables.TryGetValue(alarmNodeId, out var variable))
{
variable = CreateVariable(alarmNodeId);
_variables[alarmNodeId] = variable;
}
variable.Value = new[] { active, acknowledged };
variable.StatusCode = StatusCodes.Good;
variable.Timestamp = sourceTimestampUtc;
@@ -230,7 +245,7 @@ public sealed class OtOpcUaNodeManager : CustomNodeManager2
/// alarm condition has a notifier path to the Server object for T16's event propagation.</summary>
private void EnsureFolderIsEventNotifier(FolderState folder)
{
if (!_notifierFolders.Add(folder.NodeId)) return;
if (!_notifierFolders.TryAdd(folder.NodeId, folder)) return;
folder.EventNotifier = EventNotifiers.SubscribeToEvents;
AddRootNotifier(folder);
folder.ClearChangeMasks(SystemContext, includeChildren: false);
@@ -240,7 +255,6 @@ public sealed class OtOpcUaNodeManager : CustomNodeManager2
/// <see cref="EventSeverity"/> enum buckets the SDK's <c>SetSeverity</c> expects.</summary>
private static EventSeverity MapSeverity(int severity) => severity switch
{
<= 0 => EventSeverity.Low,
< 200 => EventSeverity.Low,
< 400 => EventSeverity.MediumLow,
< 600 => EventSeverity.Medium,
@@ -380,6 +394,15 @@ public sealed class OtOpcUaNodeManager : CustomNodeManager2
}
_folders.Clear();
// Detach the Server↔folder HasNotifier ref for every promoted folder before dropping the
// guard, otherwise the rebuild leaks an orphaned root-notifier reference on the Server
// object. RemoveRootNotifier just severs that link, so its order relative to the folder
// teardown above doesn't matter — but it must run under this same Lock.
foreach (var folder in _notifierFolders.Values)
{
RemoveRootNotifier(folder);
}
// Drop the notifier-folder guard so re-materialised alarms re-promote their (rebuilt)
// equipment folders to event notifiers.
_notifierFolders.Clear();