Browse Source

zero-copy read service

Julius Pfrommer 7 years ago
parent
commit
92b1758a64

+ 14 - 2
include/ua_server.h

@@ -638,6 +638,18 @@ UA_Server_setNodeContext(UA_Server *server, UA_NodeId nodeId,
  * be set to a null-pointer. */
 typedef struct {
     /* Copies the data from the source into the provided value.
+     *
+     * !! ZERO-COPY OPERATIONS POSSIBLE !!
+     * It is not required to return a copy of the actual content data. You can
+     * return a pointer to memory owned by the user. Memory can be reused
+     * between read callbacks of a DataSource, as the result is already encoded
+     * on the network buffer between each read operation.
+     *
+     * To use zero-copy reads, set the value of the `value->value` Variant
+     * without copying, e.g. with `UA_Variant_setScalar`. Then, also set
+     * `value->value.storageType` to `UA_VARIANT_DATA_NODELETE` to prevent the
+     * memory being cleaned up. Don't forget to also set `value->hasValue` to
+     * true to indicate the presence of a value.
      *
      * @param handle An optional pointer to user-defined data for the
      *        specific data source
@@ -659,8 +671,8 @@ typedef struct {
                           void *nodeContext, UA_Boolean includeSourceTimeStamp,
                           const UA_NumericRange *range, UA_DataValue *value);
 
-    /* Write into a data source. The write member of UA_DataSource can be empty
-     * if the operation is unsupported.
+    /* Write into a data source. This method pointer can be NULL if the
+     * operation is unsupported.
      *
      * @param handle An optional pointer to user-defined data for the
      *        specific data source

+ 55 - 11
src/server/ua_server_binary.c

@@ -50,16 +50,23 @@ sendServiceFault(UA_SecureChannel *channel, const UA_ByteString *msg,
     // Send error message. Message type is MSG and not ERR, since we are on a securechannel!
     retval = UA_SecureChannel_sendSymmetricMessage(channel, requestId, UA_MESSAGETYPE_MSG,
                                                    response, responseType);
+
     UA_RequestHeader_deleteMembers(&requestHeader);
     UA_LOG_DEBUG(channel->securityPolicy->logger, UA_LOGCATEGORY_SERVER,
                  "Sent ServiceFault with error code %s", UA_StatusCode_name(error));
     return retval;
 }
 
+typedef enum {
+    UA_SERVICETYPE_NORMAL,
+    UA_SERVICETYPE_INSITU,
+    UA_SERVICETYPE_CUSTOM
+} UA_ServiceType;
+
 static void
 getServicePointers(UA_UInt32 requestTypeId, const UA_DataType **requestType,
                    const UA_DataType **responseType, UA_Service *service,
-                   UA_Boolean *requiresSession) {
+                   UA_Boolean *requiresSession, UA_ServiceType *serviceType) {
     switch(requestTypeId) {
     case UA_NS0ID_GETENDPOINTSREQUEST_ENCODING_DEFAULTBINARY:
         *service = (UA_Service)Service_GetEndpoints;
@@ -100,11 +107,13 @@ getServicePointers(UA_UInt32 requestTypeId, const UA_DataType **requestType,
         *requestType = &UA_TYPES[UA_TYPES_CREATESESSIONREQUEST];
         *responseType = &UA_TYPES[UA_TYPES_CREATESESSIONRESPONSE];
         *requiresSession = false;
+        *serviceType = UA_SERVICETYPE_CUSTOM;
         break;
     case UA_NS0ID_ACTIVATESESSIONREQUEST_ENCODING_DEFAULTBINARY:
         *service = (UA_Service)Service_ActivateSession;
         *requestType = &UA_TYPES[UA_TYPES_ACTIVATESESSIONREQUEST];
         *responseType = &UA_TYPES[UA_TYPES_ACTIVATESESSIONRESPONSE];
+        *serviceType = UA_SERVICETYPE_CUSTOM;
         break;
     case UA_NS0ID_CLOSESESSIONREQUEST_ENCODING_DEFAULTBINARY:
         *service = (UA_Service)Service_CloseSession;
@@ -115,6 +124,7 @@ getServicePointers(UA_UInt32 requestTypeId, const UA_DataType **requestType,
         *service = (UA_Service)Service_Read;
         *requestType = &UA_TYPES[UA_TYPES_READREQUEST];
         *responseType = &UA_TYPES[UA_TYPES_READRESPONSE];
+        *serviceType = UA_SERVICETYPE_INSITU;
         break;
     case UA_NS0ID_WRITEREQUEST_ENCODING_DEFAULTBINARY:
         *service = (UA_Service)Service_Write;
@@ -381,8 +391,9 @@ processMSG(UA_Server *server, UA_SecureChannel *channel,
     const UA_DataType *requestType = NULL;
     const UA_DataType *responseType = NULL;
     UA_Boolean sessionRequired = true;
+    UA_ServiceType serviceType = UA_SERVICETYPE_NORMAL;
     getServicePointers(requestTypeId.identifier.numeric, &requestType,
-                       &responseType, &service, &sessionRequired);
+                       &responseType, &service, &sessionRequired, &serviceType);
     if(!requestType) {
         if(requestTypeId.identifier.numeric == 787) {
             UA_LOG_INFO_CHANNEL(server->config.logger, channel,
@@ -509,26 +520,59 @@ processMSG(UA_Server *server, UA_SecureChannel *channel,
     }
 #endif
 
-    /* Call the service */
-    UA_assert(service); /* For all services besides publish, the service pointer is non-NULL*/
-    service(server, session, request, response);
+    send_response:
 
-send_response:
-    /* Send the response */
+    /* Prepare the ResponseHeader */
     ((UA_ResponseHeader*)response)->requestHandle = requestHeader->requestHandle;
     ((UA_ResponseHeader*)response)->timestamp = UA_DateTime_now();
-    retval = UA_SecureChannel_sendSymmetricMessage(channel, requestId, UA_MESSAGETYPE_MSG,
-                                                   response, responseType);
 
+    /* Start the message */
+    UA_NodeId typeId = UA_NODEID_NUMERIC(0, responseType->binaryEncodingId);
+    UA_MessageContext mc;
+    retval = UA_MessageContext_begin(&mc, channel, requestId, UA_MESSAGETYPE_MSG);
+    if(retval != UA_STATUSCODE_GOOD)
+        goto cleanup;
+
+    /* Assert's required for clang-analyzer */
+    UA_assert(mc.buf_pos == &mc.messageBuffer.data[UA_SECURE_MESSAGE_HEADER_LENGTH]);
+    UA_assert(mc.buf_end == &mc.messageBuffer.data[mc.messageBuffer.length]);
+
+    retval = UA_MessageContext_encode(&mc, &typeId, &UA_TYPES[UA_TYPES_NODEID]);
+    if(retval != UA_STATUSCODE_GOOD)
+        goto cleanup;
+
+    switch(serviceType) {
+    case UA_SERVICETYPE_CUSTOM:
+        /* Was processed before...*/
+        retval = UA_MessageContext_encode(&mc, response, responseType);
+        break;
+    case UA_SERVICETYPE_INSITU: 
+        retval = ((UA_InSituService)service)
+            (server, session, &mc, request, (UA_ResponseHeader*)response);
+        break;
+    case UA_SERVICETYPE_NORMAL:
+    default:
+        service(server, session, request, response);
+        retval = UA_MessageContext_encode(&mc, response, responseType);
+        break;
+    }
+
+    /* Finish sending the message */
+    if(retval != UA_STATUSCODE_GOOD) {
+        UA_MessageContext_abort(&mc);
+        goto cleanup;
+    }
+
+    retval = UA_MessageContext_finish(&mc);
+
+ cleanup:
     if(retval != UA_STATUSCODE_GOOD)
         UA_LOG_INFO_CHANNEL(server->config.logger, channel,
                             "Could not send the message over the SecureChannel "
                             "with StatusCode %s", UA_StatusCode_name(retval));
-
     /* Clean up */
     UA_deleteMembers(request, requestType);
     UA_deleteMembers(response, responseType);
-
     return retval;
 }
 

+ 1 - 1
src/server/ua_server_internal.h

@@ -285,7 +285,7 @@ void Service_Browse_single(UA_Server *server, UA_Session *session,
 UA_DataValue
 UA_Server_readWithSession(UA_Server *server, UA_Session *session,
                           const UA_ReadValueId *item,
-                          UA_TimestampsToReturn timestamps);
+                          UA_TimestampsToReturn timestampsToReturn);
 
 /* Checks if a registration timed out and removes that registration.
  * Should be called periodically in main loop */

+ 5 - 3
src/server/ua_services.h

@@ -38,6 +38,9 @@ extern "C" {
 typedef void (*UA_Service)(UA_Server*, UA_Session*,
                            const void *request, void *response);
 
+typedef UA_StatusCode (*UA_InSituService)(UA_Server*, UA_Session*, UA_MessageContext *mc,
+                                          const void *request, UA_ResponseHeader *rh);
+
 /**
  * Discovery Service Set
  * ---------------------
@@ -293,9 +296,8 @@ void Service_UnregisterNodes(UA_Server *server, UA_Session *session,
  * elements are indexed, such as an array, this Service allows Clients to read
  * the entire set of indexed values as a composite, to read individual elements
  * or to read ranges of elements of the composite. */
-void Service_Read(UA_Server *server, UA_Session *session,
-                  const UA_ReadRequest *request,
-                  UA_ReadResponse *response);
+UA_StatusCode Service_Read(UA_Server *server, UA_Session *session, UA_MessageContext *mc,
+                           const UA_ReadRequest *request, UA_ResponseHeader *responseHeader);
 
 /**
  * Write Service

+ 143 - 88
src/server/ua_services_attribute.c

@@ -3,6 +3,7 @@
  * file, You can obtain one at http://mozilla.org/MPL/2.0/. */
 
 #include "ua_server_internal.h"
+#include "ua_types_encoding_binary.h"
 #include "ua_services.h"
 
 /******************/
@@ -56,10 +57,16 @@ readArrayDimensionsAttribute(const UA_VariableNode *vn, UA_DataValue *v) {
     UA_Variant_setArray(&v->value, vn->arrayDimensions,
                         vn->arrayDimensionsSize, &UA_TYPES[UA_TYPES_UINT32]);
     v->value.storageType = UA_VARIANT_DATA_NODELETE;
-    v->hasValue = true;
     return UA_STATUSCODE_GOOD;
 }
 
+static void
+setScalarNoDelete(UA_Variant *v, const void * UA_RESTRICT p,
+                  const UA_DataType *type) {
+    UA_Variant_setScalar(v, (void*)(uintptr_t)p, type);
+    v->storageType = UA_VARIANT_DATA_NODELETE;
+}
+
 static UA_StatusCode
 readIsAbstractAttribute(const UA_Node *node, UA_Variant *v) {
     const UA_Boolean *isAbstract;
@@ -79,7 +86,10 @@ readIsAbstractAttribute(const UA_Node *node, UA_Variant *v) {
     default:
         return UA_STATUSCODE_BADATTRIBUTEIDINVALID;
     }
-    return UA_Variant_setScalarCopy(v, isAbstract, &UA_TYPES[UA_TYPES_BOOLEAN]);
+
+    setScalarNoDelete(v, isAbstract, &UA_TYPES[UA_TYPES_BOOLEAN]);
+    v->storageType = UA_VARIANT_DATA_NODELETE;
+    return UA_STATUSCODE_GOOD;
 }
 
 static UA_StatusCode
@@ -151,18 +161,19 @@ readValueAttribute(UA_Server *server, UA_Session *session,
 static const UA_String binEncoding = {sizeof("Default Binary")-1, (UA_Byte*)"Default Binary"};
 /* static const UA_String xmlEncoding = {sizeof("Default Xml")-1, (UA_Byte*)"Default Xml"}; */
 
-/* Thread-local variables to pass additional arguments into the operation */
-static UA_THREAD_LOCAL UA_TimestampsToReturn op_timestampsToReturn;
-
 #define CHECK_NODECLASS(CLASS)                                  \
     if(!(node->nodeClass & (CLASS))) {                          \
         retval = UA_STATUSCODE_BADATTRIBUTEIDINVALID;           \
         break;                                                  \
     }
 
+/* Returns a datavalue that may point into the node via the
+ * UA_VARIANT_DATA_NODELETE tag. Don't access the returned DataValue once the
+ * node has been released! */
 static void
-Operation_Read(UA_Server *server, UA_Session *session,
-               const UA_ReadValueId *id, UA_DataValue *v) {
+Read(const UA_Node *node, UA_Server *server, UA_Session *session,
+     UA_TimestampsToReturn timestampsToReturn,
+     const UA_ReadValueId *id, UA_DataValue *v) {
     UA_LOG_DEBUG_SESSION(server->config.logger, session,
                          "Read the attribute %i", id->attributeId);
 
@@ -181,34 +192,26 @@ Operation_Read(UA_Server *server, UA_Session *session,
         return;
     }
 
-    /* Get the node */
-    const UA_Node *node = UA_Nodestore_get(server, &id->nodeId);
-    if(!node) {
-        v->hasStatus = true;
-        v->status = UA_STATUSCODE_BADNODEIDUNKNOWN;
-        return;
-    }
-
     /* Read the attribute */
     UA_StatusCode retval = UA_STATUSCODE_GOOD;
     switch(id->attributeId) {
     case UA_ATTRIBUTEID_NODEID:
-        retval = UA_Variant_setScalarCopy(&v->value, &node->nodeId, &UA_TYPES[UA_TYPES_NODEID]);
+        setScalarNoDelete(&v->value, &node->nodeId, &UA_TYPES[UA_TYPES_NODEID]);
         break;
     case UA_ATTRIBUTEID_NODECLASS:
-        retval = UA_Variant_setScalarCopy(&v->value, &node->nodeClass, &UA_TYPES[UA_TYPES_NODECLASS]);
+        setScalarNoDelete(&v->value, &node->nodeClass, &UA_TYPES[UA_TYPES_NODECLASS]);
         break;
     case UA_ATTRIBUTEID_BROWSENAME:
-        retval = UA_Variant_setScalarCopy(&v->value, &node->browseName, &UA_TYPES[UA_TYPES_QUALIFIEDNAME]);
+        setScalarNoDelete(&v->value, &node->browseName, &UA_TYPES[UA_TYPES_QUALIFIEDNAME]);
         break;
     case UA_ATTRIBUTEID_DISPLAYNAME:
-        retval = UA_Variant_setScalarCopy(&v->value, &node->displayName, &UA_TYPES[UA_TYPES_LOCALIZEDTEXT]);
+        setScalarNoDelete(&v->value, &node->displayName, &UA_TYPES[UA_TYPES_LOCALIZEDTEXT]);
         break;
     case UA_ATTRIBUTEID_DESCRIPTION:
-        retval = UA_Variant_setScalarCopy(&v->value, &node->description, &UA_TYPES[UA_TYPES_LOCALIZEDTEXT]);
+        setScalarNoDelete(&v->value, &node->description, &UA_TYPES[UA_TYPES_LOCALIZEDTEXT]);
         break;
     case UA_ATTRIBUTEID_WRITEMASK:
-        retval = UA_Variant_setScalarCopy(&v->value, &node->writeMask, &UA_TYPES[UA_TYPES_UINT32]);
+        setScalarNoDelete(&v->value, &node->writeMask, &UA_TYPES[UA_TYPES_UINT32]);
         break;
     case UA_ATTRIBUTEID_USERWRITEMASK: {
         UA_UInt32 userWriteMask = getUserWriteMask(server, session, node);
@@ -219,23 +222,23 @@ Operation_Read(UA_Server *server, UA_Session *session,
         break;
     case UA_ATTRIBUTEID_SYMMETRIC:
         CHECK_NODECLASS(UA_NODECLASS_REFERENCETYPE);
-        retval = UA_Variant_setScalarCopy(&v->value, &((const UA_ReferenceTypeNode*)node)->symmetric,
-                                          &UA_TYPES[UA_TYPES_BOOLEAN]);
+        setScalarNoDelete(&v->value, &((const UA_ReferenceTypeNode*)node)->symmetric,
+                          &UA_TYPES[UA_TYPES_BOOLEAN]);
         break;
     case UA_ATTRIBUTEID_INVERSENAME:
         CHECK_NODECLASS(UA_NODECLASS_REFERENCETYPE);
-        retval = UA_Variant_setScalarCopy(&v->value, &((const UA_ReferenceTypeNode*)node)->inverseName,
-                                          &UA_TYPES[UA_TYPES_LOCALIZEDTEXT]);
+        setScalarNoDelete(&v->value, &((const UA_ReferenceTypeNode*)node)->inverseName,
+                          &UA_TYPES[UA_TYPES_LOCALIZEDTEXT]);
         break;
     case UA_ATTRIBUTEID_CONTAINSNOLOOPS:
         CHECK_NODECLASS(UA_NODECLASS_VIEW);
-        retval = UA_Variant_setScalarCopy(&v->value, &((const UA_ViewNode*)node)->containsNoLoops,
-                                          &UA_TYPES[UA_TYPES_BOOLEAN]);
+        setScalarNoDelete(&v->value, &((const UA_ViewNode*)node)->containsNoLoops,
+                          &UA_TYPES[UA_TYPES_BOOLEAN]);
         break;
     case UA_ATTRIBUTEID_EVENTNOTIFIER:
         CHECK_NODECLASS(UA_NODECLASS_VIEW | UA_NODECLASS_OBJECT);
-        retval = UA_Variant_setScalarCopy(&v->value, &((const UA_ViewNode*)node)->eventNotifier,
-                                          &UA_TYPES[UA_TYPES_BYTE]);
+        setScalarNoDelete(&v->value, &((const UA_ViewNode*)node)->eventNotifier,
+                          &UA_TYPES[UA_TYPES_BYTE]);
         break;
     case UA_ATTRIBUTEID_VALUE: {
         CHECK_NODECLASS(UA_NODECLASS_VARIABLE | UA_NODECLASS_VARIABLETYPE);
@@ -256,18 +259,18 @@ Operation_Read(UA_Server *server, UA_Session *session,
             }
         }
         retval = readValueAttributeComplete(server, session, (const UA_VariableNode*)node,
-                                            op_timestampsToReturn, &id->indexRange, v);
+                                            timestampsToReturn, &id->indexRange, v);
         break;
     }
     case UA_ATTRIBUTEID_DATATYPE:
         CHECK_NODECLASS(UA_NODECLASS_VARIABLE | UA_NODECLASS_VARIABLETYPE);
-        retval = UA_Variant_setScalarCopy(&v->value, &((const UA_VariableTypeNode*)node)->dataType,
-                                          &UA_TYPES[UA_TYPES_NODEID]);
+        setScalarNoDelete(&v->value, &((const UA_VariableTypeNode*)node)->dataType,
+                          &UA_TYPES[UA_TYPES_NODEID]);
         break;
     case UA_ATTRIBUTEID_VALUERANK:
         CHECK_NODECLASS(UA_NODECLASS_VARIABLE | UA_NODECLASS_VARIABLETYPE);
-        retval = UA_Variant_setScalarCopy(&v->value, &((const UA_VariableTypeNode*)node)->valueRank,
-                                          &UA_TYPES[UA_TYPES_INT32]);
+        setScalarNoDelete(&v->value, &((const UA_VariableTypeNode*)node)->valueRank,
+                          &UA_TYPES[UA_TYPES_INT32]);
         break;
     case UA_ATTRIBUTEID_ARRAYDIMENSIONS:
         CHECK_NODECLASS(UA_NODECLASS_VARIABLE | UA_NODECLASS_VARIABLETYPE);
@@ -275,8 +278,8 @@ Operation_Read(UA_Server *server, UA_Session *session,
         break;
     case UA_ATTRIBUTEID_ACCESSLEVEL:
         CHECK_NODECLASS(UA_NODECLASS_VARIABLE);
-        retval = UA_Variant_setScalarCopy(&v->value, &((const UA_VariableNode*)node)->accessLevel,
-                                          &UA_TYPES[UA_TYPES_BYTE]);
+        setScalarNoDelete(&v->value, &((const UA_VariableNode*)node)->accessLevel,
+                          &UA_TYPES[UA_TYPES_BYTE]);
         break;
     case UA_ATTRIBUTEID_USERACCESSLEVEL: {
         CHECK_NODECLASS(UA_NODECLASS_VARIABLE);
@@ -286,18 +289,18 @@ Operation_Read(UA_Server *server, UA_Session *session,
         break; }
     case UA_ATTRIBUTEID_MINIMUMSAMPLINGINTERVAL:
         CHECK_NODECLASS(UA_NODECLASS_VARIABLE);
-        retval = UA_Variant_setScalarCopy(&v->value, &((const UA_VariableNode*)node)->minimumSamplingInterval,
-                                          &UA_TYPES[UA_TYPES_DOUBLE]);
+        setScalarNoDelete(&v->value, &((const UA_VariableNode*)node)->minimumSamplingInterval,
+                          &UA_TYPES[UA_TYPES_DOUBLE]);
         break;
     case UA_ATTRIBUTEID_HISTORIZING:
         CHECK_NODECLASS(UA_NODECLASS_VARIABLE);
-        retval = UA_Variant_setScalarCopy(&v->value, &((const UA_VariableNode*)node)->historizing,
-                                          &UA_TYPES[UA_TYPES_BOOLEAN]);
+        setScalarNoDelete(&v->value, &((const UA_VariableNode*)node)->historizing,
+                          &UA_TYPES[UA_TYPES_BOOLEAN]);
         break;
     case UA_ATTRIBUTEID_EXECUTABLE:
         CHECK_NODECLASS(UA_NODECLASS_METHOD);
-        retval = UA_Variant_setScalarCopy(&v->value, &((const UA_MethodNode*)node)->executable,
-                                          &UA_TYPES[UA_TYPES_BOOLEAN]);
+        setScalarNoDelete(&v->value, &((const UA_MethodNode*)node)->executable,
+                          &UA_TYPES[UA_TYPES_BOOLEAN]);
         break;
     case UA_ATTRIBUTEID_USEREXECUTABLE: {
         CHECK_NODECLASS(UA_NODECLASS_METHOD);
@@ -309,9 +312,6 @@ Operation_Read(UA_Server *server, UA_Session *session,
         retval = UA_STATUSCODE_BADATTRIBUTEIDINVALID;
     }
 
-    /* Release nodes */
-    UA_Nodestore_release(server, node);
-
     /* Return error code when reading has failed */
     if(retval != UA_STATUSCODE_GOOD) {
         v->hasStatus = true;
@@ -322,16 +322,16 @@ Operation_Read(UA_Server *server, UA_Session *session,
     v->hasValue = true;
 
     /* Create server timestamp */
-    if(op_timestampsToReturn == UA_TIMESTAMPSTORETURN_SERVER ||
-       op_timestampsToReturn == UA_TIMESTAMPSTORETURN_BOTH) {
+    if(timestampsToReturn == UA_TIMESTAMPSTORETURN_SERVER ||
+       timestampsToReturn == UA_TIMESTAMPSTORETURN_BOTH) {
         v->serverTimestamp = UA_DateTime_now();
         v->hasServerTimestamp = true;
     }
 
     /* Handle source time stamp */
     if(id->attributeId == UA_ATTRIBUTEID_VALUE) {
-        if(op_timestampsToReturn == UA_TIMESTAMPSTORETURN_SERVER ||
-           op_timestampsToReturn == UA_TIMESTAMPSTORETURN_NEITHER) {
+        if(timestampsToReturn == UA_TIMESTAMPSTORETURN_SERVER ||
+           timestampsToReturn == UA_TIMESTAMPSTORETURN_NEITHER) {
             v->hasSourceTimestamp = false;
             v->hasSourcePicoseconds = false;
         } else if(!v->hasSourceTimestamp) {
@@ -341,45 +341,114 @@ Operation_Read(UA_Server *server, UA_Session *session,
     }
 }
 
-void Service_Read(UA_Server *server, UA_Session *session,
-                  const UA_ReadRequest *request, UA_ReadResponse *response) {
+static UA_StatusCode
+Operation_Read(UA_Server *server, UA_Session *session, UA_MessageContext *mc,
+               UA_TimestampsToReturn timestampsToReturn, const UA_ReadValueId *id) {
+    UA_DataValue dv;
+    UA_DataValue_init(&dv);
+
+    /* Get the node */
+    const UA_Node *node = UA_Nodestore_get(server, &id->nodeId);
+
+    /* Perform the read operation */
+    if(node) {
+        Read(node, server, session, timestampsToReturn, id, &dv);
+    } else {
+        dv.hasStatus = true;
+        dv.status = UA_STATUSCODE_BADNODEIDUNKNOWN;
+    }
+
+    /* Encode (and send) the results */
+    UA_StatusCode retval = UA_MessageContext_encode(mc, &dv, &UA_TYPES[UA_TYPES_DATAVALUE]);
+
+    /* Free copied data and release the node */
+    UA_Variant_deleteMembers(&dv.value);
+    UA_Nodestore_release(server, node);
+    return retval;
+}
+
+UA_StatusCode Service_Read(UA_Server *server, UA_Session *session, UA_MessageContext *mc,
+                           const UA_ReadRequest *request, UA_ResponseHeader *responseHeader) {
     UA_LOG_DEBUG_SESSION(server->config.logger, session,
                          "Processing ReadRequest");
 
     /* Check if the timestampstoreturn is valid */
-    op_timestampsToReturn = request->timestampsToReturn;
-    if(op_timestampsToReturn > UA_TIMESTAMPSTORETURN_NEITHER) {
-        response->responseHeader.serviceResult = UA_STATUSCODE_BADTIMESTAMPSTORETURNINVALID;
-        return;
-    }
+    if(request->timestampsToReturn > UA_TIMESTAMPSTORETURN_NEITHER)
+        responseHeader->serviceResult = UA_STATUSCODE_BADTIMESTAMPSTORETURNINVALID;
+
+    if(request->nodesToReadSize == 0)
+        responseHeader->serviceResult = UA_STATUSCODE_BADNOTHINGTODO;
 
     /* Check if maxAge is valid */
-    if(request->maxAge < 0) {
-        response->responseHeader.serviceResult = UA_STATUSCODE_BADMAXAGEINVALID;
-        return;
-    }
+    if(request->maxAge < 0)
+        responseHeader->serviceResult = UA_STATUSCODE_BADMAXAGEINVALID;
 
+    /* Check if there are too many operations */
     if(server->config.maxNodesPerRead != 0 &&
-       request->nodesToReadSize > server->config.maxNodesPerRead) {
-        response->responseHeader.serviceResult = UA_STATUSCODE_BADTOOMANYOPERATIONS;
-        return;
+       request->nodesToReadSize > server->config.maxNodesPerRead)
+        responseHeader->serviceResult = UA_STATUSCODE_BADTOOMANYOPERATIONS;
+
+    /* Encode the response header */
+    UA_StatusCode retval =
+        UA_MessageContext_encode(mc, responseHeader, &UA_TYPES[UA_TYPES_RESPONSEHEADER]);
+    if(retval != UA_STATUSCODE_GOOD)
+        return retval;
+
+    /* Process nothing if we return an error code for the entire service */
+    UA_Int32 arraySize = (UA_Int32)request->nodesToReadSize;
+    if(responseHeader->serviceResult != UA_STATUSCODE_GOOD)
+        arraySize = 0;
+
+    /* Process all ReadValueIds */
+    retval = UA_MessageContext_encode(mc, &arraySize, &UA_TYPES[UA_TYPES_INT32]);
+    if(retval != UA_STATUSCODE_GOOD)
+        return retval;
+
+    for(UA_Int32 i = 0; i < arraySize; i++) {
+        retval = Operation_Read(server, session, mc, request->timestampsToReturn,
+                                &request->nodesToRead[i]);
+        if(retval != UA_STATUSCODE_GOOD)
+            return retval;
     }
 
-    response->responseHeader.serviceResult = 
-        UA_Server_processServiceOperations(server, session,
-                  (UA_ServiceOperation)Operation_Read,
-                  &request->nodesToReadSize, &UA_TYPES[UA_TYPES_READVALUEID],
-                  &response->resultsSize, &UA_TYPES[UA_TYPES_DATAVALUE]);
+    /* Don't return any DiagnosticInfo */
+    arraySize = -1;
+    return UA_MessageContext_encode(mc, &arraySize, &UA_TYPES[UA_TYPES_INT32]);
 }
 
 UA_DataValue
 UA_Server_readWithSession(UA_Server *server, UA_Session *session,
                           const UA_ReadValueId *item,
-                          UA_TimestampsToReturn timestamps) {
+                          UA_TimestampsToReturn timestampsToReturn) {
     UA_DataValue dv;
     UA_DataValue_init(&dv);
-    op_timestampsToReturn = timestamps;
-    Operation_Read(server, session, item, &dv);
+
+    /* Get the node */
+    const UA_Node *node = UA_Nodestore_get(server, &item->nodeId);
+    if(!node) {
+        dv.hasStatus = true;
+        dv.status = UA_STATUSCODE_BADNODEIDUNKNOWN;
+        return dv;
+    }
+
+    /* Perform the read operation */
+    Read(node, server, session, timestampsToReturn, item, &dv);
+
+    /* Do we have to copy the result before releasing the node? */
+    if(dv.hasValue && dv.value.storageType == UA_VARIANT_DATA_NODELETE) {
+        UA_DataValue dv2;
+        UA_StatusCode retval = UA_DataValue_copy(&dv, &dv2);
+        if(retval == UA_STATUSCODE_GOOD) {
+            dv = dv2;
+        } else {
+            UA_DataValue_init(&dv);
+            dv.hasStatus = true;
+            dv.status = retval;
+        }
+    }
+
+    /* Release the node and return */
+    UA_Nodestore_release(server, node);
     return dv;
 }
 
@@ -413,28 +482,14 @@ __UA_Server_read(UA_Server *server, const UA_NodeId *nodeId,
         return retval;
     }
 
-    /* Prepare the result */
     if(attributeId == UA_ATTRIBUTEID_VALUE ||
        attributeId == UA_ATTRIBUTEID_ARRAYDIMENSIONS) {
         /* Return the entire variant */
-        if(dv.value.storageType == UA_VARIANT_DATA_NODELETE) {
-            retval = UA_Variant_copy(&dv.value,(UA_Variant *) v);
-        } else {
-            /* storageType is UA_VARIANT_DATA. Copy the entire variant
-             * (including pointers and all) */
-            memcpy(v, &dv.value, sizeof(UA_Variant));
-        }
+        memcpy(v, &dv.value, sizeof(UA_Variant));
     } else {
         /* Return the variant content only */
-        if(dv.value.storageType == UA_VARIANT_DATA_NODELETE) {
-            retval = UA_copy(dv.value.data, v, dv.value.type);
-        } else {
-            /* storageType is UA_VARIANT_DATA. Copy the content of the type
-             * (including pointers and all) */
-            memcpy(v, dv.value.data, dv.value.type->memSize);
-            /* Delete the "carrier" in the variant */
-            UA_free(dv.value.data);
-        }
+        memcpy(v, dv.value.data, dv.value.type->memSize);
+        UA_free(dv.value.data);
     }
     return retval;
 }

+ 66 - 18
tests/server/check_server_readspeed.c

@@ -6,16 +6,47 @@
 
 #include <time.h>
 #include <stdio.h>
+#include <check.h>
 
 #include "ua_server.h"
 #include "ua_config_default.h"
 #include "server/ua_services.h"
 #include "ua_types_encoding_binary.h"
 
-int main(int argc, char** argv) {
-    UA_ServerConfig *config = UA_ServerConfig_new_default();
-    UA_Server *server = UA_Server_new(config);
+#include "testing_networklayers.h"
+#include "testing_policy.h"
 
+static UA_SecureChannel testChannel;
+static UA_SecurityPolicy dummyPolicy;
+static UA_Connection testingConnection;
+static funcs_called funcsCalled;
+static key_sizes keySizes;
+
+static UA_ServerConfig *config;
+static UA_Server *server;
+
+static void setup(void) {
+    config = UA_ServerConfig_new_default();
+    server = UA_Server_new(config);
+
+    TestingPolicy(&dummyPolicy, UA_BYTESTRING_NULL, &funcsCalled, &keySizes);
+    UA_SecureChannel_init(&testChannel, &dummyPolicy, &UA_BYTESTRING_NULL);
+
+    testingConnection = createDummyConnection(65535, NULL);
+    UA_Connection_attachSecureChannel(&testingConnection, &testChannel);
+    testChannel.connection = &testingConnection;
+}
+
+static void teardown(void) {
+    UA_SecureChannel_deleteMembersCleanup(&testChannel);
+    dummyPolicy.deleteMembers(&dummyPolicy);
+    testingConnection.close(&testingConnection);
+
+    UA_Server_delete(server);
+    UA_ServerConfig_delete(config);
+}
+
+START_TEST(readSpeed) {
     /* add a variable node to the address space */
     UA_VariableAttributes attr = UA_VariableAttributes_default;
     UA_Int32 myInteger = 42;
@@ -52,11 +83,11 @@ int main(int argc, char** argv) {
     retval |= UA_encodeBinary(&request, &UA_TYPES[UA_TYPES_READREQUEST], &pos, &end, NULL, NULL);
     UA_assert(retval == UA_STATUSCODE_GOOD);
 
-    UA_Byte *resp_pos = response_msg.data;
-    const UA_Byte *resp_end = &response_msg.data[response_msg.length];
-
     UA_ReadRequest rq;
-    UA_ReadResponse rr;
+    UA_MessageContext mc;
+
+    UA_ResponseHeader rh;
+    UA_ResponseHeader_init(&rh);
 
     clock_t begin, finish;
     begin = clock();
@@ -64,27 +95,44 @@ int main(int argc, char** argv) {
     for(int i = 0; i < 1000000; i++) {
         size_t offset = 0;
         retval |= UA_decodeBinary(&request_msg, &offset, &rq, &UA_TYPES[UA_TYPES_READREQUEST], 0, NULL);
-        UA_assert(retval == UA_STATUSCODE_GOOD);
-
-        UA_ReadResponse_init(&rr);
-        Service_Read(server, &adminSession, &rq, &rr);
 
-        resp_pos = response_msg.data;
-        retval |= UA_encodeBinary(&rr, &UA_TYPES[UA_TYPES_READRESPONSE], &resp_pos, &resp_end, NULL, NULL);
-        UA_assert(retval == UA_STATUSCODE_GOOD);
+        UA_MessageContext_begin(&mc, &testChannel, 0, UA_MESSAGETYPE_MSG);
+        retval |= Service_Read(server, &adminSession, &mc, &rq, &rh);
+        UA_MessageContext_finish(&mc);
 
         UA_ReadRequest_deleteMembers(&rq);
-        UA_ReadResponse_deleteMembers(&rr);
     }
 
     finish = clock();
+
+    UA_assert(retval == UA_STATUSCODE_GOOD);
     double time_spent = (double)(finish - begin) / CLOCKS_PER_SEC;
     printf("duration was %f s\n", time_spent);
     printf("retval is %s\n", UA_StatusCode_name(retval));
 
     UA_ByteString_deleteMembers(&request_msg);
     UA_ByteString_deleteMembers(&response_msg);
-    UA_Server_delete(server);
-    UA_ServerConfig_delete(config);
-    return (int)retval;
+}
+END_TEST
+
+static Suite * service_speed_suite (void) {
+    Suite *s = suite_create ("Service Speed");
+
+    TCase* tc_read = tcase_create ("Read");
+    tcase_add_checked_fixture(tc_read, setup, teardown);
+    tcase_add_test (tc_read, readSpeed);
+    suite_add_tcase (s, tc_read);
+
+    return s;
+}
+
+int main (void) {
+    int number_failed = 0;
+    Suite *s = service_speed_suite();
+    SRunner *sr = srunner_create(s);
+    srunner_set_fork_status(sr,CK_NOFORK);
+    srunner_run_all(sr, CK_NORMAL);
+    number_failed += srunner_ntests_failed (sr);
+    srunner_free(sr);
+    return (number_failed == 0) ? EXIT_SUCCESS : EXIT_FAILURE;
 }