remove nodestore_release

todo: fix the unit tests, add RCU calls to the worker loop

src/server/ua_nodestore.c

@@ -197,7 +197,7 @@ void UA_NodeStore_delete(UA_NodeStore *ns) {
     UA_free(ns);
 }
 
-UA_StatusCode UA_NodeStore_insert(UA_NodeStore *ns, UA_Node *node, const UA_Node **inserted) {
+UA_StatusCode UA_NodeStore_insert(UA_NodeStore *ns, UA_Node *node, UA_Node **inserted) {
     if(ns->size * 3 <= ns->count * 4) {
         if(expand(ns) != UA_STATUSCODE_GOOD)
             return UA_STATUSCODE_BADINTERNALERROR;
@@ -235,7 +235,8 @@ UA_StatusCode UA_NodeStore_insert(UA_NodeStore *ns, UA_Node *node, const UA_Node
 }
 
 UA_StatusCode
-UA_NodeStore_replace(UA_NodeStore *ns, const UA_Node *oldNode, UA_Node *node, const UA_Node **inserted) {
+UA_NodeStore_replace(UA_NodeStore *ns, UA_Node *oldNode,
+                     UA_Node *node, UA_Node **inserted) {
     UA_NodeStoreEntry *slot;
     if(!containsNodeId(ns, &node->nodeId, &slot))
         return UA_STATUSCODE_BADNODEIDUNKNOWN;
@@ -249,7 +250,7 @@ UA_NodeStore_replace(UA_NodeStore *ns, const UA_Node *oldNode, UA_Node *node, co
     return UA_STATUSCODE_GOOD;
 }
 
-const UA_Node * UA_NodeStore_get(const UA_NodeStore *ns, const UA_NodeId *nodeid) {
+UA_Node * UA_NodeStore_get(const UA_NodeStore *ns, const UA_NodeId *nodeid) {
     UA_NodeStoreEntry *slot;
     if(!containsNodeId(ns, nodeid, &slot))
         return NULL;
@@ -274,6 +275,3 @@ void UA_NodeStore_iterate(const UA_NodeStore *ns, UA_NodeStore_nodeVisitor visit
             visitor(&ns->entries[i].node.node);
     }
 }
-
-void UA_NodeStore_release(const UA_Node *managed) {
-}

src/server/ua_nodestore.h

@@ -1,7 +1,7 @@
 #ifndef UA_NODESTORE_H_
 #define UA_NODESTORE_H_
 
-#include "ua_types.h"
+#include "ua_types_generated.h"
 #include "ua_nodes.h"
 
 /**
@@ -25,13 +25,21 @@
  * @{
  */
 
+/* For multithreading, nodes in the nodestore are immutable */
+#ifdef UA_MULTITHREADING
+# define UA_MT_CONST const
+#else
+# define UA_MT_CONST
+#endif
+
 struct UA_NodeStore;
 typedef struct UA_NodeStore UA_NodeStore;
 
 /** Create a new nodestore */
 UA_NodeStore * UA_NodeStore_new(void);
 
-/** Delete the nodestore and all nodes in it */
+/** Delete the nodestore and all nodes in it. Do not call from a read-side
+    critical section (multithreading). */
 void UA_NodeStore_delete(UA_NodeStore *ns);
 
 /**
@@ -41,14 +49,16 @@ void UA_NodeStore_delete(UA_NodeStore *ns);
  * is not NULL, then a pointer to the managed node is returned (and must be
  * released).
  */
-UA_StatusCode UA_NodeStore_insert(UA_NodeStore *ns, UA_Node *node, const UA_Node **inserted);
+UA_StatusCode UA_NodeStore_insert(UA_NodeStore *ns, UA_Node *node, UA_MT_CONST UA_Node **inserted);
 
 /**
  * Replace an existing node in the nodestore. If the node was already replaced,
- * UA_STATUSCODE_BADINTERNALERROR is returned. If inserted is not NULL, a
- * pointer to the managed (immutable) node is returned.
+ * UA_STATUSCODE_BADINTERNALERROR is returned. A pointer to the inserted node is
+ * returned. It is important that oldNode is not used afterwards in the same
+ * thread.
  */
-UA_StatusCode UA_NodeStore_replace(UA_NodeStore *ns, const UA_Node *oldNode, UA_Node *node, const UA_Node **inserted);
+UA_StatusCode UA_NodeStore_replace(UA_NodeStore *ns, UA_MT_CONST UA_Node *oldNode,
+                                   UA_Node *node, UA_MT_CONST UA_Node **inserted);
 
 /**
  * Remove a node from the nodestore. Always succeeds, even if the node was not
@@ -62,13 +72,7 @@ UA_StatusCode UA_NodeStore_remove(UA_NodeStore *ns, const UA_NodeId *nodeid);
  * entirely. After the node is no longer used, it needs to be released to decrease
  * the reference count.
  */
-const UA_Node * UA_NodeStore_get(const UA_NodeStore *ns, const UA_NodeId *nodeid);
-
-/**
- * Release a managed node. Do never call this with a node that isn't managed by a
- * nodestore.
- */
-void UA_NodeStore_release(const UA_Node *managed);
+UA_MT_CONST UA_Node * UA_NodeStore_get(const UA_NodeStore *ns, const UA_NodeId *nodeid);
 
 /**
  * A function that can be evaluated on all entries in a nodestore via

src/server/ua_nodestore_concurrent.c

@@ -1,113 +1,90 @@
 #include "ua_util.h"
 #include "ua_nodestore.h"
 
-#define ALIVE_BIT (1 << 15) /* Alive bit in the refcount */
-
 struct nodeEntry {
-    struct cds_lfht_node htn;      /* contains next-ptr for urcu-hashmap */
-    struct rcu_head      rcu_head; /* For call-rcu */
-    UA_UInt16 refcount;            /* Counts the amount of readers on it [alive-bit, 15 counter-bits] */
-    UA_Node node;                  /* Might be cast from any _bigger_ UA_Node* type. Allocate enough memory! */
+    struct cds_lfht_node htn; ///< Contains the next-ptr for urcu-hashmap
+    struct rcu_head rcu_head; ///< For call-rcu
+    UA_Node node; ///< Might be cast from any _bigger_ UA_Node* type. Allocate enough memory!
 };
 
 struct UA_NodeStore {
-    struct cds_lfht *ht; /* Hash table */
+    struct cds_lfht *ht;
 };
 
 #include "ua_nodestore_hash.inc"
 
-static void node_deleteMembers(UA_Node *node) {
-    switch(node->nodeClass) {
+static void deleteEntry(struct rcu_head *head) {
+    struct nodeEntry *entry = caa_container_of(head, struct nodeEntry, rcu_head);
+    switch(entry->node.nodeClass) {
     case UA_NODECLASS_OBJECT:
-        UA_ObjectNode_deleteMembers((UA_ObjectNode *)node);
+        UA_ObjectNode_deleteMembers((UA_ObjectNode*)&entry->node);
         break;
     case UA_NODECLASS_VARIABLE:
-        UA_VariableNode_deleteMembers((UA_VariableNode *)node);
+        UA_VariableNode_deleteMembers((UA_VariableNode*)&entry->node);
         break;
     case UA_NODECLASS_METHOD:
-        UA_MethodNode_deleteMembers((UA_MethodNode *)node);
+        UA_MethodNode_deleteMembers((UA_MethodNode*)&entry->node);
         break;
     case UA_NODECLASS_OBJECTTYPE:
-        UA_ObjectTypeNode_deleteMembers((UA_ObjectTypeNode *)node);
+        UA_ObjectTypeNode_deleteMembers((UA_ObjectTypeNode*)&entry->node);
         break;
     case UA_NODECLASS_VARIABLETYPE:
-        UA_VariableTypeNode_deleteMembers((UA_VariableTypeNode *)node);
+        UA_VariableTypeNode_deleteMembers((UA_VariableTypeNode*)&entry->node);
         break;
     case UA_NODECLASS_REFERENCETYPE:
-        UA_ReferenceTypeNode_deleteMembers((UA_ReferenceTypeNode *)node);
+        UA_ReferenceTypeNode_deleteMembers((UA_ReferenceTypeNode*)&entry->node);
         break;
     case UA_NODECLASS_DATATYPE:
-        UA_DataTypeNode_deleteMembers((UA_DataTypeNode *)node);
+        UA_DataTypeNode_deleteMembers((UA_DataTypeNode*)&entry->node);
         break;
     case UA_NODECLASS_VIEW:
-        UA_ViewNode_deleteMembers((UA_ViewNode *)node);
+        UA_ViewNode_deleteMembers((UA_ViewNode*)&entry->node);
         break;
     default:
         UA_assert(UA_FALSE);
         break;
     }
+    free(entry);
 }
 
 /* We are in a rcu_read lock. So the node will not be freed under our feet. */
 static int compare(struct cds_lfht_node *htn, const void *orig) {
     const UA_NodeId *origid = (const UA_NodeId *)orig;
-    const UA_NodeId *newid  = &((struct nodeEntry *)htn)->node.nodeId;   /* The htn is first in the entry structure. */
+    /* The htn is first in the entry structure. */
+    const UA_NodeId *newid  = &((struct nodeEntry *)htn)->node.nodeId;
     return UA_NodeId_equal(newid, origid);
 }
 
-/* The entry was removed from the hashtable. No more readers can get it. Since
-   all readers using the node for a longer time (outside the rcu critical
-   section) increased the refcount, we only need to wait for the refcount
-   to reach zero. */
-static void markDead(struct rcu_head *head) {
-    struct nodeEntry *entry = (struct nodeEntry*) ((uintptr_t)head - offsetof(struct nodeEntry, rcu_head)); 
-    uatomic_and(&entry->refcount, ~ALIVE_BIT); // set the alive bit to zero
-    if(uatomic_read(&entry->refcount) > 0)
-        return;
-
-    node_deleteMembers(&entry->node);
-    UA_free(entry);
-}
-
-/* Free the entry if it is dead and nobody uses it anymore */
-void UA_NodeStore_release(const UA_Node *managed) {
-    struct nodeEntry *entry = (struct nodeEntry*) ((uintptr_t)managed - offsetof(struct nodeEntry, node)); 
-    if(uatomic_add_return(&entry->refcount, -1) == 0) {
-        node_deleteMembers(&entry->node);
-        UA_free(entry);
-    }
-}
-
 UA_NodeStore * UA_NodeStore_new() {
     UA_NodeStore *ns;
     if(!(ns = UA_malloc(sizeof(UA_NodeStore))))
-        return NULL;
+        return UA_NULL;
 
     /* 32 is the minimum size for the hashtable. */
     ns->ht = cds_lfht_new(32, 32, 0, CDS_LFHT_AUTO_RESIZE, NULL);
     if(!ns->ht) {
         UA_free(ns);
-        return NULL;
+        ns = UA_NULL;
     }
     return ns;
 }
 
+/* do not call with read-side critical section held!! */
 void UA_NodeStore_delete(UA_NodeStore *ns) {
-    struct cds_lfht      *ht = ns->ht;
+    struct cds_lfht *ht = ns->ht;
     struct cds_lfht_iter  iter;
-
-    rcu_read_lock();
     cds_lfht_first(ht, &iter);
+    rcu_read_lock();
     while(iter.node) {
         if(!cds_lfht_del(ht, iter.node)) {
-            struct nodeEntry *entry = (struct nodeEntry*) ((uintptr_t)iter.node - offsetof(struct nodeEntry, htn)); 
-            call_rcu(&entry->rcu_head, markDead);
+            /* points to the htn entry, which is first */
+            struct nodeEntry *entry = (struct nodeEntry*) iter.node;
+            call_rcu(&entry->rcu_head, deleteEntry);
         }
         cds_lfht_next(ht, &iter);
     }
     rcu_read_unlock();
-    cds_lfht_destroy(ht, NULL);
-
+    cds_lfht_destroy(ht, UA_NULL);
     UA_free(ns);
 }
 
@@ -146,25 +123,18 @@ UA_StatusCode UA_NodeStore_insert(UA_NodeStore *ns, UA_Node *node, const UA_Node
     struct nodeEntry *entry;
     if(!(entry = UA_malloc(sizeof(struct nodeEntry) - sizeof(UA_Node) + nodesize)))
         return UA_STATUSCODE_BADOUTOFMEMORY;
-    memcpy((void*)&entry->node, node, nodesize);
+    UA_Node *newNode = &entry->node;
+    UA_memcpy(newNode, node, nodesize);
 
     cds_lfht_node_init(&entry->htn);
-    entry->refcount = ALIVE_BIT;
-    if(inserted) // increase the counter before adding the node
-        entry->refcount++;
-
     struct cds_lfht_node *result;
-    //FIXME: a bit dirty workaround of preserving namespace
     //namespace index is assumed to be valid
     UA_NodeId tempNodeid;
-    UA_NodeId_copy(&node->nodeId, &tempNodeid);
+    tempNodeid = node->nodeId;
     tempNodeid.namespaceIndex = 0;
     if(!UA_NodeId_isNull(&tempNodeid)) {
         hash_t h = hash(&node->nodeId);
-        rcu_read_lock();
-        result = cds_lfht_add_unique(ns->ht, h, compare, &entry->node.nodeId, &entry->htn);
-        rcu_read_unlock();
-
+        result = cds_lfht_add_unique(ns->ht, h, compare, &newNode->nodeId, &entry->htn);
         /* If the nodeid exists already */
         if(result != &entry->htn) {
             UA_free(entry);
@@ -172,31 +142,28 @@ UA_StatusCode UA_NodeStore_insert(UA_NodeStore *ns, UA_Node *node, const UA_Node
         }
     } else {
         /* create a unique nodeid */
-        ((UA_Node *)&entry->node)->nodeId.identifierType = UA_NODEIDTYPE_NUMERIC;
-        if(((UA_Node *)&entry->node)->nodeId.namespaceIndex == 0) //original request for ns=0 should yield ns=1
-            ((UA_Node *)&entry->node)->nodeId.namespaceIndex = 1;
-        if(((UA_Node *)&entry->node)->nodeClass==UA_NODECLASS_VARIABLE){ //set namespaceIndex in browseName in case id is generated
-        	((UA_VariableNode*)&entry->node)->browseName.namespaceIndex=((UA_Node *)&entry->node)->nodeId.namespaceIndex;
-        }
+        newNode->nodeId.identifierType = UA_NODEIDTYPE_NUMERIC;
+        if(newNode->nodeId.namespaceIndex == 0) // original request for ns=0 should yield ns=1
+            newNode->nodeId.namespaceIndex = 1;
+        /* set namespaceIndex in browseName in case id is generated */
+        if(newNode->nodeClass == UA_NODECLASS_VARIABLE)
+        	((UA_VariableNode*)newNode)->browseName.namespaceIndex = newNode->nodeId.namespaceIndex;
 
         unsigned long identifier;
         long before, after;
-        rcu_read_lock();
         cds_lfht_count_nodes(ns->ht, &before, &identifier, &after); // current amount of nodes stored
         identifier++;
 
-        ((UA_Node *)&entry->node)->nodeId.identifier.numeric = identifier;
+        newNode->nodeId.identifier.numeric = identifier;
         while(UA_TRUE) {
-            hash_t nhash = hash(&entry->node.nodeId);
-            result = cds_lfht_add_unique(ns->ht, nhash, compare, &entry->node.nodeId, &entry->htn);
+            hash_t h = hash(&newNode->nodeId);
+            result = cds_lfht_add_unique(ns->ht, h, compare, &newNode->nodeId, &entry->htn);
             if(result == &entry->htn)
                 break;
-
-            ((UA_Node *)&entry->node)->nodeId.identifier.numeric += (identifier * 2654435761);
+            newNode->nodeId.identifier.numeric += (identifier * 2654435761);
         }
-        rcu_read_unlock();
     }
-    UA_NodeId_deleteMembers(&tempNodeid);
+
     UA_free(node);
     if(inserted)
         *inserted = &entry->node;
@@ -205,6 +172,18 @@ UA_StatusCode UA_NodeStore_insert(UA_NodeStore *ns, UA_Node *node, const UA_Node
 
 UA_StatusCode UA_NodeStore_replace(UA_NodeStore *ns, const UA_Node *oldNode, UA_Node *node,
                                    const UA_Node **inserted) {
+    /* Get the current version */
+    hash_t h = hash(&node->nodeId);
+    struct cds_lfht_iter iter;
+    cds_lfht_lookup(ns->ht, h, compare, &node->nodeId, &iter);
+    if(!iter.node)
+        return UA_STATUSCODE_BADNODEIDUNKNOWN;
+
+    /* We try to replace an obsolete version of the node */
+    struct nodeEntry *oldEntry = (struct nodeEntry*)iter.node;
+    if(&oldEntry->node != oldNode)
+        return UA_STATUSCODE_BADINTERNALERROR;
+    
     size_t nodesize;
     /* Copy the node into the entry. Then reset the original node. It shall no longer be used. */
     switch(node->nodeClass) {
@@ -239,44 +218,17 @@ UA_StatusCode UA_NodeStore_replace(UA_NodeStore *ns, const UA_Node *oldNode, UA_
     struct nodeEntry *newEntry;
     if(!(newEntry = UA_malloc(sizeof(struct nodeEntry) - sizeof(UA_Node) + nodesize)))
         return UA_STATUSCODE_BADOUTOFMEMORY;
-    memcpy((void*)&newEntry->node, node, nodesize);
-
+    UA_memcpy((void*)&newEntry->node, node, nodesize);
     cds_lfht_node_init(&newEntry->htn);
-    newEntry->refcount = ALIVE_BIT;
-    if(inserted) // increase the counter before adding the node
-        newEntry->refcount++;
 
-    hash_t h = hash(&node->nodeId);
-    struct cds_lfht_iter iter;
-    rcu_read_lock();
-    cds_lfht_lookup(ns->ht, h, compare, &node->nodeId, &iter);
-
-    /* No node found that can be replaced */
-    if(!iter.node) {
-        rcu_read_unlock();
-        UA_free(newEntry);
-        return UA_STATUSCODE_BADNODEIDUNKNOWN;
-    }
-
-    struct nodeEntry *oldEntry = (struct nodeEntry*) ((uintptr_t)iter.node - offsetof(struct nodeEntry, htn)); 
-    /* The node we found is obsolete*/
-    if(&oldEntry->node != oldNode) {
-        rcu_read_unlock();
-        UA_free(newEntry);
-        return UA_STATUSCODE_BADINTERNALERROR;
-    }
-
-    /* The old node is replaced by a managed node. */
     if(cds_lfht_replace(ns->ht, &iter, h, compare, &node->nodeId, &newEntry->htn) != 0) {
         /* Replacing failed. Maybe the node got replaced just before this thread tried to.*/
-        rcu_read_unlock();
         UA_free(newEntry);
         return UA_STATUSCODE_BADINTERNALERROR;
     }
         
     /* If an entry got replaced, mark it as dead. */
-    call_rcu(&oldEntry->rcu_head, markDead);
-    rcu_read_unlock();
+    call_rcu(&oldEntry->rcu_head, deleteEntry);
     UA_free(node);
 
     if(inserted)
@@ -285,58 +237,33 @@ UA_StatusCode UA_NodeStore_replace(UA_NodeStore *ns, const UA_Node *oldNode, UA_
 }
 
 UA_StatusCode UA_NodeStore_remove(UA_NodeStore *ns, const UA_NodeId *nodeid) {
-    hash_t nhash = hash(nodeid);
+    hash_t h = hash(nodeid);
     struct cds_lfht_iter iter;
-
-    rcu_read_lock();
-    /* If this fails, then the node has already been removed. */
-    cds_lfht_lookup(ns->ht, nhash, compare, &nodeid, &iter);
-    if(!iter.node || cds_lfht_del(ns->ht, iter.node) != 0) {
-        rcu_read_unlock();
+    cds_lfht_lookup(ns->ht, h, compare, &nodeid, &iter);
+    if(!iter.node || cds_lfht_del(ns->ht, iter.node) != 0)
         return UA_STATUSCODE_BADNODEIDUNKNOWN;
-    }
-
-    struct nodeEntry *entry = (struct nodeEntry*) ((uintptr_t)iter.node - offsetof(struct nodeEntry, htn)); 
-    call_rcu(&entry->rcu_head, markDead);
-    rcu_read_unlock();
-
+    struct nodeEntry *entry = (struct nodeEntry*)iter.node;
+    call_rcu(&entry->rcu_head, deleteEntry);
     return UA_STATUSCODE_GOOD;
 }
 
 const UA_Node * UA_NodeStore_get(const UA_NodeStore *ns, const UA_NodeId *nodeid) {
-    hash_t nhash = hash(nodeid);
+    hash_t h = hash(nodeid);
     struct cds_lfht_iter iter;
-
-    rcu_read_lock();
-    cds_lfht_lookup(ns->ht, nhash, compare, nodeid, &iter);
-    struct nodeEntry *found_entry = (struct nodeEntry *)cds_lfht_iter_get_node(&iter);
-
-    if(!found_entry) {
-        rcu_read_unlock();
-        return NULL;
-    }
-
-    /* This is done within a read-lock. The node will not be marked dead within a read-lock. */
-    uatomic_inc(&found_entry->refcount);
-    rcu_read_unlock();
+    cds_lfht_lookup(ns->ht, h, compare, nodeid, &iter);
+    struct nodeEntry *found_entry = (struct nodeEntry*)iter.node;
+    if(!found_entry)
+        return UA_NULL;
     return &found_entry->node;
 }
 
 void UA_NodeStore_iterate(const UA_NodeStore *ns, UA_NodeStore_nodeVisitor visitor) {
-    struct cds_lfht     *ht = ns->ht;
+    struct cds_lfht *ht = ns->ht;
     struct cds_lfht_iter iter;
-
-    rcu_read_lock();
     cds_lfht_first(ht, &iter);
-    while(iter.node != NULL) {
-        struct nodeEntry *found_entry = (struct nodeEntry *)cds_lfht_iter_get_node(&iter);
-        uatomic_inc(&found_entry->refcount);
-        const UA_Node      *node = &found_entry->node;
-        rcu_read_unlock();
-        visitor(node);
-        UA_NodeStore_release((const UA_Node *)node);
-        rcu_read_lock();
+    while(iter.node != UA_NULL) {
+        struct nodeEntry *found_entry = (struct nodeEntry*)iter.node;
+        visitor(&found_entry->node);
         cds_lfht_next(ht, &iter);
     }
-    rcu_read_unlock();
 }

src/server/ua_server.c

@@ -127,14 +127,11 @@ UA_Server_forEachChildNodeCall(UA_Server *server, UA_NodeId parentNodeId,
     const UA_Node *parent = UA_NodeStore_get(server->nodestore, &parentNodeId);
     if(!parent)
         return UA_STATUSCODE_BADNODEIDINVALID;
-    
     for(size_t i = 0; i < parent->referencesSize; i++) {
         UA_ReferenceNode *ref = &parent->references[i];
         retval |= callback(ref->targetId.nodeId, ref->isInverse,
                            ref->referenceTypeId, handle);
     }
-    
-    UA_NodeStore_release(parent);
     return retval;
 }
 

src/server/ua_services_attribute.c

@@ -305,8 +305,6 @@ void Service_Read_single(UA_Server *server, UA_Session *session, const UA_Timest
         break;
     }
 
-    UA_NodeStore_release(node);
-
     if(retval != UA_STATUSCODE_GOOD) {
         v->hasValue = UA_FALSE;
         v->hasStatus = UA_TRUE;
@@ -422,11 +420,9 @@ UA_StatusCode UA_Server_editNode(UA_Server *server, UA_Session *session, const U
             return UA_STATUSCODE_BADNODEIDUNKNOWN;
 #ifndef UA_MULTITHREADING
         retval = callback(server, session, (UA_Node*)(uintptr_t)node, data);
-        UA_NodeStore_release(node);
         return retval;
 #else
         UA_Node *copy = UA_Node_copyAnyNodeClass(node);
-        UA_NodeStore_release(node);
         if(!copy)
             return UA_STATUSCODE_BADOUTOFMEMORY;
         retval = callback(server, session, copy, data);
@@ -673,11 +669,10 @@ UA_StatusCode Service_Write_single(UA_Server *server, UA_Session *session, UA_Wr
             return UA_STATUSCODE_BADNODEIDUNKNOWN;
         if(orig->nodeClass & (UA_NODECLASS_VARIABLE | UA_NODECLASS_VARIABLE) &&
            ((const UA_VariableNode*)orig)->valueSource == UA_VALUESOURCE_DATASOURCE) {
-            UA_StatusCode retval = Service_Write_single_ValueDataSource(server, session, (const UA_VariableNode*)orig, wvalue);
-            UA_NodeStore_release(orig);
+            UA_StatusCode retval =
+                Service_Write_single_ValueDataSource(server, session, (const UA_VariableNode*)orig, wvalue);
             return retval;
         }
-        UA_NodeStore_release(orig);
     }
     return UA_Server_editNode(server, session, &wvalue->nodeId,
                               (UA_EditNodeCallback)MoveAttributeIntoNode, wvalue);

src/server/ua_services_call.c

@@ -21,7 +21,6 @@ getArgumentsVariableNode(UA_Server *server, const UA_MethodNode *ofMethod,
                 UA_String_equal(&withBrowseName, &refTarget->browseName.name)) {
                 return (const UA_VariableNode*) refTarget;
             }
-            UA_NodeStore_release(refTarget);
         }
     }
     return NULL;
@@ -100,26 +99,28 @@ argConformsToDefinition(UA_CallMethodRequest *rs, const UA_VariableNode *argDefi
 static void
 callMethod(UA_Server *server, UA_Session *session, UA_CallMethodRequest *request,
            UA_CallMethodResult *result) {
-    const UA_MethodNode *methodCalled = (const UA_MethodNode*)UA_NodeStore_get(server->nodestore, &request->methodId);
+    const UA_MethodNode *methodCalled =
+        (const UA_MethodNode*)UA_NodeStore_get(server->nodestore, &request->methodId);
     if(!methodCalled) {
         result->statusCode = UA_STATUSCODE_BADMETHODINVALID;
         return;
     }
     
-    const UA_ObjectNode *withObject = (const UA_ObjectNode*)UA_NodeStore_get(server->nodestore, &request->objectId);
+    const UA_ObjectNode *withObject =
+        (const UA_ObjectNode*)UA_NodeStore_get(server->nodestore, &request->objectId);
     if(!withObject) {
         result->statusCode = UA_STATUSCODE_BADNODEIDINVALID;
-        goto releaseMethodReturn;
+        return;
     }
     
     if(methodCalled->nodeClass != UA_NODECLASS_METHOD) {
         result->statusCode = UA_STATUSCODE_BADNODECLASSINVALID;
-        goto releaseBothReturn;
+        return;
     }
     
     if(withObject->nodeClass != UA_NODECLASS_OBJECT && withObject->nodeClass != UA_NODECLASS_OBJECTTYPE) {
         result->statusCode = UA_STATUSCODE_BADNODECLASSINVALID;
-        goto releaseBothReturn;
+        return;
     }
     
     /* Verify method/object relations */
@@ -136,12 +137,12 @@ callMethod(UA_Server *server, UA_Session *session, UA_CallMethodRequest *request
         }
     }
     if(result->statusCode != UA_STATUSCODE_GOOD)
-        goto releaseBothReturn;
+        return;
         
     /* Verify method executable */
     if(methodCalled->executable == UA_FALSE || methodCalled->userExecutable == UA_FALSE) {
         result->statusCode = UA_STATUSCODE_BADNOTWRITABLE; // There is no NOTEXECUTABLE?
-        goto releaseBothReturn;
+        return;
     }
 
     /* Verify Input Argument count, types and sizes */
@@ -149,12 +150,11 @@ callMethod(UA_Server *server, UA_Session *session, UA_CallMethodRequest *request
         getArgumentsVariableNode(server, methodCalled, UA_STRING("InputArguments"));
     if(inputArguments) {
         result->statusCode = argConformsToDefinition(request, inputArguments);
-        UA_NodeStore_release((const UA_Node*)inputArguments);
         if(result->statusCode != UA_STATUSCODE_GOOD)
-            goto releaseBothReturn;
+            return;
     } else if(request->inputArgumentsSize > 0) {
         result->statusCode = UA_STATUSCODE_BADINVALIDARGUMENT;
-        goto releaseBothReturn;
+        return;
     }
 
     const UA_VariableNode *outputArguments =
@@ -162,7 +162,7 @@ callMethod(UA_Server *server, UA_Session *session, UA_CallMethodRequest *request
     if(!outputArguments) {
         // A MethodNode must have an OutputArguments variable (which may be empty)
         result->statusCode = UA_STATUSCODE_BADINTERNALERROR;
-        goto releaseBothReturn;
+        return;
     }
     
     /* Call method if available */
@@ -177,13 +177,6 @@ callMethod(UA_Server *server, UA_Session *session, UA_CallMethodRequest *request
         result->statusCode = UA_STATUSCODE_BADNOTWRITABLE; // There is no NOTEXECUTABLE?
     
     /* TODO: Verify Output Argument count, types and sizes */
-    if(outputArguments)
-        UA_NodeStore_release((const UA_Node*)outputArguments);
-
- releaseBothReturn:
-    UA_NodeStore_release((const UA_Node*)withObject);
- releaseMethodReturn:
-    UA_NodeStore_release((const UA_Node*)methodCalled);
 }
 
 void Service_Call(UA_Server *server, UA_Session *session, const UA_CallRequest *request,

src/server/ua_services_nodemanagement.c

@@ -29,40 +29,40 @@ UA_Server_addExistingNode(UA_Server *server, UA_Session *session, UA_Node *node,
         (const UA_ReferenceTypeNode *)UA_NodeStore_get(server->nodestore, referenceTypeId);
     if(!referenceType) {
         result->statusCode = UA_STATUSCODE_BADREFERENCETYPEIDINVALID;
-        goto ret;
+        return;
     }
 
     if(referenceType->nodeClass != UA_NODECLASS_REFERENCETYPE) {
         result->statusCode = UA_STATUSCODE_BADREFERENCETYPEIDINVALID;
-        goto ret2;
+        return;
     }
 
     if(referenceType->isAbstract == UA_TRUE) {
         result->statusCode = UA_STATUSCODE_BADREFERENCENOTALLOWED;
-        goto ret2;
+        return;
     }
 
     // todo: test if the referencetype is hierarchical
     // todo: namespace index is assumed to be valid
-    const UA_Node *managed = NULL;
+    UA_MT_CONST UA_Node *managed = NULL;
     UA_NodeId tempNodeid = node->nodeId;
     tempNodeid.namespaceIndex = 0;
     if(UA_NodeId_isNull(&tempNodeid)) {
         if(UA_NodeStore_insert(server->nodestore, node, &managed) != UA_STATUSCODE_GOOD) {
             result->statusCode = UA_STATUSCODE_BADOUTOFMEMORY;
-            goto ret2;
+            return;
         }
         result->addedNodeId = managed->nodeId; // cannot fail as unique nodeids are numeric
     } else {
         if(UA_NodeId_copy(&node->nodeId, &result->addedNodeId) != UA_STATUSCODE_GOOD) {
             result->statusCode = UA_STATUSCODE_BADOUTOFMEMORY;
-            goto ret2;
+            return;
         }
 
         if(UA_NodeStore_insert(server->nodestore, node, &managed) != UA_STATUSCODE_GOOD) {
             result->statusCode = UA_STATUSCODE_BADNODEIDEXISTS;
             UA_NodeId_deleteMembers(&result->addedNodeId);
-            goto ret2;
+            return;
         }
     }
     
@@ -76,12 +76,6 @@ UA_Server_addExistingNode(UA_Server *server, UA_Session *session, UA_Node *node,
     Service_AddReferences_single(server, session, &item);
 
     // todo: error handling. remove new node from nodestore
-    UA_NodeStore_release(managed);
-    
- ret2:
-    UA_NodeStore_release((const UA_Node*)referenceType);
- ret:
-    UA_NodeStore_release(parent);
 }
 
 static UA_StatusCode
@@ -99,10 +93,8 @@ copyExistingVariable(UA_Server *server, UA_Session *session, const UA_NodeId *va
     const UA_VariableNode *node = (const UA_VariableNode*)UA_NodeStore_get(server->nodestore, variable);
     if(!node)
         return UA_STATUSCODE_BADNODEIDINVALID;
-    if(node->nodeClass != UA_NODECLASS_VARIABLE) {
-        UA_NodeStore_release((const UA_Node*)node);
+    if(node->nodeClass != UA_NODECLASS_VARIABLE)
         return UA_STATUSCODE_BADNODECLASSINVALID;
-    }
     
     // copy the variable attributes
     UA_VariableAttributes attr;
@@ -151,7 +143,6 @@ copyExistingVariable(UA_Server *server, UA_Session *session, const UA_NodeId *va
     }
 
     UA_AddNodesResult_deleteMembers(&res);
-    UA_NodeStore_release((const UA_Node*)node);
     return UA_STATUSCODE_GOOD;
 }
 
@@ -163,10 +154,9 @@ copyExistingObject(UA_Server *server, UA_Session *session, const UA_NodeId *vari
     const UA_ObjectNode *node = (const UA_ObjectNode*)UA_NodeStore_get(server->nodestore, variable);
     if(!node)
         return UA_STATUSCODE_BADNODEIDINVALID;
-    if(node->nodeClass != UA_NODECLASS_OBJECT) {
-        UA_NodeStore_release((const UA_Node*)node);
+    if(node->nodeClass != UA_NODECLASS_OBJECT)
         return UA_STATUSCODE_BADNODECLASSINVALID;
-    }
+
     // copy the variable attributes
     UA_ObjectAttributes attr;
     UA_ObjectAttributes_init(&attr);
@@ -206,7 +196,6 @@ copyExistingObject(UA_Server *server, UA_Session *session, const UA_NodeId *vari
     }
 
     UA_AddNodesResult_deleteMembers(&res);
-    UA_NodeStore_release((const UA_Node*)node);
     return UA_STATUSCODE_GOOD;
 }
 
@@ -224,10 +213,8 @@ instantiateObjectNode(UA_Server *server, UA_Session *session,
     const UA_ObjectTypeNode *type = (const UA_ObjectTypeNode*)UA_NodeStore_get(server->nodestore, typeId);
     if(!type)
         return UA_STATUSCODE_BADNODEIDINVALID;
-    if(type->nodeClass != UA_NODECLASS_OBJECTTYPE) {
-        UA_NodeStore_release((const UA_Node*)type);
+    if(type->nodeClass != UA_NODECLASS_OBJECTTYPE)
         return UA_STATUSCODE_BADNODECLASSINVALID;
-    }
 
     /* Add all the child nodes */
     UA_BrowseDescription browseChildren;
@@ -277,7 +264,6 @@ instantiateObjectNode(UA_Server *server, UA_Session *session,
     if(olm->constructor)
         UA_Server_editNode(server, session, nodeId,
                            (UA_EditNodeCallback)setObjectInstanceHandle, olm->constructor(*nodeId));
-    UA_NodeStore_release((const UA_Node*)type);
     return UA_STATUSCODE_GOOD;
 }
 
@@ -287,10 +273,8 @@ instantiateVariableNode(UA_Server *server, UA_Session *session, const UA_NodeId
     const UA_ObjectTypeNode *type = (const UA_ObjectTypeNode*)UA_NodeStore_get(server->nodestore, typeId);
     if(!type)
         return UA_STATUSCODE_BADNODEIDINVALID;
-    if(type->nodeClass != UA_NODECLASS_VARIABLETYPE) {
-        UA_NodeStore_release((const UA_Node*)type);
+    if(type->nodeClass != UA_NODECLASS_VARIABLETYPE)
         return UA_STATUSCODE_BADNODECLASSINVALID;
-    }
 
     /* get the references to child properties */
     UA_BrowseDescription browseChildren;
@@ -312,7 +296,6 @@ instantiateVariableNode(UA_Server *server, UA_Session *session, const UA_NodeId
         UA_ReferenceDescription *rd = &browseResult.references[i];
         copyExistingVariable(server, session, &rd->nodeId.nodeId, &rd->referenceTypeId, nodeId);
     }
-    UA_NodeStore_release((const UA_Node*)type);
 
     /* add a hastypedefinition reference */
     UA_AddReferencesItem addref;
@@ -831,7 +814,7 @@ void Service_AddReferences(UA_Server *server, UA_Session *session, const UA_AddR
 
 UA_StatusCode Service_DeleteNodes_single(UA_Server *server, UA_Session *session, const UA_NodeId *nodeId,
                                          UA_Boolean deleteReferences) {
-    const UA_Node *node = UA_NodeStore_get(server->nodestore, nodeId);
+    UA_Node *node = UA_NodeStore_get(server->nodestore, nodeId);
     if(!node)
         return UA_STATUSCODE_BADNODEIDINVALID;
     if(deleteReferences == UA_TRUE) {
@@ -864,21 +847,19 @@ UA_StatusCode Service_DeleteNodes_single(UA_Server *server, UA_Session *session,
         for(size_t i = 0; i < result.referencesSize; i++) {
             /* call the destructor */
             UA_ReferenceDescription *rd = &result.references[i];
-            const UA_ObjectTypeNode *type = (const UA_ObjectTypeNode*)UA_NodeStore_get(server->nodestore, &rd->nodeId.nodeId);
+            const UA_ObjectTypeNode *type =
+                (const UA_ObjectTypeNode*)UA_NodeStore_get(server->nodestore, &rd->nodeId.nodeId);
             if(!type)
                 continue;
-            if(type->nodeClass != UA_NODECLASS_OBJECTTYPE || !type->lifecycleManagement.destructor) {
-                UA_NodeStore_release((const UA_Node*)type);
+            if(type->nodeClass != UA_NODECLASS_OBJECTTYPE || !type->lifecycleManagement.destructor)
                 continue;
-            }
+
             /* if there are several types with lifecycle management, call all the destructors */
             type->lifecycleManagement.destructor(*nodeId, ((const UA_ObjectNode*)node)->instanceHandle);
-            UA_NodeStore_release((const UA_Node*)type);
         }
         UA_BrowseResult_deleteMembers(&result);
     }
     
-    UA_NodeStore_release(node);
     return UA_NodeStore_remove(server->nodestore, nodeId);
 }
 

src/server/ua_services_subscription.c

@@ -63,7 +63,6 @@ static void createMonitoredItems(UA_Server *server, UA_Session *session, UA_Subs
     UA_MonitoredItem *newMon = UA_MonitoredItem_new();
     if(!newMon) {
         result->statusCode = UA_STATUSCODE_BADOUTOFMEMORY;
-        UA_NodeStore_release(target);
         return;
     }
 
@@ -71,7 +70,6 @@ static void createMonitoredItems(UA_Server *server, UA_Session *session, UA_Subs
     if(retval != UA_STATUSCODE_GOOD) {
         result->statusCode = UA_STATUSCODE_BADOUTOFMEMORY;
         MonitoredItem_delete(newMon);
-        UA_NodeStore_release(target);
         return;
     }
 
@@ -100,8 +98,6 @@ static void createMonitoredItems(UA_Server *server, UA_Session *session, UA_Subs
 
     // todo: add a job that samples the value (for fixed intervals)
     // todo: add a pointer to the monitoreditem to the variable, so that events get propagated
-    
-    UA_NodeStore_release(target);
 }
 
 void Service_CreateMonitoredItems(UA_Server *server, UA_Session *session,

src/server/ua_services_view.c

@@ -125,10 +125,8 @@ returnRelevantNode(UA_Server *server, const UA_BrowseDescription *descr, UA_Bool
 
     /* return from the internal nodestore */
     const UA_Node *node = UA_NodeStore_get(server->nodestore, &reference->targetId.nodeId);
-    if(node && descr->nodeClassMask != 0 && (node->nodeClass & descr->nodeClassMask) == 0) {
-    	UA_NodeStore_release(node);
+    if(node && descr->nodeClassMask != 0 && (node->nodeClass & descr->nodeClassMask) == 0)
         return NULL;
-    }
     *isExternal = UA_FALSE;
     return node;
 }
@@ -144,11 +142,8 @@ findSubTypes(UA_NodeStore *ns, const UA_NodeId *root, UA_NodeId **reftypes, size
     const UA_Node *node = UA_NodeStore_get(ns, root);
     if(!node)
         return UA_STATUSCODE_BADNOMATCH;
-    if(node->nodeClass != UA_NODECLASS_REFERENCETYPE)  {
-        UA_NodeStore_release(node);
+    if(node->nodeClass != UA_NODECLASS_REFERENCETYPE)
         return UA_STATUSCODE_BADREFERENCETYPEIDINVALID;
-    }
-    UA_NodeStore_release(node);
 
     size_t results_size = 20; // probably too big, but saves mallocs
     UA_NodeId *results = UA_malloc(sizeof(UA_NodeId) * results_size);
@@ -188,7 +183,6 @@ findSubTypes(UA_NodeStore *ns, const UA_NodeId *root, UA_NodeId **reftypes, size
                 break;
             }
         }
-        UA_NodeStore_release(node);
     } while(++index <= last && retval == UA_STATUSCODE_GOOD);
 
     if(retval) {
@@ -258,11 +252,9 @@ Service_Browse_single(UA_Server *server, UA_Session *session, struct Continuatio
                 return;
             }
             if(rootRef->nodeClass != UA_NODECLASS_REFERENCETYPE) {
-                UA_NodeStore_release(rootRef);
                 result->statusCode = UA_STATUSCODE_BADREFERENCETYPEIDINVALID;
                 return;
             }
-            UA_NodeStore_release(rootRef);
             relevant_refs = (UA_NodeId*)(uintptr_t)&descr->referenceTypeId;
             relevant_refs_size = 1;
         }
@@ -280,7 +272,6 @@ Service_Browse_single(UA_Server *server, UA_Session *session, struct Continuatio
     /* if the node has no references, just return */
     if(node->referencesSize <= 0) {
         result->referencesSize = 0;
-        UA_NodeStore_release(node);
         if(!all_refs && descr->includeSubtypes)
             UA_Array_delete(relevant_refs, relevant_refs_size, &UA_TYPES[UA_TYPES_NODEID]);
         return;
@@ -316,10 +307,6 @@ Service_Browse_single(UA_Server *server, UA_Session *session, struct Continuatio
         		/* relevant_node returns a node malloced with UA_ObjectNode_new
                    if it is external (there is no UA_Node_new function) */
         		UA_ObjectNode_delete((UA_ObjectNode*)(uintptr_t)current);
-        	else
-        		UA_NodeStore_release(current);
-#else
-        	UA_NodeStore_release(current);
 #endif
             skipped++;
             continue;
@@ -330,10 +317,6 @@ Service_Browse_single(UA_Server *server, UA_Session *session, struct Continuatio
 #ifdef UA_EXTERNAL_NAMESPACES
         if(isExternal == UA_TRUE)
         	UA_ObjectNode_delete((UA_ObjectNode*)(uintptr_t)current);
-        else
-        	UA_NodeStore_release(current);
-#else
-        UA_NodeStore_release(current);
 #endif
         if(retval != UA_STATUSCODE_GOOD) {
             UA_Array_delete(result->references, referencesCount, &UA_TYPES[UA_TYPES_REFERENCEDESCRIPTION]);
@@ -352,7 +335,6 @@ Service_Browse_single(UA_Server *server, UA_Session *session, struct Continuatio
     }
 
     cleanup:
-    UA_NodeStore_release(node);
     if(!all_refs && descr->includeSubtypes)
         UA_Array_delete(relevant_refs, relevant_refs_size, &UA_TYPES[UA_TYPES_NODEID]);
     if(result->statusCode != UA_STATUSCODE_GOOD)
@@ -541,7 +523,6 @@ walkBrowsePath(UA_Server *server, UA_Session *session, const UA_Node *node, cons
         // test the browsename
         if(elem->targetName.namespaceIndex != next->browseName.namespaceIndex ||
            !UA_String_equal(&elem->targetName.name, &next->browseName.name)) {
-            UA_NodeStore_release(next);
             continue;
         }
 
@@ -549,7 +530,6 @@ walkBrowsePath(UA_Server *server, UA_Session *session, const UA_Node *node, cons
             // recursion if the path is longer
             retval = walkBrowsePath(server, session, next, path, pathindex + 1,
                                     targets, targets_size, target_count);
-            UA_NodeStore_release(next);
         } else {
             // add the browsetarget
             if(*target_count >= *targets_size) {
@@ -557,7 +537,6 @@ walkBrowsePath(UA_Server *server, UA_Session *session, const UA_Node *node, cons
                 newtargets = UA_realloc(targets, sizeof(UA_BrowsePathTarget) * (*targets_size) * 2);
                 if(!newtargets) {
                     retval = UA_STATUSCODE_BADOUTOFMEMORY;
-                    UA_NodeStore_release(next);
                     break;
                 }
                 *targets = newtargets;
@@ -567,7 +546,6 @@ walkBrowsePath(UA_Server *server, UA_Session *session, const UA_Node *node, cons
             UA_BrowsePathTarget *res = *targets;
             UA_ExpandedNodeId_init(&res[*target_count].targetId);
             retval = UA_NodeId_copy(&next->nodeId, &res[*target_count].targetId.nodeId);
-            UA_NodeStore_release(next);
             if(retval != UA_STATUSCODE_GOOD)
                 break;
             res[*target_count].remainingPathIndex = UA_UINT32_MAX;
@@ -603,7 +581,6 @@ void Service_TranslateBrowsePathsToNodeIds_single(UA_Server *server, UA_Session
     }
     result->statusCode = walkBrowsePath(server, session, firstNode, &path->relativePath, 0,
                                         &result->targets, &arraySize, &result->targetsSize);
-    UA_NodeStore_release(firstNode);
     if(result->targetsSize == 0 && result->statusCode == UA_STATUSCODE_GOOD)
         result->statusCode = UA_STATUSCODE_BADNOMATCH;
     if(result->statusCode != UA_STATUSCODE_GOOD) {

src/server/ua_subscription.c

@@ -462,7 +462,6 @@ void MonitoredItem_QueuePushDataValue(UA_Server *server, UA_MonitoredItem *monit
     UA_Boolean samplingError = MonitoredItem_CopyMonitoredValueToVariant(monitoredItem->attributeID, target,
                                                                          &newvalue->value);
 
-    UA_NodeStore_release(target);
     if(samplingError != UA_FALSE || !newvalue->value.value.type) {
         UA_DataValue_deleteMembers(&newvalue->value);
         UA_free(newvalue);