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

fix(opcua): #855 review 3 — plain refresh skips removed conditions; per-hop failure isolation

Browse Source 
- Both plain ConditionRefresh paths (Server object and folder) skip a
  condition a deploy removed or replaced between the off-Lock walk and the
  queueing (the SDK base did not either; only the cut path checked).
- A failure delivering at one notifier hop is logged and the walk continues
  to the remaining hops; a failure deciding the Server-object items withholds
  only them.
- Test: a condition removed during a plain refresh (folder + Server object).

Claude-Session: https://claude.ai/code/session_01GT67AD99zQQqpLCVm3p7xk
This commit is contained in:
Joseph Doherty
2026-11-11 12:51:03 -05:00
parent f775625805
commit 7783fadae1
2 changed files with 72 additions and 2 deletions
Download Patch File Download Diff File Expand all files Collapse all files
src/Server/ZB.MOM.WW.OtOpcUa.OpcUaServer/OtOpcUaNodeManager.cs
+29 -2
View File
@@ -1651,7 +1651,19 @@ public sealed class OtOpcUaNodeManager : CustomNodeManager2, INodeManager3
}
// Re-checked: a stall may have outlived the condition (the liveness check above ran before it).
if (IsLiveCondition(condition)) DeliverToNotifierItems(monitored, e);
if (IsLiveCondition(condition))
{
// #855 review 3: one hop's failure must not cost the hops after it (before #855 this hop was a channel
// write, which cannot throw; the SDK isolated failures per item).
try
{
DeliverToNotifierItems(monitored, e);
}
catch (Exception ex)
{
_logger.LogWarning(ex, "OtOpcUaNodeManager: failed to deliver a condition event at notifier {NodeId}", node.NodeId);
}
}
}
else
{
@@ -5815,7 +5827,18 @@ public sealed class OtOpcUaNodeManager : CustomNodeManager2, INodeManager3
// through its OLD equipment folder (#808).
if (!IsLiveCondition(node)) return;
var scopes = AclEnforced ? ConditionVisibilityScopes(node) : null;
List<NodeId>? scopes;
try
{
scopes = AclEnforced ? ConditionVisibilityScopes(node) : null;
}
catch (Exception ex)
{
// #855 review 3: withheld from the Server-object items (fail closed), but the walk goes on to the folders.
_logger.LogWarning(ex, "OtOpcUaNodeManager: failed to decide a condition event for Server-object items ({NodeId})", node.NodeId);
return;
}
var reportingSession = (context as ISessionSystemContext)?.SessionId;
var deferred = e is ConditionEventSnapshot { Deferred: true } d ? d : null;
@@ -6595,6 +6618,8 @@ public sealed class OtOpcUaNodeManager : CustomNodeManager2, INodeManager3
foreach (var (e, _, scopes) in replay)
{
// #855 review 3: never replay a condition a deploy removed or replaced since the walk collected it.
if (ConditionOf(e) is AlarmConditionState walked && !IsLiveCondition(walked)) continue;
if (scopes is not null && !IsConditionVisibleTo(item, scopes)) continue;
if (ServiceResult.IsBad(ValidateEventRolePermissions(item, e))) continue;
item.QueueEvent(e);
@@ -6878,6 +6903,8 @@ public sealed class OtOpcUaNodeManager : CustomNodeManager2, INodeManager3
DirectRefreshProbe?.Invoke(false, Monitor.IsEntered(Lock));
foreach (var e in events)
{
// #855 review 3: never replay a condition a deploy removed or replaced since the walk collected it.
if (ConditionOf(e) is AlarmConditionState walked && !IsLiveCondition(walked)) continue;
if (!IsDirectRefreshEventAllowed(item, e, enforced)) continue;
item.QueueEvent(e);
}
tests/Server/ZB.MOM.WW.OtOpcUa.OpcUaServer.IntegrationTests/FolderConditionRefreshWireTests.cs
+43
View File
@@ -489,6 +489,49 @@ public sealed class FolderConditionRefreshWireTests
after.ShouldBeEmpty("and none of its deferred events follows");
}
/// <summary>
/// #855 review 3: the plain refresh path (nothing deferred — the only path production takes) collects live
/// conditions off <c>Lock</c> and queues them afterwards; a condition removed in between must not be replayed.
/// </summary>
/// <param name="serverObject">Refresh a Server-object item instead of a folder item.</param>
[Theory]
[InlineData(false)]
[InlineData(true)]
public async Task A_condition_removed_during_a_plain_refresh_is_not_replayed(bool serverObject)
{
var ct = TestContext.Current.CancellationToken;
await using var rig = await ServerEventRig.StartAsync("Refresh855RemovePlain", encrypted: false, ct);
rig.KeyRoles = [];
rig.MaterialiseNative();
rig.Enforce(false, [ServerEventRig.Key()], KeyEquipmentGrant(EqA));
using var session = await rig.ConnectAsync(anonymous: false, ct);
var events = new RecordedEvents();
var subscription = await RecordedEvents.SubscribeAsync(session, events, ct);
var item = RecordedEvents.AddEventItem(subscription, serverObject ? ObjectIds.Server : ServerEventRig.UnsNodeId(session, EqA));
await subscription.ApplyChangesAsync(ct);
rig.Project(native: true, Active("seed"));
await RecordedEvents.WaitUntilAsync(() => events.Has(item, "seed"));
var removed = 0;
void Remove(bool cut, bool _)
{
if (!cut && Interlocked.Exchange(ref removed, 1) == 0)
rig.Nm.RemoveAlarmConditionNode(ServerEventRig.RawAlarmPath, AddressSpaceRealm.Raw).ShouldBeTrue();
}
if (serverObject) rig.Nm.ServerObjectRefreshProbe = Remove;
else rig.Nm.DirectRefreshProbe = Remove;
await session.CallAsync(ObjectTypeIds.ConditionType, MethodIds.ConditionType_ConditionRefresh, ct, subscription.Id);
await RecordedEvents.WaitUntilAsync(() => events.HasType(item, ObjectTypeIds.RefreshEndEventType));
rig.Nm.ServerObjectRefreshProbe = null;
rig.Nm.DirectRefreshProbe = null;
Volatile.Read(ref removed).ShouldBe(1, "the plain path ran");
Window(events.Of(item)).Replayed.ShouldBeEmpty("the condition was removed before the replay was queued");
}
/// <summary>
/// #855 review: a delivery that fails inline (here: a hop that throws) is over, not suspended — it must not be booked
/// as a deferral episode (which moved the episode a concurrent plain refresh checks, and cleared cuts under the