cleanup and fixes

Author: romanett

.github/agents/opcua-v16-migration.agent.md

@@ -173,6 +173,7 @@ Also: `ReadOnlyList<T>` → `ArrayOf<T>`, `IList<T>` parameters → `ArrayOf<T>`
 - Use `List<T>` when items are added/removed/modified, then convert to `ArrayOf<T>` with `.ToArrayOf()`.
 - `ArrayOf<T>` implicitly converts from `List<T>` but not vice versa. Use `.ToList()` to convert back.
 - `ArrayOf<T>` supports collection expressions: `ArrayOf<int> arr = [1, 2, 3];`
+- To follow best coding practices Do NOT use casts to create the ArrayOf but use `.ToArrayOf()` or create it directly using a collection expression
 
 ### ArrayOf<T> Key API
 

Libraries/Opc.Ua.Server/Diagnostics/DiagnosticsNodeManager.cs

@@ -262,29 +262,26 @@ protected ServiceResult OnGetMonitoredItems(
                 return StatusCodes.BadInvalidArgument;
             }
 
-            foreach (ISubscription subscription in Server.SubscriptionManager.GetSubscriptions())
+            if (!Server.SubscriptionManager.TryGetSubscription(subscriptionId, out ISubscription subscription))
             {
-                if (subscription.Id == subscriptionId)
-                {
-                    if (context is ISessionSystemContext session &&
-                        subscription.SessionId != session.SessionId)
-                    {
-                        // user tries to access subscription of different session
-                        return StatusCodes.BadUserAccessDenied;
-                    }
+                return StatusCodes.BadSubscriptionIdInvalid;
+            }
 
-                    subscription.GetMonitoredItems(
-                        out ArrayOf<uint> serverHandles,
-                        out ArrayOf<uint> clientHandles);
+            if (context is ISessionSystemContext session &&
+                subscription.SessionId != session.SessionId)
+            {
+                // user tries to access subscription of different session
+                return StatusCodes.BadUserAccessDenied;
+            }
 
-                    outputArguments[0] = serverHandles;
-                    outputArguments[1] = clientHandles;
+            subscription.GetMonitoredItems(
+                out ArrayOf<uint> serverHandles,
+                out ArrayOf<uint> clientHandles);
 
-                    return ServiceResult.Good;
-                }
-            }
+            outputArguments[0] = serverHandles;
+            outputArguments[1] = clientHandles;
 
-            return StatusCodes.BadSubscriptionIdInvalid;
+            return ServiceResult.Good;
         }
 
         /// <summary>
@@ -306,24 +303,21 @@ protected ServiceResult OnResendData(
                 return StatusCodes.BadInvalidArgument;
             }
 
-            foreach (ISubscription subscription in Server.SubscriptionManager.GetSubscriptions())
+            if (!Server.SubscriptionManager.TryGetSubscription(subscriptionId, out ISubscription subscription))
             {
-                if (subscription.Id == subscriptionId)
-                {
-                    if (context is not ServerSystemContext session ||
-                        subscription.SessionId != session.SessionId)
-                    {
-                        // user tries to access subscription of different session
-                        return StatusCodes.BadUserAccessDenied;
-                    }
-
-                    subscription.ResendData(session.OperationContext);
+                return StatusCodes.BadSubscriptionIdInvalid;
+            }
 
-                    return ServiceResult.Good;
-                }
+            if (context is not ServerSystemContext session ||
+                subscription.SessionId != session.SessionId)
+            {
+                // user tries to access subscription of different session
+                return StatusCodes.BadUserAccessDenied;
             }
 
-            return StatusCodes.BadSubscriptionIdInvalid;
+            subscription.ResendData(session.OperationContext);
+
+            return ServiceResult.Good;
         }
 
         /// <summary>

Libraries/Opc.Ua.Server/Subscription/ISubscriptionManager.cs

@@ -58,6 +58,14 @@ public interface ISubscriptionManager : IDisposable
         /// <returns>A list of the subscriptions.</returns>
         IList<ISubscription> GetSubscriptions();
 
+        /// <summary>
+        /// Get the subscription with the specified id
+        /// </summary>
+        /// <param name="id">The id of the subscription</param>
+        /// <param name="subscription">The subscription if found else null</param>
+        /// <returns>True if found</returns>
+        bool TryGetSubscription(uint id, out ISubscription subscription);
+
         /// <summary>
         /// Set a subscription into durable mode
         /// </summary>

Libraries/Opc.Ua.Server/Subscription/SubscriptionManager.cs

@@ -181,15 +181,19 @@ public event SubscriptionEventHandler SubscriptionDeleted
             }
         }
 
-        /// <summary>
-        /// Returns all of the subscriptions known to the subscription manager.
-        /// </summary>
-        /// <returns>A list of the subscriptions.</returns>
+
+        /// <inheritdoc/>
         public IList<ISubscription> GetSubscriptions()
         {
             return [.. m_subscriptions.Values];
         }
 
+        /// <inheritdoc/>
+        public bool TryGetSubscription(uint id, out ISubscription subscription)
+        {
+            return m_subscriptions.TryGetValue(id, out subscription);
+        }
+
         /// <summary>
         /// Raises an event related to a subscription.
         /// </summary>

README.md

