Browse Source

remove the pointer monitoreditem->node, release the node only after the pointer is no longer used.

Julius Pfrommer 9 years ago
parent
commit
2737bec03f
3 changed files with 24 additions and 29 deletions
  1. 2 4
      src/server/ua_services_subscription.c
  2. 22 20
      src/server/ua_subscription.c
  3. 0 5
      src/server/ua_subscription.h

+ 2 - 4
src/server/ua_services_subscription.c

@@ -82,13 +82,11 @@ void Service_CreateMonitoredItems(UA_Server *server, UA_Session *session,
             thisItemsResult->monitoredItemId = 0;
             thisItemsResult->revisedSamplingInterval = 0;
             thisItemsResult->revisedQueueSize = 0;
-        }
-        else {
+        } else {
             thisItemsResult->statusCode = UA_STATUSCODE_GOOD;
             
             newMon = UA_MonitoredItem_new();
-            newMon->monitoredNode = target;
-            UA_NodeId_copy(&(newMon->monitoredNode->nodeId), &(newMon->monitoredNodeId));
+            UA_NodeId_copy(&target->nodeId, &newMon->monitoredNodeId);
             newMon->ItemId = ++(session->subscriptionManager.LastSessionID);
             thisItemsResult->monitoredItemId = newMon->ItemId;
             

+ 22 - 20
src/server/ua_subscription.c

@@ -236,7 +236,7 @@ UA_MonitoredItem *UA_MonitoredItem_new() {
     
     LIST_INIT(&new->queue);
     LIST_INITENTRY(new, listEntry);
-    INITPOINTER(new->monitoredNode);
+    UA_NodeId_init(&new->monitoredNodeId);
     INITPOINTER(new->LastSampledValue.data );
     return new;
 }
@@ -403,21 +403,10 @@ void MonitoredItem_QueuePushDataValue(UA_Server *server, UA_MonitoredItem *monit
   // another function to handle status and events.
   if (monitoredItem->MonitoredItemType != MONITOREDITEM_TYPE_CHANGENOTIFY)
       return;
-  
-  // Verify that the *Node being monitored is still valid
-  // Looking up the in the nodestore is only necessary if we suspect that it is changed during writes
-  // e.g. in multithreaded applications
-#ifdef MONITOREDITEM_FORCE_NODEPOINTER_VERIFY
-  const UA_Node *target;
-  target = UA_NodeStore_get((const UA_NodeStore *) (server->nodestore), (const UA_NodeId *) &( monitoredItem->monitoredNodeId));
-  if (target == NULL)
-    return;
-  if (target != monitoredItem->monitoredNode)
-    monitoredItem->monitoredNode = target;
-  UA_NodeStore_release(target);
-#endif
-  
+
   newvalue = (MonitoredItem_queuedValue *) UA_malloc(sizeof(MonitoredItem_queuedValue));
+  if(!newvalue)
+      return;
   LIST_INITENTRY(newvalue,listEntry);
   newvalue->value.arrayLength         = 0;
   newvalue->value.arrayDimensionsSize = 0;
@@ -425,12 +414,25 @@ void MonitoredItem_QueuePushDataValue(UA_Server *server, UA_MonitoredItem *monit
   newvalue->value.data                = NULL;
   newvalue->value.type                = NULL;
   
-  samplingError = MonitoredItem_CopyMonitoredValueToVariant(monitoredItem->AttributeID, monitoredItem->monitoredNode, &(newvalue->value));
+  // Verify that the *Node being monitored is still valid
+  // Looking up the in the nodestore is only necessary if we suspect that it is changed during writes
+  // e.g. in multithreaded applications
+  const UA_Node *target = UA_NodeStore_get(server->nodestore, &monitoredItem->monitoredNodeId);
+  if(!target) {
+      UA_free(newvalue);
+      return;
+  }
   
-  if ((monitoredItem->QueueSize).currentValue >= (monitoredItem->QueueSize).maxValue) {
-    if (newvalue->value.type != NULL && monitoredItem->DiscardOldest == UA_TRUE && monitoredItem->queue.lh_first != NULL ) {
-          for(queueItem = monitoredItem->queue.lh_first; queueItem->listEntry.le_next != NULL; queueItem = queueItem->listEntry.le_next) {}
-
+  samplingError = MonitoredItem_CopyMonitoredValueToVariant(monitoredItem->AttributeID, target,
+                                                            &newvalue->value);
+  UA_NodeStore_release(target);
+  
+  if(monitoredItem->QueueSize.currentValue >= monitoredItem->QueueSize.maxValue) {
+    if(newvalue->value.type && monitoredItem->DiscardOldest == UA_TRUE &&
+        monitoredItem->queue.lh_first != NULL ) {
+        for(queueItem = monitoredItem->queue.lh_first;
+            queueItem->listEntry.le_next != NULL;
+            queueItem = queueItem->listEntry.le_next) {}
           LIST_REMOVE(queueItem, listEntry);
           UA_free(queueItem);
           (monitoredItem->QueueSize).currentValue--;

+ 0 - 5
src/server/ua_subscription.h

@@ -17,10 +17,6 @@
 /* MonitoredItem */
 /*****************/
 
-// This will force the monitored UA_Node* to be retrieved from the
-// Nodestore based on its UA_NodeId on each PushQueueValue call.
-#define MONITOREDITEM_FORCE_NODEPOINTER_VERIFY
-
 typedef struct {
     UA_Int32 currentValue;
     UA_Int32 minValue;
@@ -49,7 +45,6 @@ typedef struct UA_MonitoredItem_s {
     MONITOREDITEM_TYPE		    MonitoredItemType;
     UA_UInt32                       TimestampsToReturn;
     UA_UInt32                       MonitoringMode;
-    const UA_Node                  *monitoredNode; // Pointer to a node of any type
     UA_NodeId                       monitoredNodeId; 
     UA_UInt32                       AttributeID;
     UA_UInt32                       ClientHandle;