first try to repair variables, then typecheck

src/server/ua_server_utils.c

@@ -273,7 +273,9 @@ UA_Server_editNode(UA_Server *server, UA_Session *session,
 #else
     UA_StatusCode retval;
     do {
+        UA_RCU_LOCK();
         UA_Node *copy = UA_NodeStore_getCopy(server->nodestore, nodeId);
+        UA_RCU_UNLOCK();
         if(!copy)
             return UA_STATUSCODE_BADOUTOFMEMORY;
         retval = callback(server, session, copy, data);
@@ -281,7 +283,9 @@ UA_Server_editNode(UA_Server *server, UA_Session *session,
             UA_NodeStore_deleteNode(copy);
             return retval;
         }
+        UA_RCU_LOCK();
         retval = UA_NodeStore_replace(server->nodestore, copy);
+        UA_RCU_UNLOCK();
     } while(retval != UA_STATUSCODE_GOOD);
     return UA_STATUSCODE_GOOD;
 #endif

src/server/ua_services_nodemanagement.c

@@ -92,42 +92,8 @@ typeCheckVariableNodeWithValue(UA_Server *server, UA_Session *session,
                                const UA_VariableNode *node,
                                const UA_VariableTypeNode *vt,
                                UA_DataValue *value) {
-    /* Workaround: set a sane valueRank (the most permissive -2) */
-    UA_Int32 valueRank = node->valueRank;
-    if(valueRank == 0 &&
-       (!value->hasValue || !value->value.type || UA_Variant_isScalar(&value->value))) {
-        UA_LOG_INFO_SESSION(server->config.logger, session,
-                            "AddNodes: Use a default ValueRank of -2");
-        valueRank = -2;
-        UA_StatusCode retval = UA_Server_writeValueRank(server, node->nodeId, valueRank);
-        if(retval != UA_STATUSCODE_GOOD)
-            return retval;
-    }
-
-    /* If no value is set, see if the vt provides one and copy that. This needs to be done
-     * before copying the datatype from the vt. Setting the datatype triggers the
-     * typecheck. Here, we have only a typecheck when the datatype is already not null. */
-    if(!value->hasValue || !value->value.type) {
-        UA_StatusCode retval = readValueAttribute(server, (const UA_VariableNode*)vt, value);
-        if(retval == UA_STATUSCODE_GOOD && value->hasValue && value->value.type)
-            retval = UA_Server_writeValue(server, node->nodeId, value->value);
-        if(retval != UA_STATUSCODE_GOOD)
-            return retval;
-    }
-
-    /* Workaround: Replace with datatype of the vt if not set */
-    const UA_NodeId *dataType = &node->dataType;
-    if(UA_NodeId_isNull(dataType)) {
-        UA_LOG_INFO_SESSION(server->config.logger, session, "AddNodes: "
-                            "Use a default DataType (from the TypeDefinition)");
-        dataType = &vt->dataType;
-        UA_StatusCode retval = UA_Server_writeDataType(server, node->nodeId, vt->dataType);
-        if(retval != UA_STATUSCODE_GOOD)
-            return retval;
-    }
-
     /* Check the datatype against the vt */
-    if(!compatibleDataType(server, dataType, &vt->dataType))
+    if(!compatibleDataType(server, &node->dataType, &vt->dataType))
         return UA_STATUSCODE_BADTYPEMISMATCH;
 
     /* Get the array dimensions */
@@ -138,12 +104,12 @@ typeCheckVariableNodeWithValue(UA_Server *server, UA_Session *session,
     }
 
     /* Check valueRank against array dimensions */
-    UA_StatusCode retval = compatibleValueRankArrayDimensions(valueRank, arrayDims);
+    UA_StatusCode retval = compatibleValueRankArrayDimensions(node->valueRank, arrayDims);
     if(retval != UA_STATUSCODE_GOOD)
         return retval;
 
     /* Check valueRank against the vt */
-    retval = compatibleValueRanks(valueRank, vt->valueRank);
+    retval = compatibleValueRanks(node->valueRank, vt->valueRank);
     if(retval != UA_STATUSCODE_GOOD)
         return retval;
 
@@ -155,13 +121,16 @@ typeCheckVariableNodeWithValue(UA_Server *server, UA_Session *session,
 
     /* Typecheck the value */
     if(value->hasValue) {
-        retval = typeCheckValue(server, dataType, valueRank,
+        retval = typeCheckValue(server, &node->dataType, node->valueRank,
                                 node->arrayDimensionsSize, node->arrayDimensions,
                                 &value->value, NULL, NULL);
         /* The type-check failed. Write the same value again. The write-service
          * tries to convert to the correct type... */
-        if(retval != UA_STATUSCODE_GOOD)
+        if(retval != UA_STATUSCODE_GOOD) {
+            UA_RCU_UNLOCK();
             retval = UA_Server_writeValue(server, node->nodeId, value->value);
+            UA_RCU_LOCK();
+        }
     }
     return retval;
 }
@@ -195,8 +164,55 @@ typeCheckVariableNode(UA_Server *server, UA_Session *session,
     if(retval != UA_STATUSCODE_GOOD)
         return retval;
 
-    retval = typeCheckVariableNodeWithValue(server, session, node, vt, &value);
+    /* Fix the variable: Set a sane valueRank if required (the most permissive -2) */
+    if(node->valueRank == 0 &&
+       (!value.hasValue || !value.value.type || UA_Variant_isScalar(&value.value))) {
+        UA_LOG_INFO_SESSION(server->config.logger, session,
+                            "AddNodes: Use a default ValueRank of -2");
+        UA_RCU_UNLOCK();
+        retval = UA_Server_writeValueRank(server, node->nodeId, -2);
+        UA_RCU_LOCK();
+        if(retval != UA_STATUSCODE_GOOD) {
+            UA_DataValue_deleteMembers(&value);
+            return retval;
+        }
+    }
+
+    /* If no value is set, see if the vt provides one and copy that. This needs
+     * to be done before copying the datatype from the vt, as setting the datatype
+     * triggers a typecheck. */
+    if(!value.hasValue || !value.value.type) {
+        retval = readValueAttribute(server, (const UA_VariableNode*)vt, &value);
+        if(retval == UA_STATUSCODE_GOOD && value.hasValue && value.value.type) {
+            UA_RCU_UNLOCK();
+            retval = UA_Server_writeValue(server, node->nodeId, value.value);
+            UA_RCU_LOCK();
+        }
+        if(retval != UA_STATUSCODE_GOOD) {
+            UA_DataValue_deleteMembers(&value);
+            return retval;
+        }
+    }
 
+    /* Fix the variable: If no datatype is given, use the datatype of the vt */
+    if(UA_NodeId_isNull(&node->dataType)) {
+        UA_LOG_INFO_SESSION(server->config.logger, session, "AddNodes: "
+                            "Use a default DataType (from the TypeDefinition)");
+        UA_RCU_UNLOCK();
+        retval = UA_Server_writeDataType(server, node->nodeId, vt->dataType);
+        UA_RCU_LOCK();
+        if(retval != UA_STATUSCODE_GOOD) {
+            UA_DataValue_deleteMembers(&value);
+            return retval;
+        }
+    }
+
+#ifdef UA_ENABLE_MULTITHREADING
+    /* Re-read the node to get the changes */
+    node = (const UA_VariableNode*)UA_NodeStore_get(server->nodestore, &node->nodeId);
+#endif
+
+    retval = typeCheckVariableNodeWithValue(server, session, node, vt, &value);
     UA_DataValue_deleteMembers(&value);
     return retval;
 }
@@ -444,10 +460,13 @@ instantiateNode(UA_Server *server, UA_Session *session, const UA_NodeId *nodeId,
     if(typenode->nodeClass == UA_NODECLASS_OBJECTTYPE) {
         const UA_ObjectLifecycleManagement *olm =
             &((const UA_ObjectTypeNode*)typenode)->lifecycleManagement;
-        if(olm->constructor)
+        if(olm->constructor) {
+            UA_RCU_UNLOCK();
             UA_Server_editNode(server, session, nodeId,
                                (UA_EditNodeCallback)setObjectInstanceHandle,
                                olm->constructor);
+            UA_RCU_LOCK();
+        }
     }
 
     /* Add a hasType reference */
@@ -861,7 +880,9 @@ __UA_Server_addNode(UA_Server *server, const UA_NodeClass nodeClass,
     /* Call the normal addnodes service */
     UA_AddNodesResult result;
     UA_AddNodesResult_init(&result);
+    UA_RCU_LOCK();
     Service_AddNodes_single(server, &adminSession, &item, &result, instantiationCallback);
+    UA_RCU_UNLOCK();
     if(outNewNodeId)
         *outNewNodeId = result.addedNodeId;
     else
@@ -889,11 +910,13 @@ __UA_Server_addNode_begin(UA_Server *server, const UA_NodeClass nodeClass,
     /* Add the node without checks or instantiation */
     UA_AddNodesResult result;
     UA_AddNodesResult_init(&result);
+    UA_RCU_LOCK();
     Service_AddNode_begin(server, &adminSession, &item, &result);
     if(outNewNodeId)
         *outNewNodeId = result.addedNodeId;
     else
         UA_NodeId_deleteMembers(&result.addedNodeId);
+    UA_RCU_UNLOCK();
     return result.statusCode;
 }
 
@@ -903,9 +926,12 @@ UA_Server_addNode_finish(UA_Server *server, const UA_NodeId nodeId,
                          const UA_NodeId referenceTypeId,
                          const UA_NodeId typeDefinition,
                          UA_InstantiationCallback *instantiationCallback) {
-    return Service_AddNode_finish(server, &adminSession, &nodeId, &parentNodeId,
-                                  &referenceTypeId, &typeDefinition,
-                                  instantiationCallback);
+    UA_RCU_LOCK();
+    UA_StatusCode retval = Service_AddNode_finish(server, &adminSession, &nodeId, &parentNodeId,
+                                                  &referenceTypeId, &typeDefinition,
+                                                  instantiationCallback);
+    UA_RCU_UNLOCK();
+    return retval;
 }
 
 /**************************************************/
@@ -1002,11 +1028,15 @@ UA_Server_addMethodNode_finish(UA_Server *server, const UA_NodeId nodeId,
     
     UA_BrowseResult br;
     UA_BrowseResult_init(&br);
+    UA_RCU_LOCK();
     Service_Browse_single(server, &adminSession, NULL, &bd, 0, &br);
+    UA_RCU_UNLOCK();
 
     UA_StatusCode retval = br.statusCode;
     if(retval != UA_STATUSCODE_GOOD) {
+        UA_RCU_LOCK();
         Service_DeleteNodes_single(server, &adminSession, &nodeId, true);
+        UA_RCU_UNLOCK();
         UA_BrowseResult_deleteMembers(&br);
         return retval;
     }
@@ -1061,8 +1091,10 @@ UA_Server_addMethodNode_finish(UA_Server *server, const UA_NodeId nodeId,
     }
 
     /* Call finish to add the parent reference */
+    UA_RCU_LOCK();
     retval |= Service_AddNode_finish(server, &adminSession, &nodeId, &parentNodeId,
                                      &referenceTypeId, &UA_NODEID_NULL, NULL);
+    UA_RCU_UNLOCK();
 
     if(retval != UA_STATUSCODE_GOOD) {
         Service_DeleteNodes_single(server, &adminSession, &nodeId, true);
@@ -1153,9 +1185,11 @@ Service_AddReferences_single(UA_Server *server, UA_Session *session,
 
     /* Add the first direction */
 #ifndef UA_ENABLE_EXTERNAL_NAMESPACES
+    UA_RCU_UNLOCK();
     UA_StatusCode retval =
         UA_Server_editNode(server, session, &item->sourceNodeId,
                            (UA_EditNodeCallback)addOneWayReference, item);
+    UA_RCU_LOCK();
 #else
     UA_StatusCode retval = UA_STATUSCODE_GOOD;
     UA_Boolean handledExternally = UA_FALSE;
@@ -1169,9 +1203,12 @@ Service_AddReferences_single(UA_Server *server, UA_Session *session,
             break;
         }
     }
-    if(!handledExternally)
+    if(!handledExternally) {
+        UA_RCU_UNLOCK();
         retval = UA_Server_editNode(server, session, &item->sourceNodeId,
                                     (UA_EditNodeCallback)addOneWayReference, item);
+        UA_RCU_LOCK();
+    }
 #endif
 
     if(retval != UA_STATUSCODE_GOOD)
@@ -1186,8 +1223,10 @@ Service_AddReferences_single(UA_Server *server, UA_Session *session,
     secondItem.targetNodeId.nodeId = item->sourceNodeId;
     /* keep default secondItem.targetNodeClass = UA_NODECLASS_UNSPECIFIED */
 #ifndef UA_ENABLE_EXTERNAL_NAMESPACES
+    UA_RCU_UNLOCK();
     retval = UA_Server_editNode(server, session, &secondItem.sourceNodeId,
                                 (UA_EditNodeCallback)addOneWayReference, &secondItem);
+    UA_RCU_LOCK();
 #else
     handledExternally = UA_FALSE;
     for(size_t j = 0; j < server->externalNamespacesSize; ++j) {
@@ -1200,9 +1239,12 @@ Service_AddReferences_single(UA_Server *server, UA_Session *session,
             break;
         }
     }
-    if(!handledExternally)
+    if(!handledExternally) {
+        UA_RCU_UNLOCK();
         retval = UA_Server_editNode(server, session, &secondItem.sourceNodeId,
                                     (UA_EditNodeCallback)addOneWayReference, &secondItem);
+        UA_RCU_LOCK();
+    }
 #endif
 
     /* remove reference if the second direction failed */
@@ -1214,8 +1256,10 @@ Service_AddReferences_single(UA_Server *server, UA_Session *session,
         deleteItem.targetNodeId = item->targetNodeId;
         deleteItem.deleteBidirectional = false;
         /* ignore returned status code */
+        UA_RCU_UNLOCK();
         UA_Server_editNode(server, session, &item->sourceNodeId,
                            (UA_EditNodeCallback)deleteOneWayReference, &deleteItem);
+        UA_RCU_LOCK();
     }
     return retval;
 }
@@ -1519,11 +1563,9 @@ setDataSource(UA_Server *server, UA_Session *session,
 UA_StatusCode
 UA_Server_setVariableNode_dataSource(UA_Server *server, const UA_NodeId nodeId,
                                      const UA_DataSource dataSource) {
-    UA_RCU_LOCK();
     UA_StatusCode retval = UA_Server_editNode(server, &adminSession, &nodeId,
                                               (UA_EditNodeCallback)setDataSource,
                                               &dataSource);
-    UA_RCU_UNLOCK();
     return retval;
 }
 
@@ -1543,10 +1585,8 @@ setOLM(UA_Server *server, UA_Session *session,
 UA_StatusCode UA_EXPORT
 UA_Server_setObjectTypeNode_lifecycleManagement(UA_Server *server, UA_NodeId nodeId,
                                                 UA_ObjectLifecycleManagement olm) {
-    UA_RCU_LOCK();
     UA_StatusCode retval = UA_Server_editNode(server, &adminSession, &nodeId,
                                               (UA_EditNodeCallback)setOLM, &olm);
-    UA_RCU_UNLOCK();
     return retval;
 }
 
@@ -1580,11 +1620,9 @@ UA_Server_setMethodNode_callback(UA_Server *server, const UA_NodeId methodNodeId
     cb.callback = method;
     cb.handle = handle;
 
-    UA_RCU_LOCK();
     UA_StatusCode retval =
         UA_Server_editNode(server, &adminSession,
                            &methodNodeId, editMethodCallback, &cb);
-    UA_RCU_UNLOCK();
     return retval;
 }