fix(opcua): condition events never block under the node-manager lock; folder-event Part 3 note (#838)

#838(a): each notifier's SDK MonitoredNode2 drains a bounded channel (4096,
FullMode=Wait) and the synchronous NodeState.ReportEvent blocks when it is
full. Condition events are reported under Lock, and the delivery task takes
Lock on a verdict-cache miss (the SDK's own GetNodeMetadata, flag off too, and
#826's IsNotifierEventVisibleTo). A burst of >4096 events under one Lock hold
therefore deadlocked the node manager -- FolderEventFloodWireTests stalled at
exactly 4097 events with enforcement on AND off (so it predates #826).

ReportConditionEvent now goes through ReportEventWithoutBlocking:
ReportEventAsync completes inline while channels have room (unchanged normal
path); on overflow the report continues asynchronously holding no thread or
lock, and later reports queue behind it so events keep report order. The
deferred work holds only frozen snapshots and drains once Lock is released;
client delivery stays bounded by each item's queue.

#838(b): documented in docs/security.md that Part 3 ReceiveEvents applies to
the event's EventType and SourceNode (the SDK check, kept first), not to the
notifier; our decision is layered on top and non-condition events on a folder
item fail closed under enforcement. The reflection item stays open.
This commit is contained in:
Joseph Doherty
2026-11-11 04:06:28 -05:00
parent ffba75bb93
commit 91e28e87be
4 changed files with 268 additions and 1 deletions
@@ -1362,7 +1362,7 @@ public sealed class OtOpcUaNodeManager : CustomNodeManager2, INodeManager3
// subscriptions, per item, through the ACL (see OnConditionReportEvent).
var snapshot = new InstanceStateSnapshot();
snapshot.Initialize(SystemContext, alarm);
alarm.ReportEvent(SystemContext, snapshot);
ReportEventWithoutBlocking(alarm, snapshot);
}
catch (Exception ex)
{
@@ -1375,6 +1375,95 @@ public sealed class OtOpcUaNodeManager : CustomNodeManager2, INodeManager3
}
}
/// <summary>Serializes <see cref="_eventReportChain"/>.</summary>
private readonly object _eventReportGate = new();
/// <summary>The tail of the deferred condition-event reports (Gitea #838); completed when none is pending.</summary>
private Task _eventReportChain = Task.CompletedTask;
private int _deferredEventReports;
/// <summary>Condition-event reports currently deferred because a notifier's delivery channel was full (test observation).</summary>
internal int DeferredEventReports => Volatile.Read(ref _deferredEventReports);
/// <summary>
/// Reports a condition event through the condition's notifier chain WITHOUT blocking the caller, in report
/// order (Gitea #838).
/// </summary>
/// <param name="source">The condition.</param>
/// <param name="e">The frozen snapshot of the event.</param>
/// <remarks>
/// <para>
/// <b>Why not <c>NodeState.ReportEvent</c>.</b> Each notifier with event items has an SDK
/// <c>MonitoredNode2</c> whose delivery task drains a BOUNDED channel (4096, <c>FullMode = Wait</c>), and
/// the synchronous <c>ReportEvent</c> BLOCKS when that channel is full. Condition events are reported
/// under <c>Lock</c>, and the delivery task takes <c>Lock</c> on every per-event verdict-cache miss (the
/// SDK's own <c>GetNodeMetadata</c>, and #826's <see cref="IsNotifierEventVisibleTo"/>). So a burst of more
/// than 4096 events under one hold of <c>Lock</c>, starting at a miss, parked the consumer on <c>Lock</c>
/// while the producer waited on the consumer: a deadlock that froze the whole node manager (pinned by
/// <c>FolderEventFloodWireTests</c>, which stalled at exactly 4097 events before this).
/// </para>
/// <para>
/// <b>What this does.</b> The common case is unchanged and synchronous: <c>ReportEventAsync</c> completes
/// inline while every channel has room. Only when a channel is full does that report continue
/// asynchronously (awaiting channel space, holding no thread and no lock), and every later report queues
/// behind it, so events still leave in the order they were reported. The deferred work holds only frozen
/// snapshots, so it is bounded by the burst that caused it and drains once <c>Lock</c> is released.
/// Delivery to clients stays bounded by each monitored item's own queue (oldest discarded). A deferred
/// report runs the condition's synchronous <c>OnReportEvent</c> hook (<see cref="OnConditionReportEvent"/>)
/// off <c>Lock</c>; that hook takes <c>Lock</c> itself where it reads guarded state.
/// </para>
/// </remarks>
private void ReportEventWithoutBlocking(NodeState source, IFilterTarget e)
{
lock (_eventReportGate)
{
if (_eventReportChain.IsCompleted)
{
var report = source.ReportEventAsync(SystemContext, e);
if (report.IsCompletedSuccessfully) return;
Interlocked.Increment(ref _deferredEventReports);
_eventReportChain = ObserveEventReportAsync(report.AsTask(), source);
return;
}
Interlocked.Increment(ref _deferredEventReports);
_eventReportChain = ReportAfterAsync(_eventReportChain, source, e);
}
}
private async Task ReportAfterAsync(Task previous, NodeState source, IFilterTarget e)
{
await previous.ConfigureAwait(false); // never faults: every link observes its own failure
Task report;
try
{
report = source.ReportEventAsync(SystemContext, e).AsTask();
}
catch (Exception ex)
{
report = Task.FromException(ex);
}
await ObserveEventReportAsync(report, source).ConfigureAwait(false);
}
private async Task ObserveEventReportAsync(Task report, NodeState source)
{
try
{
await report.ConfigureAwait(false);
}
catch (Exception ex)
{
_logger.LogError(ex, "OtOpcUaNodeManager: failed to report Part 9 condition event for {NodeId}", source.NodeId);
}
finally
{
Interlocked.Decrement(ref _deferredEventReports);
}
}
/// <summary>
/// The gate-relevant slice of a Part 9 condition's state — exactly the fields that drive a
/// condition event AND that an <see cref="AlarmConditionSnapshot"/> can change. As a record, two