From de688aca18b632f8a0cf6298bb9e7348de37bbef Mon Sep 17 00:00:00 2001 From: romanett Date: Thu, 19 Dec 2024 17:17:25 +0100 Subject: [PATCH] [Server] Improve GetManagerHandle & introduce a threadSafe NodeIdDictionary (#2915) * Make NodeIdDictionary threadsafe by changing the underlying dict from `Dictionary` to `ConcurrentDictionary` * reduce blocking in CustomNodeManager2 for GetManagerHandle & other uncritical supporting functions --- .../Diagnostics/CustomNodeManager.cs | 700 +++++++++--------- .../Types/BuiltIn/NodeIdDictionary.cs | 35 +- .../CustomNodeManagerTests.cs | 232 ++++++ 3 files changed, 616 insertions(+), 351 deletions(-) create mode 100644 Tests/Opc.Ua.Server.Tests/CustomNodeManagerTests.cs diff --git a/Libraries/Opc.Ua.Server/Diagnostics/CustomNodeManager.cs b/Libraries/Opc.Ua.Server/Diagnostics/CustomNodeManager.cs index fa25395da6..cc74f527c7 100644 --- a/Libraries/Opc.Ua.Server/Diagnostics/CustomNodeManager.cs +++ b/Libraries/Opc.Ua.Server/Diagnostics/CustomNodeManager.cs @@ -1,3 +1,4 @@ + /* ======================================================================== * Copyright (c) 2005-2022 The OPC Foundation, Inc. All rights reserved. * @@ -32,6 +33,7 @@ using System.Collections.Generic; using System.Globalization; using System.Linq; using System.Reflection; +using System.Threading; namespace Opc.Ua.Server @@ -70,12 +72,10 @@ namespace Opc.Ua.Server // set defaults. m_maxQueueSize = 1000; - if (configuration != null) + + if (configuration?.ServerConfiguration != null) { - if (configuration.ServerConfiguration != null) - { - m_maxQueueSize = (uint)configuration.ServerConfiguration.MaxNotificationQueueSize; - } + m_maxQueueSize = (uint)configuration.ServerConfiguration.MaxNotificationQueueSize; } // save a reference to the UA server instance that owns the node manager. @@ -88,23 +88,25 @@ namespace Opc.Ua.Server // the strategy used by a NodeManager depends on what kind of information it provides. m_systemContext.NodeIdFactory = this; - // create the table of namespaces that are used by the NodeManager. - m_namespaceUris = namespaceUris; - // add the uris to the server's namespace table and cache the indexes. + ushort[] namespaceIndexes = Array.Empty(); if (namespaceUris != null) { - m_namespaceIndexes = new ushort[m_namespaceUris.Length]; + namespaceIndexes = new ushort[namespaceUris.Length]; - for (int ii = 0; ii < m_namespaceUris.Length; ii++) + for (int ii = 0; ii < namespaceUris.Length; ii++) { - m_namespaceIndexes[ii] = m_server.NamespaceUris.GetIndexOrAppend(m_namespaceUris[ii]); + namespaceIndexes[ii] = m_server.NamespaceUris.GetIndexOrAppend(namespaceUris[ii]); } } + // add the table of namespaces that are used by the NodeManager. + m_namespaceUris = namespaceUris; + m_namespaceIndexes = namespaceIndexes; + // create the table of monitored nodes. // these are created by the node manager whenever a client subscribe to an attribute of the node. - m_monitoredNodes = new Dictionary(); + m_monitoredNodes = new NodeIdDictionary(); } #endregion @@ -125,17 +127,14 @@ namespace Opc.Ua.Server { if (disposing) { - lock (m_lock) + lock (Lock) { - if (m_predefinedNodes != null) + foreach (NodeState node in m_predefinedNodes?.Values) { - foreach (NodeState node in m_predefinedNodes.Values) - { - Utils.SilentDispose(node); - } - - m_predefinedNodes.Clear(); + Utils.SilentDispose(node); } + + m_predefinedNodes.Clear(); } } } @@ -191,7 +190,7 @@ namespace Opc.Ua.Server /// Gets the namespace indexes owned by the node manager. /// /// The namespace indexes. - public ushort[] NamespaceIndexes + public IReadOnlyList NamespaceIndexes { get { return m_namespaceIndexes; } } @@ -220,10 +219,7 @@ namespace Opc.Ua.Server /// /// The predefined nodes managed by the node manager. /// - protected NodeIdDictionary PredefinedNodes - { - get { return m_predefinedNodes; } - } + protected NodeIdDictionary PredefinedNodes => m_predefinedNodes; /// /// The root notifiers for the node manager. @@ -236,10 +232,7 @@ namespace Opc.Ua.Server /// /// Gets the table of nodes being monitored. /// - protected Dictionary MonitoredNodes - { - get { return m_monitoredNodes; } - } + protected NodeIdDictionary MonitoredNodes => m_monitoredNodes; /// /// Sets the namespaces supported by the NodeManager. @@ -247,16 +240,17 @@ namespace Opc.Ua.Server /// The namespace uris. protected void SetNamespaces(params string[] namespaceUris) { - // create the table of namespaces that are used by the NodeManager. - m_namespaceUris = namespaceUris; - // add the uris to the server's namespace table and cache the indexes. - m_namespaceIndexes = new ushort[m_namespaceUris.Length]; + var namespaceIndexes = new ushort[namespaceUris.Length]; - for (int ii = 0; ii < m_namespaceUris.Length; ii++) + for (int ii = 0; ii < namespaceUris.Length; ii++) { - m_namespaceIndexes[ii] = m_server.NamespaceUris.GetIndexOrAppend(m_namespaceUris[ii]); + namespaceIndexes[ii] = m_server.NamespaceUris.GetIndexOrAppend(namespaceUris[ii]); } + + // create the immutable table of namespaces that are used by the NodeManager. + m_namespaceUris = namespaceUris; + m_namespaceIndexes = namespaceIndexes; } /// @@ -264,18 +258,24 @@ namespace Opc.Ua.Server /// protected void SetNamespaceIndexes(ushort[] namespaceIndexes) { - m_namespaceIndexes = namespaceIndexes; - m_namespaceUris = new string[namespaceIndexes.Length]; + var namespaceUris = new string[namespaceIndexes.Length]; for (int ii = 0; ii < namespaceIndexes.Length; ii++) { - m_namespaceUris[ii] = m_server.NamespaceUris.GetString(namespaceIndexes[ii]); + namespaceUris[ii] = m_server.NamespaceUris.GetString(namespaceIndexes[ii]); } + + // create the immutable table of namespaces that are used by the NodeManager. + m_namespaceUris = namespaceUris; + m_namespaceIndexes = namespaceIndexes; } /// /// Returns true if the namespace for the node id is one of the namespaces managed by the node manager. /// + /// + /// It is thread safe to call this method outside the node manager lock. + /// /// The node id to check. /// True if the namespace is one of the nodes. protected virtual bool IsNodeIdInNamespace(NodeId nodeId) @@ -287,20 +287,15 @@ namespace Opc.Ua.Server } // quickly exclude nodes that not in the namespace. - for (int ii = 0; ii < m_namespaceIndexes.Length; ii++) - { - if (nodeId.NamespaceIndex == m_namespaceIndexes[ii]) - { - return true; - } - } - - return false; + return m_namespaceIndexes.Contains(nodeId.NamespaceIndex); } /// /// Returns the node if the handle refers to a node managed by this manager. /// + /// + /// It is thread safe to call this method outside the node manager lock. + /// /// The handle to check. /// Non-null if the handle belongs to the node manager. protected virtual NodeHandle IsHandleInNamespace(object managerHandle) @@ -325,22 +320,13 @@ namespace Opc.Ua.Server /// public NodeState Find(NodeId nodeId) { - lock (Lock) + NodeState node = null; + if (m_predefinedNodes?.TryGetValue(nodeId, out node) == true) { - if (PredefinedNodes == null) - { - return null; - } - - NodeState node = null; - - if (!PredefinedNodes.TryGetValue(nodeId, out node)) - { - return null; - } - return node; } + + return null; } /// @@ -359,7 +345,7 @@ namespace Opc.Ua.Server QualifiedName browseName, BaseInstanceState instance) { - ServerSystemContext contextToUse = (ServerSystemContext)m_systemContext.Copy(context); + ServerSystemContext contextToUse = m_systemContext.Copy(context); lock (Lock) { @@ -401,34 +387,23 @@ namespace Opc.Ua.Server { ServerSystemContext contextToUse = m_systemContext.Copy(context); - bool found = false; List referencesToRemove = new List(); - lock (Lock) + NodeState node = null; + if (m_predefinedNodes?.TryGetValue(nodeId, out node) != true) { - if (m_predefinedNodes == null) - { - return false; - } - - NodeState node = null; - - if (PredefinedNodes.TryGetValue(nodeId, out node)) - { - RemovePredefinedNode(contextToUse, node, referencesToRemove); - found = true; - } - - RemoveRootNotifier(node); + return false; } - // must release the lock before removing cross references to other node managers. + RemovePredefinedNode(contextToUse, node, referencesToRemove); + RemoveRootNotifier(node); + if (referencesToRemove.Count > 0) { Server.NodeManager.RemoveReferences(referencesToRemove); } - return found; + return true; } /// @@ -586,7 +561,7 @@ namespace Opc.Ua.Server } NodeState activeNode = AddBehaviourToPredefinedNode(context, node); - m_predefinedNodes[activeNode.NodeId] = activeNode; + m_predefinedNodes.AddOrUpdate(activeNode.NodeId, activeNode, (key, _) => activeNode); BaseTypeState type = activeNode as BaseTypeState; @@ -594,28 +569,30 @@ namespace Opc.Ua.Server { AddTypesToTypeTree(type); } - - // update the root notifiers. - if (m_rootNotifiers != null) + lock (Lock) { - for (int ii = 0; ii < m_rootNotifiers.Count; ii++) + // update the root notifiers. + if (m_rootNotifiers != null) { - if (m_rootNotifiers[ii].NodeId == activeNode.NodeId) + for (int ii = 0; ii < m_rootNotifiers.Count; ii++) { - m_rootNotifiers[ii] = activeNode; - - // need to prevent recursion with the server object. - if (activeNode.NodeId != ObjectIds.Server) + if (m_rootNotifiers[ii].NodeId == activeNode.NodeId) { - activeNode.OnReportEvent = OnReportEvent; + m_rootNotifiers[ii] = activeNode; - if (!activeNode.ReferenceExists(ReferenceTypeIds.HasNotifier, true, ObjectIds.Server)) + // need to prevent recursion with the server object. + if (activeNode.NodeId != ObjectIds.Server) { - activeNode.AddReference(ReferenceTypeIds.HasNotifier, true, ObjectIds.Server); - } - } + activeNode.OnReportEvent = OnReportEvent; - break; + if (!activeNode.ReferenceExists(ReferenceTypeIds.HasNotifier, true, ObjectIds.Server)) + { + activeNode.AddReference(ReferenceTypeIds.HasNotifier, true, ObjectIds.Server); + } + } + + break; + } } } } @@ -637,20 +614,16 @@ namespace Opc.Ua.Server NodeState node, List referencesToRemove) { - if (m_predefinedNodes == null) + if (m_predefinedNodes?.TryRemove(node.NodeId, out _) != true) { return; } - - m_predefinedNodes.Remove(node.NodeId); node.UpdateChangeMasks(NodeStateChangeMasks.Deleted); node.ClearChangeMasks(context, false); OnNodeRemoved(node); // remove from the parent. - BaseInstanceState instance = node as BaseInstanceState; - - if (instance != null && instance.Parent != null) + if (node is BaseInstanceState instance && instance.Parent != null) { instance.Parent.RemoveChild(instance); } @@ -714,14 +687,8 @@ namespace Opc.Ua.Server /// A list of references to add to external targets. protected virtual void AddReverseReferences(IDictionary> externalReferences) { - if (m_predefinedNodes == null) + foreach (NodeState source in m_predefinedNodes?.Values) { - return; - } - - foreach (NodeState source in m_predefinedNodes.Values) - { - IList references = new List(); source.GetReferences(SystemContext, references); @@ -804,11 +771,11 @@ namespace Opc.Ua.Server } // add reserve reference from external node. - ReferenceNode referenceToAdd = new ReferenceNode(); - - referenceToAdd.ReferenceTypeId = referenceTypeId; - referenceToAdd.IsInverse = isInverse; - referenceToAdd.TargetId = targetId; + ReferenceNode referenceToAdd = new ReferenceNode { + ReferenceTypeId = referenceTypeId, + IsInverse = isInverse, + TargetId = targetId + }; referencesToAdd.Add(referenceToAdd); } @@ -841,16 +808,13 @@ namespace Opc.Ua.Server /// protected void AddTypesToTypeTree(NodeId typeId) { - NodeState node = null; - - if (!PredefinedNodes.TryGetValue(typeId, out node)) + if (!m_predefinedNodes.TryGetValue(typeId, out NodeState node)) { return; } - BaseTypeState type = node as BaseTypeState; - if (type == null) + if (!(node is BaseTypeState type)) { return; } @@ -869,9 +833,7 @@ namespace Opc.Ua.Server return null; } - NodeState node = null; - - if (!PredefinedNodes.TryGetValue(nodeId, out node)) + if (!m_predefinedNodes.TryGetValue(nodeId, out NodeState node)) { return null; } @@ -893,16 +855,14 @@ namespace Opc.Ua.Server /// public virtual void DeleteAddressSpace() { - lock (m_lock) + if (m_predefinedNodes != null) { - if (m_predefinedNodes != null) - { - foreach (NodeState node in m_predefinedNodes.Values) - { - Utils.SilentDispose(node); - } + var nodes = m_predefinedNodes.Values.ToArray(); + m_predefinedNodes.Clear(); - m_predefinedNodes.Clear(); + foreach (NodeState node in nodes) + { + Utils.SilentDispose(node); } } } @@ -917,15 +877,15 @@ namespace Opc.Ua.Server /// public virtual object GetManagerHandle(NodeId nodeId) { - lock (m_lock) - { - return GetManagerHandle(m_systemContext, nodeId, null); - } + return GetManagerHandle(m_systemContext, nodeId, null); } /// /// Returns a unique handle for the node. /// + /// + /// It is thread safe to call this method outside the node manager lock. + /// protected virtual NodeHandle GetManagerHandle(ServerSystemContext context, NodeId nodeId, IDictionary cache) { if (!IsNodeIdInNamespace(nodeId)) @@ -934,18 +894,16 @@ namespace Opc.Ua.Server } NodeState node = null; - if (m_predefinedNodes?.TryGetValue(nodeId, out node) == true) { - NodeHandle handle = new NodeHandle(); - - handle.NodeId = nodeId; - handle.Node = node; - handle.Validated = true; + var handle = new NodeHandle { + NodeId = nodeId, + Node = node, + Validated = true + }; return handle; } - return null; } @@ -965,11 +923,12 @@ namespace Opc.Ua.Server NodeHandle source = GetManagerHandle(m_systemContext, current.Key, null); // only support external references to nodes that are stored in memory. - if (source == null || !source.Validated || source.Node == null) + if (source?.Node == null || !source.Validated) { continue; } + // add reference to external target. foreach (IReference reference in current.Value) { @@ -992,22 +951,22 @@ namespace Opc.Ua.Server ExpandedNodeId targetId, bool deleteBidirectional) { + // get the handle. + NodeHandle source = IsHandleInNamespace(sourceHandle); + + if (source == null) + { + return StatusCodes.BadNodeIdUnknown; + } + + // only support external references to nodes that are stored in memory. + if (!source.Validated || source.Node == null) + { + return StatusCodes.BadNotSupported; + } + lock (Lock) { - // get the handle. - NodeHandle source = IsHandleInNamespace(sourceHandle); - - if (source == null) - { - return StatusCodes.BadNodeIdUnknown; - } - - // only support external references to nodes that are stored in memory. - if (!source.Validated || source.Node == null) - { - return StatusCodes.BadNotSupported; - } - // only support references to Source Areas. source.Node.RemoveReference(referenceTypeId, isInverse, targetId); @@ -1042,16 +1001,16 @@ namespace Opc.Ua.Server { ServerSystemContext systemContext = m_systemContext.Copy(context); + // check for valid handle. + NodeHandle handle = IsHandleInNamespace(targetHandle); + + if (handle == null) + { + return null; + } + lock (Lock) { - // check for valid handle. - NodeHandle handle = IsHandleInNamespace(targetHandle); - - if (handle == null) - { - return null; - } - // validate node. NodeState target = ValidateNode(systemContext, handle, null); @@ -1078,11 +1037,11 @@ namespace Opc.Ua.Server Attributes.UserRolePermissions); // construct the meta-data object. - NodeMetadata metadata = new NodeMetadata(target, target.NodeId); - - metadata.NodeClass = target.NodeClass; - metadata.BrowseName = target.BrowseName; - metadata.DisplayName = target.DisplayName; + NodeMetadata metadata = new NodeMetadata(target, target.NodeId) { + NodeClass = target.NodeClass, + BrowseName = target.BrowseName, + DisplayName = target.DisplayName + }; if (values[0] != null && values[1] != null) { @@ -1131,9 +1090,7 @@ namespace Opc.Ua.Server SetDefaultPermissions(systemContext, target, metadata); // get instance references. - BaseInstanceState instance = target as BaseInstanceState; - - if (instance != null) + if (target is BaseInstanceState instance) { metadata.TypeDefinition = instance.TypeDefinitionId; metadata.ModellingRule = instance.ModellingRuleId; @@ -1222,16 +1179,15 @@ namespace Opc.Ua.Server INodeBrowser browser = null; + // check for valid handle. + NodeHandle handle = IsHandleInNamespace(continuationPoint.NodeToBrowse); + + if (handle == null) + { + throw new ServiceResultException(StatusCodes.BadNodeIdUnknown); + } lock (Lock) { - // check for valid handle. - NodeHandle handle = IsHandleInNamespace(continuationPoint.NodeToBrowse); - - if (handle == null) - { - throw new ServiceResultException(StatusCodes.BadNodeIdUnknown); - } - // validate node. NodeState source = ValidateNode(systemContext, handle, null); @@ -1487,16 +1443,16 @@ namespace Opc.Ua.Server ServerSystemContext systemContext = m_systemContext.Copy(context); IDictionary operationCache = new NodeIdDictionary(); + // check for valid handle. + NodeHandle handle = IsHandleInNamespace(sourceHandle); + + if (handle == null) + { + return; + } + lock (Lock) { - // check for valid handle. - NodeHandle handle = IsHandleInNamespace(sourceHandle); - - if (handle == null) - { - return; - } - // validate node. NodeState source = ValidateNode(systemContext, handle, operationCache); @@ -2945,16 +2901,16 @@ namespace Opc.Ua.Server MethodState method = null; + // check for valid handle. + NodeHandle handle = GetManagerHandle(systemContext, methodToCall.ObjectId, operationCache); + + if (handle == null) + { + continue; + } + lock (Lock) { - // check for valid handle. - NodeHandle handle = GetManagerHandle(systemContext, methodToCall.ObjectId, operationCache); - - if (handle == null) - { - continue; - } - // owned by this node manager. methodToCall.Processed = true; @@ -3106,16 +3062,16 @@ namespace Opc.Ua.Server { ServerSystemContext systemContext = SystemContext.Copy(context); + // check for valid handle. + NodeHandle handle = IsHandleInNamespace(sourceId); + + if (handle == null) + { + return StatusCodes.BadNodeIdInvalid; + } + lock (Lock) { - // check for valid handle. - NodeHandle handle = IsHandleInNamespace(sourceId); - - if (handle == null) - { - return StatusCodes.BadNodeIdInvalid; - } - // check for valid node. NodeState source = ValidateNode(systemContext, handle, null); @@ -3172,58 +3128,61 @@ namespace Opc.Ua.Server /// protected virtual void AddRootNotifier(NodeState notifier) { - if (m_rootNotifiers == null) + lock (Lock) { - m_rootNotifiers = new List(); - } - - bool mustAdd = true; - - for (int ii = 0; ii < m_rootNotifiers.Count; ii++) - { - if (Object.ReferenceEquals(notifier, m_rootNotifiers[ii])) + if (m_rootNotifiers == null) { - return; + m_rootNotifiers = new List(); } - if (m_rootNotifiers[ii].NodeId == notifier.NodeId) + bool mustAdd = true; + + for (int ii = 0; ii < m_rootNotifiers.Count; ii++) { - m_rootNotifiers[ii] = notifier; - mustAdd = false; - break; - } - } - - if (mustAdd) - { - m_rootNotifiers.Add(notifier); - } - - // need to prevent recursion with the server object. - if (notifier.NodeId != ObjectIds.Server) - { - notifier.OnReportEvent = OnReportEvent; - - if (!notifier.ReferenceExists(ReferenceTypeIds.HasNotifier, true, ObjectIds.Server)) - { - notifier.AddReference(ReferenceTypeIds.HasNotifier, true, ObjectIds.Server); - } - } - - // subscribe to existing events. - if (m_server.EventManager != null) - { - IList monitoredItems = m_server.EventManager.GetMonitoredItems(); - - for (int ii = 0; ii < monitoredItems.Count; ii++) - { - if (monitoredItems[ii].MonitoringAllEvents) + if (Object.ReferenceEquals(notifier, m_rootNotifiers[ii])) { - SubscribeToEvents( + return; + } + + if (m_rootNotifiers[ii].NodeId == notifier.NodeId) + { + m_rootNotifiers[ii] = notifier; + mustAdd = false; + break; + } + } + + if (mustAdd) + { + m_rootNotifiers.Add(notifier); + } + + // need to prevent recursion with the server object. + if (notifier.NodeId != ObjectIds.Server) + { + notifier.OnReportEvent = OnReportEvent; + + if (!notifier.ReferenceExists(ReferenceTypeIds.HasNotifier, true, ObjectIds.Server)) + { + notifier.AddReference(ReferenceTypeIds.HasNotifier, true, ObjectIds.Server); + } + } + + // subscribe to existing events. + if (m_server.EventManager != null) + { + IList monitoredItems = m_server.EventManager.GetMonitoredItems(); + + for (int ii = 0; ii < monitoredItems.Count; ii++) + { + if (monitoredItems[ii].MonitoringAllEvents) + { + SubscribeToEvents( SystemContext, notifier, monitoredItems[ii], true); + } } } } @@ -3235,16 +3194,19 @@ namespace Opc.Ua.Server /// The notifier. protected virtual void RemoveRootNotifier(NodeState notifier) { - if (m_rootNotifiers != null) + lock (Lock) { - for (int ii = 0; ii < m_rootNotifiers.Count; ii++) + if (m_rootNotifiers != null) { - if (Object.ReferenceEquals(notifier, m_rootNotifiers[ii])) + for (int ii = 0; ii < m_rootNotifiers.Count; ii++) { - notifier.OnReportEvent = null; - notifier.RemoveReference(ReferenceTypeIds.HasNotifier, true, ObjectIds.Server); - m_rootNotifiers.RemoveAt(ii); - break; + if (Object.ReferenceEquals(notifier, m_rootNotifiers[ii])) + { + notifier.OnReportEvent = null; + notifier.RemoveReference(ReferenceTypeIds.HasNotifier, true, ObjectIds.Server); + m_rootNotifiers.RemoveAt(ii); + break; + } } } } @@ -3305,13 +3267,9 @@ namespace Opc.Ua.Server } // only objects or views can be subscribed to. - BaseObjectState instance = source as BaseObjectState; - - if (instance == null || (instance.EventNotifier & EventNotifiers.SubscribeToEvents) == 0) + if (!(source is BaseObjectState instance) || (instance.EventNotifier & EventNotifiers.SubscribeToEvents) == 0) { - ViewState view = source as ViewState; - - if (view == null || (view.EventNotifier & EventNotifiers.SubscribeToEvents) == 0) + if (!(source is ViewState view) || (view.EventNotifier & EventNotifiers.SubscribeToEvents) == 0) { return StatusCodes.BadNotSupported; } @@ -3457,43 +3415,41 @@ namespace Opc.Ua.Server List nodesToValidate = new List(); List createdItems = new List(); - lock (Lock) + + for (int ii = 0; ii < itemsToCreate.Count; ii++) { - for (int ii = 0; ii < itemsToCreate.Count; ii++) + MonitoredItemCreateRequest itemToCreate = itemsToCreate[ii]; + + // skip items that have already been processed. + if (itemToCreate.Processed) { - MonitoredItemCreateRequest itemToCreate = itemsToCreate[ii]; - - // skip items that have already been processed. - if (itemToCreate.Processed) - { - continue; - } - - ReadValueId itemToMonitor = itemToCreate.ItemToMonitor; - - // check for valid handle. - NodeHandle handle = GetManagerHandle(systemContext, itemToMonitor.NodeId, operationCache); - - if (handle == null) - { - continue; - } - - // owned by this node manager. - itemToCreate.Processed = true; - - // must validate node in a separate operation. - errors[ii] = StatusCodes.BadNodeIdUnknown; - - handle.Index = ii; - nodesToValidate.Add(handle); + continue; } - // check for nothing to do. - if (nodesToValidate.Count == 0) + ReadValueId itemToMonitor = itemToCreate.ItemToMonitor; + + // check for valid handle. + NodeHandle handle = GetManagerHandle(systemContext, itemToMonitor.NodeId, operationCache); + + if (handle == null) { - return; + continue; } + + // owned by this node manager. + itemToCreate.Processed = true; + + // must validate node in a separate operation. + errors[ii] = StatusCodes.BadNodeIdUnknown; + + handle.Index = ii; + nodesToValidate.Add(handle); + } + + // check for nothing to do. + if (nodesToValidate.Count == 0) + { + return; } // validates the nodes (reads values from the underlying data source if required). @@ -3986,28 +3942,44 @@ namespace Opc.Ua.Server IList filterErrors) { ServerSystemContext systemContext = m_systemContext.Copy(context); - List modifiedItems = new List(); + var nodesInNamespace = new List<(int, NodeHandle)>(monitoredItems.Count); + + for (int ii = 0; ii < monitoredItems.Count; ii++) + { + MonitoredItemModifyRequest itemToModify = itemsToModify[ii]; + + // skip items that have already been processed. + if (itemToModify.Processed || monitoredItems[ii] == null) + { + continue; + } + + // check handle. + NodeHandle handle = IsHandleInNamespace(monitoredItems[ii].ManagerHandle); + + if (handle == null) + { + continue; + } + + nodesInNamespace.Add((ii, handle)); + } + + if (nodesInNamespace.Count == 0) + { + return; + } + + var modifiedItems = new List(); lock (Lock) { - for (int ii = 0; ii < monitoredItems.Count; ii++) + foreach (var nodeInNamespace in nodesInNamespace) { + int ii = nodeInNamespace.Item1; + var handle = nodeInNamespace.Item2; MonitoredItemModifyRequest itemToModify = itemsToModify[ii]; - // skip items that have already been processed. - if (itemToModify.Processed || monitoredItems[ii] == null) - { - continue; - } - - // check handle. - NodeHandle handle = IsHandleInNamespace(monitoredItems[ii].ManagerHandle); - - if (handle == null) - { - continue; - } - // owned by this node manager. itemToModify.Processed = true; @@ -4168,25 +4140,40 @@ namespace Opc.Ua.Server IList errors) { ServerSystemContext systemContext = m_systemContext.Copy(context); - List deletedItems = new List(); + var nodesInNamespace = new List<(int, NodeHandle)>(monitoredItems.Count); + + for (int ii = 0; ii < monitoredItems.Count; ii++) + { + // skip items that have already been processed. + if (processedItems[ii] || monitoredItems[ii] == null) + { + continue; + } + + // check handle. + NodeHandle handle = IsHandleInNamespace(monitoredItems[ii].ManagerHandle); + + if (handle == null) + { + continue; + } + + nodesInNamespace.Add((ii, handle)); + } + + if (nodesInNamespace.Count == 0) + { + return; + } + + var deletedItems = new List(); lock (Lock) { - for (int ii = 0; ii < monitoredItems.Count; ii++) + foreach (var nodeInNamespace in nodesInNamespace) { - // skip items that have already been processed. - if (processedItems[ii] || monitoredItems[ii] == null) - { - continue; - } - - // check handle. - NodeHandle handle = IsHandleInNamespace(monitoredItems[ii].ManagerHandle); - - if (handle == null) - { - continue; - } + int ii = nodeInNamespace.Item1; + var handle = nodeInNamespace.Item2; // owned by this node manager. processedItems[ii] = true; @@ -4239,7 +4226,7 @@ namespace Opc.Ua.Server // check if node is no longer being monitored. if (!monitoredNode.HasMonitoredItems) { - MonitoredNodes.Remove(handle.NodeId); + m_monitoredNodes.Remove(handle.NodeId); } } @@ -4344,25 +4331,40 @@ namespace Opc.Ua.Server IList errors) { ServerSystemContext systemContext = m_systemContext.Copy(context); - List changedItems = new List(); + var nodesInNamespace = new List<(int, NodeHandle)>(monitoredItems.Count); + + for (int ii = 0; ii < monitoredItems.Count; ii++) + { + // skip items that have already been processed. + if (processedItems[ii] || monitoredItems[ii] == null) + { + continue; + } + + // check handle. + NodeHandle handle = IsHandleInNamespace(monitoredItems[ii].ManagerHandle); + + if (handle == null) + { + continue; + } + + nodesInNamespace.Add((ii, handle)); + } + + if (nodesInNamespace.Count == 0) + { + return; + } + + var changedItems = new List(); lock (Lock) { - for (int ii = 0; ii < monitoredItems.Count; ii++) + foreach (var nodeInNamespace in nodesInNamespace) { - // skip items that have already been processed. - if (processedItems[ii] || monitoredItems[ii] == null) - { - continue; - } - - // check handle. - NodeHandle handle = IsHandleInNamespace(monitoredItems[ii].ManagerHandle); - - if (handle == null) - { - continue; - } + int ii = nodeInNamespace.Item1; + var handle = nodeInNamespace.Item2; // indicate whether it was processed or not. processedItems[ii] = true; @@ -4494,16 +4496,16 @@ namespace Opc.Ua.Server { ServerSystemContext systemContext = m_systemContext.Copy(context); + // check for valid handle. + NodeHandle handle = IsHandleInNamespace(targetHandle); + + if (handle == null) + { + return null; + } + lock (Lock) { - // check for valid handle. - NodeHandle handle = IsHandleInNamespace(targetHandle); - - if (handle == null) - { - return null; - } - // validate node. NodeState target = ValidateNode(systemContext, handle, null); @@ -4699,7 +4701,7 @@ namespace Opc.Ua.Server if (m_componentCache == null) { - m_componentCache = new Dictionary(); + m_componentCache = new NodeIdDictionary(); } // check if a component is actually specified. @@ -4756,10 +4758,10 @@ namespace Opc.Ua.Server private readonly object m_lock = new object(); private IServerInternal m_server; private ServerSystemContext m_systemContext; - private string[] m_namespaceUris; - private ushort[] m_namespaceIndexes; - private Dictionary m_monitoredNodes; - private Dictionary m_componentCache; + private IReadOnlyList m_namespaceUris; + private IReadOnlyList m_namespaceIndexes; + private NodeIdDictionary m_monitoredNodes; + private NodeIdDictionary m_componentCache; private NodeIdDictionary m_predefinedNodes; private List m_rootNotifiers; private uint m_maxQueueSize; diff --git a/Stack/Opc.Ua.Core/Types/BuiltIn/NodeIdDictionary.cs b/Stack/Opc.Ua.Core/Types/BuiltIn/NodeIdDictionary.cs index 87aa08341f..ef0f1c94f8 100644 --- a/Stack/Opc.Ua.Core/Types/BuiltIn/NodeIdDictionary.cs +++ b/Stack/Opc.Ua.Core/Types/BuiltIn/NodeIdDictionary.cs @@ -19,6 +19,7 @@ using System; using System.Collections; +using System.Collections.Concurrent; using System.Collections.Generic; namespace Opc.Ua @@ -27,7 +28,7 @@ namespace Opc.Ua /// /// A dictionary designed to provide efficient lookups for objects identified by a NodeId /// - public class NodeIdDictionary : Dictionary + public sealed class NodeIdDictionary : ConcurrentDictionary { private static readonly NodeIdComparer s_comparer = new NodeIdComparer(); @@ -41,9 +42,39 @@ namespace Opc.Ua /// /// Creates an empty dictionary with capacity. /// - public NodeIdDictionary(int capacity) : base(capacity, s_comparer) + public NodeIdDictionary(int capacity) : base(Environment.ProcessorCount, capacity, s_comparer) { } + + // helpers for the legacy implementation + + /// + public void Add(NodeId key, T value) + { + if (!TryAdd(key, value)) + { + throw new ArgumentException("An element with the same key already exists."); + } + } + + /// + public void Remove(NodeId key) + { + TryRemove(key, out _); + } + + /// + /// remove a entry from the dictionary only if it has the provided value + /// https://devblogs.microsoft.com/pfxteam/little-known-gems-atomic-conditional-removals-from-concurrentdictionary/ + /// + /// the key of the entry to remove + /// the value of the entry to remove + /// true if removed, false if not removed + public bool TryRemove(NodeId key, T value) + { + return ((ICollection>)this).Remove( + new KeyValuePair(key, value)); + } } #else // USE_LEGACY_IMPLEMENTATION diff --git a/Tests/Opc.Ua.Server.Tests/CustomNodeManagerTests.cs b/Tests/Opc.Ua.Server.Tests/CustomNodeManagerTests.cs new file mode 100644 index 0000000000..9565861c5a --- /dev/null +++ b/Tests/Opc.Ua.Server.Tests/CustomNodeManagerTests.cs @@ -0,0 +1,232 @@ +using System; +using System.Linq; +using System.Threading; +using System.Threading.Tasks; +using NUnit.Framework; + +namespace Opc.Ua.Server.Tests +{ + /// + /// Test + /// + [TestFixture, Category("CustomNodeManager")] + [SetCulture("en-us"), SetUICulture("en-us")] + [Parallelizable] + public class CustomNodeManagerTests + { + #region Test Methods + /// + /// Tests the componentCache methods with multiple threads + /// + [Test] + public async Task TestComponentCacheAsync() + { + var fixture = new ServerFixture(); + + try + { + // Arrange + const string ns = "http://test.org/UA/Data/"; + var server = await fixture.StartAsync(TestContext.Out).ConfigureAwait(false); + + var nodeManager = new TestableCustomNodeManger2(server.CurrentInstance, ns); + + + var baseObject = new BaseObjectState(null); + var nodeHandle = new NodeHandle(new NodeId((string)CommonTestWorkers.NodeIdTestSetStatic.First().Identifier, 0), baseObject); + + //Act + await RunTaskInParallel(() => UseComponentCacheAsync(nodeManager, baseObject, nodeHandle), 100); + + + //Assert, that entry was deleted from cache after parallel operations on the same node + NodeState handleFromCache = nodeManager.LookupNodeInComponentCache(nodeManager.SystemContext, nodeHandle); + + Assert.That(handleFromCache, Is.Null); + } + finally + { + await fixture.StopAsync().ConfigureAwait(false); + } + } + + /// + /// Tests the Predefined Nodes methods with multiple threads + /// + [Test] + public async Task TestPredefinedNodes() + { + var fixture = new ServerFixture(); + + try + { + // Arrange + const string ns = "http://test.org/UA/Data/"; + var server = await fixture.StartAsync(TestContext.Out).ConfigureAwait(false); + + var nodeManager = new TestableCustomNodeManger2(server.CurrentInstance, ns); + var index = server.CurrentInstance.NamespaceUris.GetIndex(ns); + + var baseObject = new DataItemState(null); + var nodeId = new NodeId((string)CommonTestWorkers.NodeIdTestSetStatic.First().Identifier, (ushort)index); + + baseObject.NodeId = nodeId; + + + //single threaded test + nodeManager.AddPredefinedNode(nodeManager.SystemContext, baseObject); + + Assert.That(nodeManager.PredefinedNodes.ContainsKey(nodeId), Is.True); + + NodeState nodeState = nodeManager.Find(nodeId); + Assert.That(nodeState, Is.Not.Null); + + NodeHandle handle = nodeManager.GetManagerHandle(nodeId) as NodeHandle; + Assert.That(handle, Is.Not.Null); + + nodeManager.DeleteNode(nodeManager.SystemContext, nodeId); + + Assert.That(nodeManager.PredefinedNodes, Is.Empty); + + nodeState = nodeManager.Find(nodeId); + Assert.That(nodeState, Is.Null); + + handle = nodeManager.GetManagerHandle(nodeId) as NodeHandle; + Assert.That(handle, Is.Null); + + nodeManager.AddPredefinedNode(nodeManager.SystemContext, baseObject); + + nodeManager.DeleteAddressSpace(); + + Assert.That(nodeManager.PredefinedNodes, Is.Empty); + + + //Act + await RunTaskInParallel(() => UsePredefinedNodesAsync(nodeManager, baseObject, nodeId), 100); + + //last operation added the Node back into the dictionary + Assert.That(nodeManager.PredefinedNodes.ContainsKey(nodeId), Is.True); + + //delete full adress space + nodeManager.DeleteAddressSpace(); + Assert.That(nodeManager.PredefinedNodes, Is.Empty); + } + finally + { + await fixture.StopAsync().ConfigureAwait(false); + } + } + + private static async Task UsePredefinedNodesAsync(TestableCustomNodeManger2 nodeManager, DataItemState baseObject, NodeId nodeId) + { + nodeManager.AddPredefinedNode(nodeManager.SystemContext, baseObject); + + NodeState nodeState = nodeManager.Find(nodeId); + + NodeHandle handle = nodeManager.GetManagerHandle(nodeId) as NodeHandle; + + nodeManager.DeleteNode(nodeManager.SystemContext, nodeId); + + nodeState = nodeManager.Find(nodeId); + + handle = nodeManager.GetManagerHandle(nodeId) as NodeHandle; + + nodeManager.AddPredefinedNode(nodeManager.SystemContext, baseObject); + await Task.CompletedTask; + } + + /// + /// Test Methods AddNodeToComponentCache, RemoveNodeFromComponentCache & LookupNodeInComponentCache & verify the node is added to the cache + /// + /// + private static async Task UseComponentCacheAsync(TestableCustomNodeManger2 nodeManager, BaseObjectState baseObject, NodeHandle nodeHandle) + { + //-- Act + nodeManager.AddNodeToComponentCache(nodeManager.SystemContext, nodeHandle, baseObject); + nodeManager.AddNodeToComponentCache(nodeManager.SystemContext, nodeHandle, baseObject); + + NodeState handleFromCache = nodeManager.LookupNodeInComponentCache(nodeManager.SystemContext, nodeHandle); + + //-- Assert + + Assert.That(handleFromCache, Is.Not.Null); + + nodeManager.RemoveNodeFromComponentCache(nodeManager.SystemContext, nodeHandle); + + handleFromCache = nodeManager.LookupNodeInComponentCache(nodeManager.SystemContext, nodeHandle); + + Assert.That(handleFromCache, Is.Not.Null); + + nodeManager.RemoveNodeFromComponentCache(nodeManager.SystemContext, nodeHandle); + + await Task.CompletedTask; + } + #endregion + + public static async Task<(bool IsSuccess, Exception Error)> RunTaskInParallel(Func task, int iterations) + { + var cancellationTokenSource = new CancellationTokenSource(); + Exception error = null; + int tasksCompletedCount = 0; + var result = Parallel.For(0, iterations, new ParallelOptions(), + async index => { + try + { + await task(); + } + catch (Exception ex) + { + error = ex; + cancellationTokenSource.Cancel(); + } + finally + { + tasksCompletedCount++; + } + + }); + + int spinWaitCount = 0; + int maxSpinWaitCount = 100; + while (iterations > tasksCompletedCount && error is null && spinWaitCount < maxSpinWaitCount) + { + await Task.Delay(TimeSpan.FromMilliseconds(100)); + spinWaitCount++; + } + + return (error == null, error); + } + + } + + public class TestableCustomNodeManger2 : CustomNodeManager2 + { + public TestableCustomNodeManger2(IServerInternal server, params string[] namespaceUris) : base(server, namespaceUris) + { } + + #region componentCache + public new NodeState AddNodeToComponentCache(ISystemContext context, NodeHandle handle, NodeState node) + { + return base.AddNodeToComponentCache(context, handle, node); + } + public new void RemoveNodeFromComponentCache(ISystemContext context, NodeHandle handle) + { + base.RemoveNodeFromComponentCache(context, handle); + } + public new NodeState LookupNodeInComponentCache(ISystemContext context, NodeHandle handle) + { + return base.LookupNodeInComponentCache(context, handle); + } + #endregion + + #region PredefinedNodes + + public new NodeIdDictionary PredefinedNodes => base.PredefinedNodes; + public new virtual void AddPredefinedNode(ISystemContext context, NodeState node) + { + base.AddPredefinedNode(context, node); + } + + #endregion + } +}