fix(plugin): Enable amalgamation with the plugin Nodestore

The server only allows one thread to access the nodestore internally.

plugins/ua_nodestore_hashmap.c

@@ -8,14 +8,6 @@
 
 #include <open62541/plugin/nodestore_default.h>
 
-#if UA_MULTITHREADING >= 100
-#define BEGIN_CRITSECT(NODEMAP) UA_LOCK(NODEMAP->lock)
-#define END_CRITSECT(NODEMAP) UA_UNLOCK(NODEMAP->lock)
-#else
-#define BEGIN_CRITSECT(NODEMAP) do {} while(0)
-#define END_CRITSECT(NODEMAP) do {} while(0)
-#endif
-
 /* container_of */
 #define container_of(ptr, type, member) \
     (type *)((uintptr_t)ptr - offsetof(type,member))
@@ -27,13 +19,9 @@
  * - Matching NodeId: Return the entry
  * - NULL: Abort the search
  *
- * The nodestore uses atomic operations to set entries of the hash-map. If
- * UA_ENABLE_IMMUTABLE_NODES is configured, the nodestore allows read-access
- * from an interrupt without seeing corrupted nodes. For true multi-threaded
- * access, a mutex is used.
- *
- * Multi-threading without a mutex could be realized with the Linux RCU mechanism.
- * But this is not done for this implementation of the nodestore. */
+ * The nodestore uses atomic operations to set entries of the hash-map. So the
+ * nodestore allows read-access from an interrupt without seeing corrupted
+ * nodes. */
 
 typedef struct UA_NodeMapEntry {
     UA_UInt32 nodeIdHash;
@@ -51,9 +39,6 @@ typedef struct {
     UA_UInt32 size;
     UA_UInt32 count;
     UA_UInt32 sizePrimeIndex;
-#if UA_MULTITHREADING >= 100
-    UA_LOCK_TYPE(lock) /* Protect access */
-#endif
 } UA_NodeMap;
 
 /*********************/
@@ -196,15 +181,15 @@ createEntry(UA_NodeClass nodeClass) {
 }
 
 static void
-deleteEntry(UA_NodeMapEntry *entry) {
+deleteNodeMapEntry(UA_NodeMapEntry *entry) {
     UA_Node_clear(&entry->node);
     UA_free(entry);
 }
 
 static void
-cleanupEntry(UA_NodeMapEntry *entry) {
+cleanupNodeMapEntry(UA_NodeMapEntry *entry) {
     if(entry->deleted && entry->refCount == 0)
-        deleteEntry(entry);
+        deleteNodeMapEntry(entry);
 }
 
 static UA_StatusCode
@@ -213,7 +198,7 @@ clearSlot(UA_NodeMap *ns, UA_NodeMapEntry **slot) {
     if(UA_atomic_cmpxchg((void**)slot, entry, UA_NODEMAP_TOMBSTONE) != entry)
         return UA_STATUSCODE_BADINTERNALERROR;
     entry->deleted = true;
-    cleanupEntry(entry);
+    cleanupNodeMapEntry(entry);
     --ns->count;
     /* Downsize the hashmap if it is very empty */
     if(ns->count * 8 < ns->size && ns->size > 32)
@@ -265,27 +250,18 @@ UA_NodeMap_newNode(void *context, UA_NodeClass nodeClass) {
 
 static void
 UA_NodeMap_deleteNode(void *context, UA_Node *node) {
-#if UA_MULTITHREADING >= 100
-    UA_NodeMap *ns = (UA_NodeMap*)context;
-#endif
-    BEGIN_CRITSECT(ns);
     UA_NodeMapEntry *entry = container_of(node, UA_NodeMapEntry, node);
     UA_assert(&entry->node == node);
-    deleteEntry(entry);
-    END_CRITSECT(ns);
+    deleteNodeMapEntry(entry);
 }
 
 static const UA_Node *
 UA_NodeMap_getNode(void *context, const UA_NodeId *nodeid) {
     UA_NodeMap *ns = (UA_NodeMap*)context;
-    BEGIN_CRITSECT(ns);
     UA_NodeMapEntry **entry = findOccupiedSlot(ns, nodeid);
-    if(!entry) {
-        END_CRITSECT(ns);
+    if(!entry)
         return NULL;
-    }
     ++(*entry)->refCount;
-    END_CRITSECT(ns);
     return (const UA_Node*)&(*entry)->node;
 }
 
@@ -293,56 +269,43 @@ static void
 UA_NodeMap_releaseNode(void *context, const UA_Node *node) {
     if (!node)
         return;
-#if UA_MULTITHREADING >= 100
-    UA_NodeMap *ns = (UA_NodeMap*)context;
-#endif
-    BEGIN_CRITSECT(ns);
     UA_NodeMapEntry *entry = container_of(node, UA_NodeMapEntry, node);
     UA_assert(&entry->node == node);
     UA_assert(entry->refCount > 0);
     --entry->refCount;
-    cleanupEntry(entry);
-    END_CRITSECT(ns);
+    cleanupNodeMapEntry(entry);
 }
 
 static UA_StatusCode
 UA_NodeMap_getNodeCopy(void *context, const UA_NodeId *nodeid,
                        UA_Node **outNode) {
     UA_NodeMap *ns = (UA_NodeMap*)context;
-    BEGIN_CRITSECT(ns);
     UA_NodeMapEntry **slot = findOccupiedSlot(ns, nodeid);
-    if(!slot) {
-        END_CRITSECT(ns);
+    if(!slot)
         return UA_STATUSCODE_BADNODEIDUNKNOWN;
-    }
     UA_NodeMapEntry *entry = *slot;
     UA_NodeMapEntry *newItem = createEntry(entry->node.nodeClass);
-    if(!newItem) {
-        END_CRITSECT(ns);
+    if(!newItem)
         return UA_STATUSCODE_BADOUTOFMEMORY;
-    }
     UA_StatusCode retval = UA_Node_copy(&entry->node, &newItem->node);
     if(retval == UA_STATUSCODE_GOOD) {
         newItem->orig = entry; /* Store the pointer to the original */
         *outNode = &newItem->node;
     } else {
-        deleteEntry(newItem);
+        deleteNodeMapEntry(newItem);
     }
-    END_CRITSECT(ns);
     return retval;
 }
 
 static UA_StatusCode
 UA_NodeMap_removeNode(void *context, const UA_NodeId *nodeid) {
     UA_NodeMap *ns = (UA_NodeMap*)context;
-    BEGIN_CRITSECT(ns);
     UA_NodeMapEntry **slot = findOccupiedSlot(ns, nodeid);
     UA_StatusCode retval = UA_STATUSCODE_GOOD;
     if(slot)
         retval = clearSlot(ns, slot);
     else
         retval = UA_STATUSCODE_BADNODEIDUNKNOWN;
-    END_CRITSECT(ns);
     return retval;
 }
 
@@ -350,12 +313,9 @@ static UA_StatusCode
 UA_NodeMap_insertNode(void *context, UA_Node *node,
                       UA_NodeId *addedNodeId) {
     UA_NodeMap *ns = (UA_NodeMap*)context;
-    BEGIN_CRITSECT(ns);
     if(ns->size * 3 <= ns->count * 4) {
-        if(expand(ns) != UA_STATUSCODE_GOOD) {
-            END_CRITSECT(ns);
+        if(expand(ns) != UA_STATUSCODE_GOOD)
             return UA_STATUSCODE_BADINTERNALERROR;
-        }
     }
 
     UA_NodeMapEntry **slot;
@@ -389,8 +349,7 @@ UA_NodeMap_insertNode(void *context, UA_Node *node,
     }
 
     if(!slot) {
-        deleteEntry(container_of(node, UA_NodeMapEntry, node));
-        END_CRITSECT(ns);
+        deleteNodeMapEntry(container_of(node, UA_NodeMapEntry, node));
         return UA_STATUSCODE_BADNODEIDEXISTS;
     }
 
@@ -399,8 +358,7 @@ UA_NodeMap_insertNode(void *context, UA_Node *node,
     if(addedNodeId) {
         retval = UA_NodeId_copy(&node->nodeId, addedNodeId);
         if(retval != UA_STATUSCODE_GOOD) {
-            deleteEntry(container_of(node, UA_NodeMapEntry, node));
-            END_CRITSECT(ns);
+            deleteNodeMapEntry(container_of(node, UA_NodeMapEntry, node));
             return retval;
         }
     }
@@ -412,12 +370,11 @@ UA_NodeMap_insertNode(void *context, UA_Node *node,
     /* Insert the node */
     UA_NodeMapEntry *oldEntry = *slot;
     if(UA_atomic_cmpxchg((void**)slot, oldEntry, newEntry) != oldEntry) {
-        deleteEntry(container_of(node, UA_NodeMapEntry, node));
-        END_CRITSECT(ns);
+        deleteNodeMapEntry(container_of(node, UA_NodeMapEntry, node));
         return UA_STATUSCODE_BADNODEIDEXISTS;
     }
+
     ++ns->count;
-    END_CRITSECT(ns);
     return retval;
 }
 
@@ -426,21 +383,17 @@ UA_NodeMap_replaceNode(void *context, UA_Node *node) {
     UA_NodeMap *ns = (UA_NodeMap*)context;
     UA_NodeMapEntry *newEntry = container_of(node, UA_NodeMapEntry, node);
 
-    BEGIN_CRITSECT(ns);
-
     /* Find the node */
     UA_NodeMapEntry **slot = findOccupiedSlot(ns, &node->nodeId);
     if(!slot) {
-        deleteEntry(newEntry);
-        END_CRITSECT(ns);
+        deleteNodeMapEntry(newEntry);
         return UA_STATUSCODE_BADNODEIDUNKNOWN;
     }
     UA_NodeMapEntry *oldEntry = *slot;
 
     /* The node was already updated since the copy was made? */
     if(oldEntry != newEntry->orig) {
-        deleteEntry(newEntry);
-        END_CRITSECT(ns);
+        deleteNodeMapEntry(newEntry);
         return UA_STATUSCODE_BADINTERNALERROR;
     }
 
@@ -449,14 +402,12 @@ UA_NodeMap_replaceNode(void *context, UA_Node *node) {
 
     /* Replace the entry with an atomic operation */
     if(UA_atomic_cmpxchg((void**)slot, oldEntry, newEntry) != oldEntry) {
-        deleteEntry(newEntry);
-        END_CRITSECT(ns);
+        deleteNodeMapEntry(newEntry);
         return UA_STATUSCODE_BADINTERNALERROR;
     }
 
     oldEntry->deleted = true;
-    cleanupEntry(oldEntry);
-    END_CRITSECT(ns);
+    cleanupNodeMapEntry(oldEntry);
     return UA_STATUSCODE_GOOD;
 }
 
@@ -464,27 +415,20 @@ static void
 UA_NodeMap_iterate(void *context, UA_NodestoreVisitor visitor,
                    void *visitorContext) {
     UA_NodeMap *ns = (UA_NodeMap*)context;
-    BEGIN_CRITSECT(ns);
     for(UA_UInt32 i = 0; i < ns->size; ++i) {
         if(ns->entries[i] > UA_NODEMAP_TOMBSTONE) {
-            END_CRITSECT(ns);
             UA_NodeMapEntry *entry = ns->entries[i];
             entry->refCount++;
             visitor(visitorContext, &entry->node);
             entry->refCount--;
-            cleanupEntry(entry);
-            BEGIN_CRITSECT(ns);
+            cleanupNodeMapEntry(entry);
         }
     }
-    END_CRITSECT(ns);
 }
 
 static void
 UA_NodeMap_delete(void *context) {
     UA_NodeMap *ns = (UA_NodeMap*)context;
-#if UA_MULTITHREADING >= 100
-    UA_LOCK_DESTROY(ns->lock);
-#endif
     UA_UInt32 size = ns->size;
     UA_NodeMapEntry **entries = ns->entries;
     for(UA_UInt32 i = 0; i < size; ++i) {
@@ -492,7 +436,7 @@ UA_NodeMap_delete(void *context) {
             /* On debugging builds, check that all nodes were release */
             UA_assert(entries[i]->refCount == 0);
             /* Delete the node */
-            deleteEntry(entries[i]);
+            deleteNodeMapEntry(entries[i]);
         }
     }
     UA_free(ns->entries);
@@ -515,10 +459,6 @@ UA_Nodestore_HashMap(UA_Nodestore *ns) {
         return UA_STATUSCODE_BADOUTOFMEMORY;
     }
 
-#if UA_MULTITHREADING >= 100
-    UA_LOCK_INIT(nodemap->lock)
-#endif
-
     /* Populate the nodestore */
     ns->context = nodemap;
     ns->clear = UA_NodeMap_delete;

plugins/ua_nodestore_ziptree.c

@@ -9,14 +9,6 @@
 #include <open62541/plugin/nodestore_default.h>
 #include "ziptree.h"
 
-#if UA_MULTITHREADING >= 100
-#define BEGIN_CRITSECT(ctx) UA_LOCK(ctx->lock)
-#define END_CRITSECT(ctx) UA_UNLOCK(ctx->lock)
-#else
-#define BEGIN_CRITSECT(ctx) do {} while(0)
-#define END_CRITSECT(ctx) do {} while(0)
-#endif
-
 /* container_of */
 #define container_of(ptr, type, member) \
     (type *)((uintptr_t)ptr - offsetof(type,member))
@@ -56,9 +48,6 @@ typedef struct NodeTree NodeTree;
 
 typedef struct {
     NodeTree root;
-#if UA_MULTITHREADING >= 100
-    UA_LOCK_TYPE(lock) /* Protect access */
-#endif
 } ZipContext;
 
 ZIP_PROTTYPE(NodeTree, NodeEntry, NodeEntry)
@@ -137,17 +126,13 @@ zipNsDeleteNode(void *nsCtx, UA_Node *node) {
 static const UA_Node *
 zipNsGetNode(void *nsCtx, const UA_NodeId *nodeId) {
     ZipContext *ns = (ZipContext*)nsCtx;
-    BEGIN_CRITSECT(ns);
     NodeEntry dummy;
     dummy.nodeIdHash = UA_NodeId_hash(nodeId);
     dummy.nodeId = *nodeId;
     NodeEntry *entry = ZIP_FIND(NodeTree, &ns->root, &dummy);
-    if(!entry) {
-        END_CRITSECT(ns);
+    if(!entry)
         return NULL;
-    }
     ++entry->refCount;
-    END_CRITSECT(ns);
     return (const UA_Node*)&entry->nodeId;
 }
 
@@ -155,15 +140,10 @@ static void
 zipNsReleaseNode(void *nsCtx, const UA_Node *node) {
     if(!node)
         return;
-#if UA_MULTITHREADING >= 100
-        ZipContext *ns = (ZipContext*)nsCtx;
-#endif
-    BEGIN_CRITSECT(ns);
     NodeEntry *entry = container_of(node, NodeEntry, nodeId);
     UA_assert(entry->refCount > 0);
     --entry->refCount;
     cleanupEntry(entry);
-    END_CRITSECT(ns);
 }
 
 static UA_StatusCode
@@ -199,7 +179,6 @@ static UA_StatusCode
 zipNsInsertNode(void *nsCtx, UA_Node *node, UA_NodeId *addedNodeId) {
     NodeEntry *entry = container_of(node, NodeEntry, nodeId);
     ZipContext *ns = (ZipContext*)nsCtx;
-    BEGIN_CRITSECT(ns);
 
     /* Ensure that the NodeId is unique */
     NodeEntry dummy;
@@ -215,7 +194,6 @@ zipNsInsertNode(void *nsCtx, UA_Node *node, UA_NodeId *addedNodeId) {
         dummy.nodeIdHash = UA_NodeId_hash(&node->nodeId);
         if(ZIP_FIND(NodeTree, &ns->root, &dummy)) { /* The nodeid exists */
             deleteEntry(entry);
-            END_CRITSECT(ns);
             return UA_STATUSCODE_BADNODEIDEXISTS;
         }
     }
@@ -225,7 +203,6 @@ zipNsInsertNode(void *nsCtx, UA_Node *node, UA_NodeId *addedNodeId) {
         UA_StatusCode retval = UA_NodeId_copy(&node->nodeId, addedNodeId);
         if(retval != UA_STATUSCODE_GOOD) {
             deleteEntry(entry);
-            END_CRITSECT(ns);
             return retval;
         }
     }
@@ -233,7 +210,6 @@ zipNsInsertNode(void *nsCtx, UA_Node *node, UA_NodeId *addedNodeId) {
     /* Insert the node */
     entry->nodeIdHash = dummy.nodeIdHash;
     ZIP_INSERT(NodeTree, &ns->root, entry, ZIP_FFS32(UA_UInt32_random()));
-    END_CRITSECT(ns);
     return UA_STATUSCODE_GOOD;
 }
 
@@ -258,12 +234,10 @@ zipNsReplaceNode(void *nsCtx, UA_Node *node) {
 
     /* Replace */
     ZipContext *ns = (ZipContext*)nsCtx;
-    BEGIN_CRITSECT(ns);
     ZIP_REMOVE(NodeTree, &ns->root, oldEntry);
     entry->nodeIdHash = oldEntry->nodeIdHash;
     ZIP_INSERT(NodeTree, &ns->root, entry, ZIP_RANK(entry, zipfields));
     oldEntry->deleted = true;
-    END_CRITSECT(ns);
 
     zipNsReleaseNode(nsCtx, oldNode);
     return UA_STATUSCODE_GOOD;
@@ -272,19 +246,15 @@ zipNsReplaceNode(void *nsCtx, UA_Node *node) {
 static UA_StatusCode
 zipNsRemoveNode(void *nsCtx, const UA_NodeId *nodeId) {
     ZipContext *ns = (ZipContext*)nsCtx;
-    BEGIN_CRITSECT(ns);
     NodeEntry dummy;
     dummy.nodeIdHash = UA_NodeId_hash(nodeId);
     dummy.nodeId = *nodeId;
     NodeEntry *entry = ZIP_FIND(NodeTree, &ns->root, &dummy);
-    if(!entry) {
-        END_CRITSECT(ns);
+    if(!entry)
         return UA_STATUSCODE_BADNODEIDUNKNOWN;
-    }
     ZIP_REMOVE(NodeTree, &ns->root, entry);
     entry->deleted = true;
     cleanupEntry(entry);
-    END_CRITSECT(ns);
     return UA_STATUSCODE_GOOD;
 }
 
@@ -306,9 +276,7 @@ zipNsIterate(void *nsCtx, UA_NodestoreVisitor visitor,
     d.visitor = visitor;
     d.visitorContext = visitorCtx;
     ZipContext *ns = (ZipContext*)nsCtx;
-    BEGIN_CRITSECT(ns);
     ZIP_ITER(NodeTree, &ns->root, nodeVisitor, &d);
-    END_CRITSECT(ns);
 }
 
 static void
@@ -325,9 +293,6 @@ zipNsClear(void *nsCtx) {
     if (!nsCtx)
         return;
     ZipContext *ns = (ZipContext*)nsCtx;
-#if UA_MULTITHREADING >= 100
-    UA_LOCK_DESTROY(ns->lock);
-#endif
     ZIP_ITER(NodeTree, &ns->root, deleteNodeVisitor, NULL);
     UA_free(ns);
 }
@@ -338,9 +303,6 @@ UA_Nodestore_ZipTree(UA_Nodestore *ns) {
     ZipContext *ctx = (ZipContext*)UA_malloc(sizeof(ZipContext));
     if(!ctx)
         return UA_STATUSCODE_BADOUTOFMEMORY;
-#if UA_MULTITHREADING >= 100
-    UA_LOCK_INIT(ctx->lock)
-#endif
 
     ZIP_INIT(&ctx->root);