@@ -37,6 +37,8 @@ More samples based on the official [Nuget](https://www.nuget.org/packages/OPCFou
 * Source generator generated code behind during build
 * See [MigrationGuide](Docs/MigrationGuide.md) for details.
 * New AsyncCustomNodeManager (successor of CustomNodeManager2) with improved Locking Strategy, see [Server Async (TAP) Support](Docs/AsyncServerSupport.md)
+* In our Load Test the Server shows at least 2.5x higher throughput under load with 750 subscriptions totaling 450k Monitored items 
+ and write times for 600 items below 5 seconds were before > 10 seconds were needed. Also for event at least 3x faster event reporting was observed.
 
 #### **New in 1.05.378**
 

Stack/Opc.Ua.Core/Stack/Server/ServerBase.cs

@@ -1669,6 +1669,23 @@ protected virtual void Dispose(bool disposing)
 
                     m_queue.Writer.Complete();
 
+                    // Wait for all worker threads to complete
+                    Task[] workerTasks;
+                    lock (m_workers)
+                    {
+                        workerTasks = [.. m_workers];
+                    }
+                    
+                    try
+                    {
+                        Task.WaitAll(workerTasks, TimeSpan.FromSeconds(5));
+                    }
+                    catch (AggregateException)
+                    {
+                        // Ignore exceptions during shutdown
+                    }
+
+                    // Now drain any remaining requests from the queue
                     while (m_queue.Reader.TryRead(out IEndpointIncomingRequest request))
                     {
                         request.OperationCompleted(null, StatusCodes.BadServerHalted);

Tests/Opc.Ua.Server.Tests/DiagnosticsNodeManagerTests.cs

@@ -206,7 +206,8 @@ public async Task GetMonitoredItems_ValidatesSessionAndReturnsItemsAsync()
             ArrayOf<uint> clientHandles = [3, 4];
             subMock.Setup(s => s.GetMonitoredItems(out serverHandles, out clientHandles));
 
-            m_subscriptionManagerMock.Setup(m => m.GetSubscriptions()).Returns([subMock.Object]);
+            ISubscription outSub = subMock.Object;
+            m_subscriptionManagerMock.Setup(m => m.TryGetSubscription(It.IsAny<uint>(), out outSub)).Returns(true);
 
             using var manager = new DiagnosticsNodeManager(m_serverMock.Object, config, NullLogger.Instance);
             var externalRefs = new Dictionary<NodeId, IList<IReference>>();
@@ -247,7 +248,8 @@ public async Task GetMonitoredItems_InvalidSubscriptionId_ReturnsBadSubscription
             var config = new ApplicationConfiguration { ServerConfiguration = new ServerConfiguration() };
             SetupServerMock();
 
-            m_subscriptionManagerMock.Setup(m => m.GetSubscriptions()).Returns(System.Array.Empty<ISubscription>());
+            ISubscription outSub = null;
+            m_subscriptionManagerMock.Setup(m => m.TryGetSubscription(It.IsAny<uint>(), out outSub)).Returns(false);
 
             using var manager = new DiagnosticsNodeManager(m_serverMock.Object, config, NullLogger.Instance);
             var externalRefs = new Dictionary<NodeId, IList<IReference>>();
@@ -279,7 +281,8 @@ public async Task ResendData_ValidatesAccessAndCallsResendDataAsync()
             subMock.Setup(s => s.SessionId).Returns(new NodeId(1, 1));
             subMock.Setup(s => s.ResendData(It.IsAny<OperationContext>()));
 
-            m_subscriptionManagerMock.Setup(m => m.GetSubscriptions()).Returns([subMock.Object]);
+            ISubscription outSub = subMock.Object;
+            m_subscriptionManagerMock.Setup(m => m.TryGetSubscription(It.IsAny<uint>(), out outSub)).Returns(true);
 
             using var manager = new DiagnosticsNodeManager(m_serverMock.Object, config, NullLogger.Instance);
             var externalRefs = new Dictionary<NodeId, IList<IReference>>();
@@ -324,7 +327,8 @@ public async Task ResendData_InvalidSubscriptionId_ReturnsBadSubscriptionIdInval
             var config = new ApplicationConfiguration { ServerConfiguration = new ServerConfiguration() };
             SetupServerMock();
 
-            m_subscriptionManagerMock.Setup(m => m.GetSubscriptions()).Returns(System.Array.Empty<ISubscription>());
+            ISubscription outSub = null;
+            m_subscriptionManagerMock.Setup(m => m.TryGetSubscription(It.IsAny<uint>(), out outSub)).Returns(false);
 
             using var manager = new DiagnosticsNodeManager(m_serverMock.Object, config, NullLogger.Instance);
             var externalRefs = new Dictionary<NodeId, IList<IReference>>();

Tests/Opc.Ua.Server.Tests/ReferenceServerTest.cs

@@ -739,10 +739,10 @@ public async Task ServerEventSubscribeTestAsync()
             Assert.IsFalse(eventFields[2].IsNull); // SourceNode
             Assert.IsFalse(eventFields[3].IsNull); // SourceName
             Assert.IsFalse(eventFields[4].IsNull); // Time
-            eventFields[5].TryGet(out LocalizedText receivedMessage); // Message
+            Assert.That(eventFields[5].TryGet(out LocalizedText receivedMessage), Is.True); // Message
             Assert.IsFalse(receivedMessage.IsNull);
             Assert.AreEqual(eventMessage, receivedMessage.Text);
-            eventFields[6].TryGet(out ushort receiveSeverity);
+            Assert.That(eventFields[6].TryGet(out ushort receiveSeverity), Is.True);
             Assert.AreEqual((ushort)EventSeverity.Medium, receiveSeverity); // Severity
 
             // Delete subscription