Преглед на файлове

remove public ua_job.h; call message processing from the network layer

Julius Pfrommer преди 7 години
родител
ревизия
1969bad066

+ 0 - 1
CMakeLists.txt

@@ -235,7 +235,6 @@ set(exported_headers ${PROJECT_BINARY_DIR}/src_generated/ua_config.h
                      ${PROJECT_BINARY_DIR}/src_generated/ua_types_generated.h
                      ${PROJECT_BINARY_DIR}/src_generated/ua_types_generated_handling.h
                      ${PROJECT_SOURCE_DIR}/include/ua_connection.h
-                     ${PROJECT_SOURCE_DIR}/include/ua_job.h
                      ${PROJECT_SOURCE_DIR}/include/ua_log.h
                      ${PROJECT_SOURCE_DIR}/include/ua_server.h
                      ${PROJECT_SOURCE_DIR}/include/ua_client.h

+ 2 - 2
examples/discovery/server_multicast.c

@@ -149,8 +149,8 @@ int main(int argc, char **argv) {
     UA_LOG_INFO(logger, UA_LOGCATEGORY_SERVER, "LDS-ME server found on %s", discovery_url);
 
     // periodic server register after 10 Minutes, delay first register for 500ms
-    retval = UA_Server_addPeriodicServerRegisterJob(server, discovery_url,
-                                                    10 * 60 * 1000, 500, NULL);
+    retval = UA_Server_addPeriodicServerRegisterCallback(server, discovery_url,
+                                                         10 * 60 * 1000, 500, NULL);
     if (retval != UA_STATUSCODE_GOOD) {
         UA_LOG_ERROR(logger, UA_LOGCATEGORY_SERVER,
                      "Could not create periodic job for server register. StatusCode %s",

+ 14 - 5
examples/discovery/server_register.c

@@ -82,10 +82,15 @@ int main(int argc, char **argv) {
 
 
     // periodic server register after 10 Minutes, delay first register for 500ms
-    UA_StatusCode retval = UA_Server_addPeriodicServerRegisterJob(server, DISCOVERY_SERVER_ENDPOINT, 10 * 60 * 1000, 500, NULL);
-    //UA_StatusCode retval = UA_Server_addPeriodicServerRegisterJob(server, "opc.tcp://localhost:4840", 10*60*1000, 500, NULL);
+    UA_StatusCode retval =
+        UA_Server_addPeriodicServerRegisterCallback(server, DISCOVERY_SERVER_ENDPOINT,
+                                                    10 * 60 * 1000, 500, NULL);
+    // UA_StatusCode retval = UA_Server_addPeriodicServerRegisterJob(server,
+    // "opc.tcp://localhost:4840", 10*60*1000, 500, NULL);
     if (retval != UA_STATUSCODE_GOOD) {
-        UA_LOG_ERROR(logger, UA_LOGCATEGORY_SERVER, "Could not create periodic job for server register. StatusCode %s", UA_StatusCode_name(retval));
+        UA_LOG_ERROR(logger, UA_LOGCATEGORY_SERVER,
+                     "Could not create periodic job for server register. StatusCode %s",
+                     UA_StatusCode_name(retval));
         UA_String_deleteMembers(&config.applicationDescription.applicationUri);
         UA_Server_delete(server);
         nl.deleteMembers(&nl);
@@ -94,7 +99,9 @@ int main(int argc, char **argv) {
 
     retval = UA_Server_run(server, &running);
     if (retval != UA_STATUSCODE_GOOD) {
-        UA_LOG_ERROR(logger, UA_LOGCATEGORY_SERVER, "Could not start the server. StatusCode %s", UA_StatusCode_name(retval));
+        UA_LOG_ERROR(logger, UA_LOGCATEGORY_SERVER,
+                     "Could not start the server. StatusCode %s",
+                     UA_StatusCode_name(retval));
         UA_String_deleteMembers(&config.applicationDescription.applicationUri);
         UA_Server_delete(server);
         nl.deleteMembers(&nl);
@@ -105,7 +112,9 @@ int main(int argc, char **argv) {
     retval = UA_Server_unregister_discovery(server, DISCOVERY_SERVER_ENDPOINT);
     //retval = UA_Server_unregister_discovery(server, "opc.tcp://localhost:4840" );
     if (retval != UA_STATUSCODE_GOOD) {
-        UA_LOG_ERROR(logger, UA_LOGCATEGORY_SERVER, "Could not unregister server from discovery server. StatusCode %s", UA_StatusCode_name(retval));
+        UA_LOG_ERROR(logger, UA_LOGCATEGORY_SERVER,
+                     "Could not unregister server from discovery server. StatusCode %s",
+                     UA_StatusCode_name(retval));
         UA_String_deleteMembers(&config.applicationDescription.applicationUri);
         UA_Server_delete(server);
         nl.deleteMembers(&nl);

+ 2 - 6
examples/server_repeated_job.c

@@ -26,12 +26,8 @@ int main(void) {
     config.networkLayersSize = 1;
     UA_Server *server = UA_Server_new(config);
 
-    /* add a repeated job to the server */
-    UA_Job job;
-    job.type = UA_JOBTYPE_METHODCALL;
-    job.job.methodCall.data = NULL;
-    job.job.methodCall.method = testCallback;
-    UA_Server_addRepeatedJob(server, job, 2000, NULL); /* call every 2 sec */
+    /* Add a repeated callback to the server */
+    UA_Server_addRepeatedCallback(server, testCallback, NULL, 2000, NULL); /* call every 2 sec */
 
     UA_Server_run(server, &running);
     UA_Server_delete(server);

+ 9 - 2
include/ua_connection.h

@@ -99,11 +99,18 @@ struct UA_Connection {
     /* Release the buffer of a received message */
     void (*releaseRecvBuffer)(UA_Connection *connection, UA_ByteString *buf);
 
-    /* Close the connection */
+    /* Close the connection. The network layer closes the socket, removes
+     * internal pointers to the connection and calls
+     * UA_Server_removeConnection. */
     void (*close)(UA_Connection *connection);
+
+    /* To be called only from within the server (and not the network layer).
+     * Frees up the connection's memory. */
+    void (*free)(UA_Connection *connection);
 };
 
-void UA_EXPORT UA_Connection_deleteMembers(UA_Connection *connection);
+void UA_EXPORT
+UA_Connection_deleteMembers(UA_Connection *connection);
 
 /**
  * EndpointURL Helper

+ 0 - 48
include/ua_job.h

@@ -1,48 +0,0 @@
-/* This Source Code Form is subject to the terms of the Mozilla Public
- * License, v. 2.0. If a copy of the MPL was not distributed with this
- * file, You can obtain one at http://mozilla.org/MPL/2.0/. */
-
-#ifndef UA_JOB_H_
-#define UA_JOB_H_
-
-#include "ua_connection.h"
-
-#ifdef __cplusplus
-extern "C" {
-#endif
-
-struct UA_Server;
-typedef struct UA_Server UA_Server;
-
-typedef void (*UA_ServerCallback)(UA_Server *server, void *data);
-
-typedef enum {
-    UA_JOBTYPE_NOTHING,
-    UA_JOBTYPE_DETACHCONNECTION, /* Detach the connection from the secure channel (but don't delete it) */
-    UA_JOBTYPE_BINARYMESSAGE_NETWORKLAYER, /* The binary message is memory managed by the networklayer */
-    UA_JOBTYPE_BINARYMESSAGE_ALLOCATED, /* The binary message was relocated away from the networklayer */
-    UA_JOBTYPE_METHODCALL, /* Call the method as soon as possible */
-    UA_JOBTYPE_METHODCALL_DELAYED, /* Call the method as soon as all previous jobs have finished */
-} UA_JobType;
-
-/* Jobs describe work that is executed once or repeatedly in the server */
-typedef struct {
-    UA_JobType type;
-    union {
-        UA_Connection *closeConnection;
-        struct {
-            UA_Connection *connection;
-            UA_ByteString message;
-        } binaryMessage;
-        struct {
-            void *data;
-            UA_ServerCallback method;
-        } methodCall;
-    } job;
-} UA_Job;
-
-#ifdef __cplusplus
-} // extern "C"
-#endif
-
-#endif /* UA_JOB_H_ */

+ 75 - 47
include/ua_server.h

@@ -15,9 +15,14 @@ extern "C" {
 #include "ua_types_generated_handling.h"
 #include "ua_nodeids.h"
 #include "ua_log.h"
-#include "ua_job.h"
 #include "ua_connection.h"
 
+struct UA_Server;
+typedef struct UA_Server UA_Server;
+
+struct UA_ServerNetworkLayer;
+typedef struct UA_ServerNetworkLayer UA_ServerNetworkLayer;
+
 /**
  * .. _server:
  *
@@ -29,9 +34,6 @@ extern "C" {
  * Interface to the binary network layers. The functions in the network layer
  * are never called in parallel but only sequentially from the server's main
  * loop. So the network layer does not need to be thread-safe. */
-struct UA_ServerNetworkLayer;
-typedef struct UA_ServerNetworkLayer UA_ServerNetworkLayer;
-
 struct UA_ServerNetworkLayer {
     void *handle; // pointer to internal data
     UA_String discoveryUrl;
@@ -43,27 +45,26 @@ struct UA_ServerNetworkLayer {
      * @return Returns UA_STATUSCODE_GOOD or an error code. */
     UA_StatusCode (*start)(UA_ServerNetworkLayer *nl, UA_Logger logger);
 
-    /* Gets called from the main server loop and returns the jobs (accumulated
-     * messages and close events) for dispatch.
+    /* Gets called from the main server loop and dispatches the received
+     * messages in the server.
      *
      * @param nl The network layer
-     * @param jobs When the returned integer is >0, *jobs points to an array of
-     *        UA_Job of the returned size.
+     * @param server The server that processes the incoming packets and for closing
+     *               connections before deleting them.
      * @param timeout The timeout during which an event must arrive in
-     *        microseconds
-     * @return The size of the jobs array. If the result is negative,
-     *         an error has occurred. */
-    size_t (*getJobs)(UA_ServerNetworkLayer *nl, UA_Job **jobs, UA_UInt16 timeout);
+     *                microseconds
+     * @return A statuscode for the status of the network layer. */
+    UA_StatusCode (*listen)(UA_ServerNetworkLayer *nl, UA_Server *server,
+                            UA_UInt16 timeout);
 
-    /* Closes the network connection and returns all the jobs that need to be
-     * finished before the network layer can be safely deleted.
+    /* Closes the network socket and all open connections before the network
+     * layer can be safely deleted.
      *
      * @param nl The network layer
-     * @param jobs When the returned integer is >0, jobs points to an array of
-     *        UA_Job of the returned size.
-     * @return The size of the jobs array. If the result is negative,
-     *         an error has occurred. */
-    size_t (*stop)(UA_ServerNetworkLayer *nl, UA_Job **jobs);
+     * @param server The server that processes the incoming packets and for closing
+     *               connections before deleting them.
+     * @return A statuscode for the status of the closing operation. */
+    void (*stop)(UA_ServerNetworkLayer *nl, UA_Server *server);
 
     /** Deletes the network content. Call only after stopping. */
     void (*deleteMembers)(UA_ServerNetworkLayer *nl);
@@ -207,10 +208,6 @@ typedef struct {
 #endif
 } UA_ServerConfig;
 
-/* Add a new namespace to the server. Returns the index of the new namespace */
-UA_UInt16 UA_EXPORT
-UA_Server_addNamespace(UA_Server *server, const char* name);
-
 /**
  * .. _server-lifecycle:
  *
@@ -220,8 +217,7 @@ UA_Server UA_EXPORT * UA_Server_new(const UA_ServerConfig config);
 void UA_EXPORT UA_Server_delete(UA_Server *server);
 
 /* Runs the main loop of the server. In each iteration, this calls into the
- * networklayers to see if jobs have arrived and checks if repeated jobs need to
- * be triggered.
+ * networklayers to see if messages have arrived.
  *
  * @param server The server object.
  * @param running The loop is run as long as *running is true.
@@ -241,7 +237,7 @@ UA_StatusCode UA_EXPORT UA_Server_run_startup(UA_Server *server);
  *        Otherwise, the timouts for the networklayers are set to zero.
  *        The default max wait time is 50millisec.
  * @return Returns how long we can wait until the next scheduled
- *         job (in millisec) */
+ *         callback (in ms) */
 UA_UInt16 UA_EXPORT
 UA_Server_run_iterate(UA_Server *server, UA_Boolean waitInternal);
 
@@ -250,31 +246,57 @@ UA_Server_run_iterate(UA_Server *server, UA_Boolean waitInternal);
 UA_StatusCode UA_EXPORT UA_Server_run_shutdown(UA_Server *server);
 
 /**
- * Repeated jobs
- * ------------- */
-/* Add a job for cyclic repetition to the server.
+ * Repeated Callbacks
+ * ------------------ */
+typedef void (*UA_ServerCallback)(UA_Server *server, void *data);
+
+/* Add a callback for cyclic repetition to the server.
  *
  * @param server The server object.
- * @param job The job that shall be added.
- * @param interval The job shall be repeatedly executed with the given interval
+ * @param callback The callback that shall be added.
+ * @param interval The callback shall be repeatedly executed with the given interval
  *        (in ms). The interval must be larger than 5ms. The first execution
  *        occurs at now() + interval at the latest.
- * @param jobId Set to the guid of the repeated job. This can be used to cancel
- *        the job later on. If the pointer is null, the guid is not set.
+ * @param callbackId Set to the identifier of the repeated callback . This can be used to cancel
+ *        the callback later on. If the pointer is null, the identifier is not set.
  * @return Upon success, UA_STATUSCODE_GOOD is returned.
  *         An error code otherwise. */
 UA_StatusCode UA_EXPORT
-UA_Server_addRepeatedJob(UA_Server *server, UA_Job job,
-                         UA_UInt32 interval, UA_Guid *jobId);
+UA_Server_addRepeatedCallback(UA_Server *server, UA_ServerCallback callback,
+                              void *data, UA_UInt32 interval, UA_UInt64 *callbackId);
 
-/* Remove repeated job.
+UA_StatusCode UA_EXPORT
+UA_Server_changeRepeatedCallbackInterval(UA_Server *server, UA_UInt64 callbackId,
+                                         UA_UInt32 interval);
+
+/* Remove a repeated callback.
  *
  * @param server The server object.
- * @param jobId The id of the job that shall be removed.
+ * @param callbackId The id of the callback that shall be removed.
  * @return Upon sucess, UA_STATUSCODE_GOOD is returned.
  *         An error code otherwise. */
 UA_StatusCode UA_EXPORT
-UA_Server_removeRepeatedJob(UA_Server *server, UA_Guid jobId);
+UA_Server_removeRepeatedCallback(UA_Server *server, UA_UInt64 callbackId);
+
+/*
+ * Networking
+ * ----------
+ *
+ * Connections are created in the network layer. The server gets called to
+ * remove the connection.
+ * - Unlink the securechannel, and so on.
+ * - Free the connection when no (concurrent) server thread uses it any more. */
+void UA_EXPORT
+UA_Server_removeConnection(UA_Server *server,
+                           UA_Connection *connection);
+
+/* Process a binary message (packet). The message can contain partial chunks.
+ * (TCP is a streaming protocol and packets may be split/merge during
+ * transport.) The message is freed with connection->releaseRecvBuffer. */
+void UA_EXPORT
+UA_Server_processBinaryMessage(UA_Server *server,
+                               UA_Connection *connection,
+                               UA_ByteString *message);
 
 /**
  * Reading and Writing Node Attributes
@@ -635,7 +657,7 @@ UA_Server_register_discovery(UA_Server *server, const char* discoveryServerUrl,
 UA_StatusCode UA_EXPORT
 UA_Server_unregister_discovery(UA_Server *server, const char* discoveryServerUrl);
 
- /* Adds a periodic job to register the server with the LDS (local discovery server)
+ /* Adds a periodic callback to register the server with the LDS (local discovery server)
   * periodically. The interval between each register call is given as second parameter.
   * It should be 10 minutes by default (= 10*60*1000).
   *
@@ -643,26 +665,26 @@ UA_Server_unregister_discovery(UA_Server *server, const char* discoveryServerUrl
   * If it is 0, the first register call will be after intervalMs milliseconds,
   * otherwise the server's first register will be after delayFirstRegisterMs.
   *
-  * When you manually unregister the server, you also need to cancel the periodic job,
-  * otherwise it will be automatically be registered again.
+  * When you manually unregister the server, you also need to cancel the
+  * periodic callback, otherwise it will be automatically be registered again.
   *
   * @param server
   * @param discoveryServerUrl if set to NULL, the default value
   *        'opc.tcp://localhost:4840' will be used
   * @param intervalMs
   * @param delayFirstRegisterMs
-  * @param periodicJobId */
+  * @param periodicCallbackId */
 UA_StatusCode UA_EXPORT
-UA_Server_addPeriodicServerRegisterJob(UA_Server *server, const char* discoveryServerUrl,
-                                       const UA_UInt32 intervalMs,
-                                       const UA_UInt32 delayFirstRegisterMs,
-                                       UA_Guid* periodicJobId);
+UA_Server_addPeriodicServerRegisterCallback(UA_Server *server, const char* discoveryServerUrl,
+                                            UA_UInt32 intervalMs,
+                                            UA_UInt32 delayFirstRegisterMs,
+                                            UA_UInt64 *periodicCallbackId);
 
 /* Callback for RegisterServer. Data is passed from the register call */
 typedef void (*UA_Server_registerServerCallback)(const UA_RegisteredServer *registeredServer,
                                                  void* data);
 
-/* Set the callback which is called if another server registeres or unregisteres
+/* Set the callback which is called if another server registeres or unregisters
  * with this instance. If called multiple times, previous data will be
  * overwritten.
  *
@@ -1065,6 +1087,12 @@ UA_Server_deleteReference(UA_Server *server, const UA_NodeId sourceNodeId,
                           const UA_ExpandedNodeId targetNodeId,
                           UA_Boolean deleteBidirectional);
 
+/**
+ * Utility Functions
+ * ----------------- */
+/* Add a new namespace to the server. Returns the index of the new namespace */
+UA_UInt16 UA_EXPORT UA_Server_addNamespace(UA_Server *server, const char* name);
+
 #ifdef __cplusplus
 }
 #endif

+ 85 - 140
plugins/ua_network_tcp.c

@@ -12,6 +12,7 @@
 #endif
 
 #include "ua_network_tcp.h"
+#include "queue.h"
 
 #include <stdlib.h> // malloc, free
 #include <stdio.h> // snprintf
@@ -212,11 +213,6 @@ static UA_StatusCode socket_set_nonblocking(SOCKET sockfd) {
     return UA_STATUSCODE_GOOD;
 }
 
-static void FreeConnectionCallback(UA_Server *server, void *ptr) {
-    UA_Connection_deleteMembers((UA_Connection*)ptr);
-    free(ptr);
- }
-
 /***************************/
 /* Server NetworkLayer TCP */
 /***************************/
@@ -256,10 +252,12 @@ static void FreeConnectionCallback(UA_Server *server, void *ptr) {
 
 #define MAXBACKLOG 100
 
-typedef struct {
-  UA_Connection *connection;
-  UA_Int32 sockfd;
-} ConnectionMapping;
+/* UA_Connection is the first element. We can exchange pointers to the first
+ * element and free the entire structure. */
+typedef struct ConnectionEntry {
+    UA_Connection connection;
+    LIST_ENTRY(ConnectionEntry) pointers;
+} ConnectionEntry;
 
 typedef struct {
     UA_ConnectionConfig conf;
@@ -268,71 +266,51 @@ typedef struct {
 
     /* open sockets and connections */
     UA_Int32 serversockfd;
-    size_t mappingsSize;
-    ConnectionMapping *mappings;
+    LIST_HEAD(, ConnectionEntry) connections;
 } ServerNetworkLayerTCP;
 
 static UA_StatusCode
-ServerNetworkLayerGetSendBuffer(UA_Connection *connection, size_t length, UA_ByteString *buf) {
+ServerNetworkLayerGetSendBuffer(UA_Connection *connection,
+                                size_t length, UA_ByteString *buf) {
     if(length > connection->remoteConf.recvBufferSize)
         return UA_STATUSCODE_BADCOMMUNICATIONERROR;
     return UA_ByteString_allocBuffer(buf, length);
 }
 
 static void
-ServerNetworkLayerReleaseSendBuffer(UA_Connection *connection, UA_ByteString *buf) {
+ServerNetworkLayerReleaseSendBuffer(UA_Connection *connection,
+                                    UA_ByteString *buf) {
     UA_ByteString_deleteMembers(buf);
 }
 
 static void
-ServerNetworkLayerReleaseRecvBuffer(UA_Connection *connection, UA_ByteString *buf) {
+ServerNetworkLayerReleaseRecvBuffer(UA_Connection *connection,
+                                    UA_ByteString *buf) {
     UA_ByteString_deleteMembers(buf);
 }
 
-/* after every select, we need to reset the sockets we want to listen on */
-static UA_Int32
-setFDSet(ServerNetworkLayerTCP *layer, fd_set *fdset) {
-    FD_ZERO(fdset);
-    UA_fd_set(layer->serversockfd, fdset);
-    UA_Int32 highestfd = layer->serversockfd;
-    for(size_t i = 0; i < layer->mappingsSize; ++i) {
-        UA_fd_set(layer->mappings[i].sockfd, fdset);
-        if(layer->mappings[i].sockfd > highestfd)
-            highestfd = layer->mappings[i].sockfd;
-    }
-    return highestfd;
-}
+static void
+ServerNetworkLayerTCP_freeConnection(UA_Connection *connection) {
+    UA_Connection_deleteMembers(connection);
+    free(connection);
+ }
 
-/* callback triggered from the server */
+/* Callback triggered from the server (any thread). Only "shutdown" here. This
+ * triggers the select, where the connection is closed and freed in the server's
+ * mainloop. */
 static void
 ServerNetworkLayerTCP_closeConnection(UA_Connection *connection) {
-#ifdef UA_ENABLE_MULTITHREADING
-    if(uatomic_xchg(&connection->state, UA_CONNECTION_CLOSED) == UA_CONNECTION_CLOSED)
-        return;
-#else
-    if(connection->state == UA_CONNECTION_CLOSED)
-        return;
     connection->state = UA_CONNECTION_CLOSED;
-#endif
-#if UA_LOGLEVEL <= 300
-   //cppcheck-suppress unreadVariable
-    ServerNetworkLayerTCP *layer = (ServerNetworkLayerTCP *)connection->handle;
-    UA_LOG_INFO(layer->logger, UA_LOGCATEGORY_NETWORK,
+    UA_LOG_INFO(((ServerNetworkLayerTCP*)connection->handle)->logger,
+                UA_LOGCATEGORY_NETWORK,
                 "Connection %i | Force closing the connection",
                 connection->sockfd);
-#endif
-    /* only "shutdown" here. this triggers the select, where the socket is
-       "closed" in the mainloop */
     shutdown(connection->sockfd, 2);
 }
 
-/* call only from the single networking thread */
 static UA_StatusCode
 ServerNetworkLayerTCP_add(ServerNetworkLayerTCP *layer, UA_Int32 newsockfd) {
-    UA_Connection *c = (UA_Connection *)malloc(sizeof(UA_Connection));
-    if(!c)
-        return UA_STATUSCODE_BADINTERNALERROR;
-
+    /* Look up the peer name for logging */
     struct sockaddr_in addr;
     socklen_t addrlen = sizeof(struct sockaddr_in);
     int res = getpeername(newsockfd, (struct sockaddr*)&addr, &addrlen);
@@ -345,6 +323,12 @@ ServerNetworkLayerTCP_add(ServerNetworkLayerTCP *layer, UA_Int32 newsockfd) {
                        "Connection %i | New connection over TCP, "
                        "getpeername failed with errno %i", newsockfd, errno);
     }
+
+    /* Allocate and initialize the connection */
+    ConnectionEntry *e = (ConnectionEntry*)malloc(sizeof(ConnectionEntry));
+    if(!e)
+        return UA_STATUSCODE_BADOUTOFMEMORY;
+    UA_Connection *c = &e->connection;
     memset(c, 0, sizeof(UA_Connection));
     c->sockfd = newsockfd;
     c->handle = layer;
@@ -352,22 +336,14 @@ ServerNetworkLayerTCP_add(ServerNetworkLayerTCP *layer, UA_Int32 newsockfd) {
     c->remoteConf = layer->conf;
     c->send = socket_write;
     c->close = ServerNetworkLayerTCP_closeConnection;
+    c->free = ServerNetworkLayerTCP_freeConnection;
     c->getSendBuffer = ServerNetworkLayerGetSendBuffer;
     c->releaseSendBuffer = ServerNetworkLayerReleaseSendBuffer;
     c->releaseRecvBuffer = ServerNetworkLayerReleaseRecvBuffer;
     c->state = UA_CONNECTION_OPENING;
-    ConnectionMapping *nm;
-    nm  = (ConnectionMapping *)realloc(layer->mappings, sizeof(ConnectionMapping)*(layer->mappingsSize+1));
-    if(!nm) {
-        UA_LOG_ERROR(layer->logger, UA_LOGCATEGORY_NETWORK,
-                     "No memory for a new Connection");
-        free(c);
-        return UA_STATUSCODE_BADINTERNALERROR;
-    }
-    layer->mappings = nm;
-    layer->mappings[layer->mappingsSize].connection = c;
-    layer->mappings[layer->mappingsSize].sockfd = newsockfd;
-    ++layer->mappingsSize;
+
+    /* Add to the linked list */
+    LIST_INSERT_HEAD(&layer->connections, e, pointers);
     return UA_STATUSCODE_GOOD;
 }
 
@@ -444,39 +420,42 @@ ServerNetworkLayerTCP_start(UA_ServerNetworkLayer *nl, UA_Logger logger) {
     return UA_STATUSCODE_GOOD;
 }
 
-static size_t
-removeClosedConnections(ServerNetworkLayerTCP *layer, UA_Job *js) {
-    size_t c = 0;
-    for(size_t i = 0; i < layer->mappingsSize; ++i) {
-        if(layer->mappings[i].connection &&
-           layer->mappings[i].connection->state != UA_CONNECTION_CLOSED)
+/* Remove closed connections before going into "listen". Returns whether a
+ * connection was removed. */
+static void
+removeClosedConnections(ServerNetworkLayerTCP *layer, UA_Server *server) {
+    ConnectionEntry *e, *e_tmp;
+    LIST_FOREACH_SAFE(e, &layer->connections, pointers, e_tmp) {
+        if(e->connection.state != UA_CONNECTION_CLOSED)
             continue;
-        /* the socket was closed from remote */
-        UA_Connection *conn = layer->mappings[i].connection;
-        js[c].type = UA_JOBTYPE_DETACHCONNECTION;
-        js[c].job.closeConnection = conn;
-        layer->mappings[i] = layer->mappings[layer->mappingsSize-1];
-        --layer->mappingsSize;
-        ++c;
-        js[c].type = UA_JOBTYPE_METHODCALL_DELAYED;
-        js[c].job.methodCall.method = FreeConnectionCallback;
-        js[c].job.methodCall.data = conn;
-        ++c;
+        LIST_REMOVE(e, pointers);
+        UA_Server_removeConnection(server, &e->connection);
+    }
+}
+
+/* After every select, we need to reset the sockets we want to listen on */
+static UA_Int32
+setFDSet(ServerNetworkLayerTCP *layer, fd_set *fdset) {
+    FD_ZERO(fdset);
+    UA_fd_set(layer->serversockfd, fdset);
+    UA_Int32 highestfd = layer->serversockfd;
+    ConnectionEntry *e;
+    LIST_FOREACH(e, &layer->connections, pointers) {
+        UA_fd_set(e->connection.sockfd, fdset);
+        if(e->connection.sockfd > highestfd)
+            highestfd = e->connection.sockfd;
     }
-    return c;
+    return highestfd;
 }
 
-static size_t
-ServerNetworkLayerTCP_getJobs(UA_ServerNetworkLayer *nl, UA_Job **jobs,
-                              UA_UInt16 timeout) {
+static UA_StatusCode
+ServerNetworkLayerTCP_listen(UA_ServerNetworkLayer *nl, UA_Server *server,
+                             UA_UInt16 timeout) {
     /* Every open socket can generate two jobs */
     ServerNetworkLayerTCP *layer = (ServerNetworkLayerTCP *)nl->handle;
-    UA_Job *js = (UA_Job *)malloc(sizeof(UA_Job) * (size_t)((layer->mappingsSize * 2)));
-    if(!js)
-        return UA_STATUSCODE_BADOUTOFMEMORY;
 
     /* Remove closed sockets */
-    size_t totalJobs = removeClosedConnections(layer, js);
+    removeClosedConnections(layer, server);
 
     /* Listen on open sockets (including the server) */
     fd_set fdset, errset;
@@ -484,11 +463,6 @@ ServerNetworkLayerTCP_getJobs(UA_ServerNetworkLayer *nl, UA_Job **jobs,
     setFDSet(layer, &errset);
     struct timeval tmptv = {0, timeout * 1000};
     UA_Int32 resultsize = select(highestfd+1, &fdset, NULL, &errset, &tmptv);
-    if(totalJobs == 0 && resultsize <= 0) {
-        free(js);
-        *jobs = NULL;
-        return 0;
-    }
 
     /* Accept new connection via the server socket (can only be a single one) */
     if(UA_fd_isset(layer->serversockfd, &fdset)) {
@@ -509,77 +483,47 @@ ServerNetworkLayerTCP_getJobs(UA_ServerNetworkLayer *nl, UA_Job **jobs,
     }
 
     /* Read from established sockets */
-    size_t j = 0;
-    UA_ByteString buf = UA_BYTESTRING_NULL;
-    for(size_t i = 0; i < layer->mappingsSize && j < (size_t)resultsize; ++i) {
-        if(!UA_fd_isset(layer->mappings[i].sockfd, &errset) &&
-           !UA_fd_isset(layer->mappings[i].sockfd, &fdset))
+    ConnectionEntry *e, *e_tmp;
+    LIST_FOREACH_SAFE(e, &layer->connections, pointers, e_tmp) {
+        UA_ByteString buf = UA_BYTESTRING_NULL;
+        if(!UA_fd_isset(e->connection.sockfd, &errset) &&
+           !UA_fd_isset(e->connection.sockfd, &fdset))
           continue;
 
-        UA_StatusCode retval = socket_recv(layer->mappings[i].connection, &buf, 0);
+        UA_StatusCode retval = socket_recv(&e->connection, &buf, 0);
         if(retval == UA_STATUSCODE_GOOD) {
-            js[totalJobs + j].job.binaryMessage.connection = layer->mappings[i].connection;
-            js[totalJobs + j].job.binaryMessage.message = buf;
-            js[totalJobs + j].type = UA_JOBTYPE_BINARYMESSAGE_NETWORKLAYER;
-            ++j;
+            UA_Server_processBinaryMessage(server, &e->connection, &buf);
         } else if (retval == UA_STATUSCODE_BADCONNECTIONCLOSED) {
-            UA_Connection *c = layer->mappings[i].connection;
-            UA_LOG_INFO(layer->logger, UA_LOGCATEGORY_NETWORK,
-                        "Connection %i | Connection closed from remote", c->sockfd);
-            /* the socket was closed from remote */
-            js[totalJobs + j].type = UA_JOBTYPE_DETACHCONNECTION;
-            js[totalJobs + j].job.closeConnection = c;
-            layer->mappings[i] = layer->mappings[layer->mappingsSize-1];
-            --layer->mappingsSize;
-            ++totalJobs; /* increase j only once */
-            js[totalJobs + j].type = UA_JOBTYPE_METHODCALL_DELAYED;
-            js[totalJobs + j].job.methodCall.method = FreeConnectionCallback;
-            js[totalJobs + j].job.methodCall.data = c;
-            ++j;
+            LIST_REMOVE(e, pointers);
+            UA_Server_removeConnection(server, &e->connection);
         }
     }
-    totalJobs += j;
-
-    if(totalJobs == 0) {
-        free(js);
-        js = NULL;
-    }
-    *jobs = js;
-    return totalJobs;
+    return UA_STATUSCODE_GOOD;
 }
 
-static size_t
-ServerNetworkLayerTCP_stop(UA_ServerNetworkLayer *nl, UA_Job **jobs) {
+static void
+ServerNetworkLayerTCP_stop(UA_ServerNetworkLayer *nl, UA_Server *server) {
     ServerNetworkLayerTCP *layer = (ServerNetworkLayerTCP *)nl->handle;
     UA_LOG_INFO(layer->logger, UA_LOGCATEGORY_NETWORK,
-                "Shutting down the TCP network layer with %d open connection(s)",
-                layer->mappingsSize);
+                "Shutting down the TCP network layer");
     shutdown((SOCKET)layer->serversockfd,2);
     CLOSESOCKET(layer->serversockfd);
-    UA_Job *items = (UA_Job *)malloc(sizeof(UA_Job) * layer->mappingsSize * 2);
-    if(!items)
-        return 0;
-    for(size_t i = 0; i < layer->mappingsSize; ++i) {
-        socket_close(layer->mappings[i].connection);
-        items[i*2].type = UA_JOBTYPE_DETACHCONNECTION;
-        items[i*2].job.closeConnection = layer->mappings[i].connection;
-        items[(i*2)+1].type = UA_JOBTYPE_METHODCALL_DELAYED;
-        items[(i*2)+1].job.methodCall.method = FreeConnectionCallback;
-        items[(i*2)+1].job.methodCall.data = layer->mappings[i].connection;
+    ConnectionEntry *e, *e_tmp;
+    LIST_FOREACH_SAFE(e, &layer->connections, pointers, e_tmp) {
+        LIST_REMOVE(e, pointers);
+        UA_Server_removeConnection(server, &e->connection);
     }
 #ifdef _WIN32
     WSACleanup();
 #endif
-    *jobs = items;
-    return layer->mappingsSize*2;
 }
 
 /* run only when the server is stopped */
-static void ServerNetworkLayerTCP_deleteMembers(UA_ServerNetworkLayer *nl) {
+static void
+ServerNetworkLayerTCP_deleteMembers(UA_ServerNetworkLayer *nl) {
     ServerNetworkLayerTCP *layer = (ServerNetworkLayerTCP *)nl->handle;
-    free(layer->mappings);
-    free(layer);
     UA_String_deleteMembers(&nl->discoveryUrl);
+    UA_free(layer);
 }
 
 UA_ServerNetworkLayer
@@ -602,7 +546,7 @@ UA_ServerNetworkLayerTCP(UA_ConnectionConfig conf, UA_UInt16 port) {
 
     nl.handle = layer;
     nl.start = ServerNetworkLayerTCP_start;
-    nl.getJobs = ServerNetworkLayerTCP_getJobs;
+    nl.listen = ServerNetworkLayerTCP_listen;
     nl.stop = ServerNetworkLayerTCP_stop;
     nl.deleteMembers = ServerNetworkLayerTCP_deleteMembers;
     return nl;
@@ -659,6 +603,7 @@ UA_ClientConnectionTCP(UA_ConnectionConfig conf, const char *endpointUrl,
     connection.send = socket_write;
     connection.recv = socket_recv;
     connection.close = ClientNetworkLayerClose;
+    connection.free = NULL; /* Not used in the client */
     connection.getSendBuffer = ClientNetworkLayerGetBuffer;
     connection.releaseSendBuffer = ClientNetworkLayerReleaseBuffer;
     connection.releaseRecvBuffer = ClientNetworkLayerReleaseBuffer;

+ 6 - 1
src/server/ua_mdns.c

@@ -111,6 +111,11 @@ mdns_record_add_or_get(UA_Server *server, const char *record, const char *server
     return listEntry;
 }
 
+static void
+delayedFree(UA_Server *server, void *data) {
+    UA_free(data);
+}
+
 static void
 mdns_record_remove(UA_Server *server, const char *record,
                    struct serverOnNetwork_list_entry *entry) {
@@ -146,7 +151,7 @@ mdns_record_remove(UA_Server *server, const char *record,
     UA_free(entry);
 #else
     server->serverOnNetworkSize = uatomic_add_return(&server->serverOnNetworkSize, -1);
-    UA_Server_delayedFree(server, entry);
+    UA_Server_delayedCallback(server, delayedFree, entry);
 #endif
 }
 

+ 29 - 28
src/server/ua_server.c

@@ -94,10 +94,7 @@ UA_Server_forEachChildNodeCall(UA_Server *server, UA_NodeId parentNodeId,
 
 /* The server needs to be stopped before it can be deleted */
 void UA_Server_delete(UA_Server *server) {
-    // Delete the timed work
-    UA_RepeatedJobsList_deleteMembers(&server->repeatedJobs);
-
-    // Delete all internal data
+    /* Delete all internal data */
     UA_SecureChannelManager_deleteMembers(&server->secureChannelManager);
     UA_SessionManager_deleteMembers(&server->sessionManager);
     UA_RCU_LOCK();
@@ -114,8 +111,8 @@ void UA_Server_delete(UA_Server *server) {
         UA_RegisteredServer_deleteMembers(&rs->registeredServer);
         UA_free(rs);
     }
-    if(server->periodicServerRegisterJob)
-        UA_free(server->periodicServerRegisterJob);
+    if(server->periodicServerRegisterCallback)
+        UA_free(server->periodicServerRegisterCallback);
 
 # ifdef UA_ENABLE_DISCOVERY_MULTICAST
     if(server->config.applicationDescription.applicationType == UA_APPLICATIONTYPE_DISCOVERYSERVER)
@@ -146,11 +143,17 @@ void UA_Server_delete(UA_Server *server) {
     pthread_cond_destroy(&server->dispatchQueue_condition);
     pthread_mutex_destroy(&server->dispatchQueue_mutex);
 #endif
+
+    /* Delete the timed work */
+    UA_Timer_deleteMembers(&server->timer);
+
+    /* Delete the server itself */
     UA_free(server);
 }
 
 /* Recurring cleanup. Removing unused and timed-out channels and sessions */
-static void UA_Server_cleanup(UA_Server *server, void *_) {
+static void
+UA_Server_cleanup(UA_Server *server, void *_) {
     UA_DateTime nowMonotonic = UA_DateTime_nowMonotonic();
     UA_SessionManager_cleanupTimedOut(&server->sessionManager, nowMonotonic);
     UA_SecureChannelManager_cleanupTimedOut(&server->secureChannelManager, nowMonotonic);
@@ -223,14 +226,8 @@ UA_Server_new(const UA_ServerConfig config) {
     UA_random_seed((UA_UInt64)UA_DateTime_now());
 #endif
 
-    /* Initialize the handling of repeated jobs */
-#ifdef UA_ENABLE_MULTITHREADING
-    UA_RepeatedJobsList_init(&server->repeatedJobs,
-                             (UA_RepeatedJobsListProcessCallback)UA_Server_dispatchJob, server);
-#else
-    UA_RepeatedJobsList_init(&server->repeatedJobs,
-                             (UA_RepeatedJobsListProcessCallback)UA_Server_processJob, server);
-#endif
+    /* Initialize the handling of repeated callbacks */
+    UA_Timer_init(&server->timer);
 
     /* Initialized the linked list for delayed callbacks */
 #ifndef UA_ENABLE_MULTITHREADING
@@ -241,7 +238,6 @@ UA_Server_new(const UA_ServerConfig config) {
 #ifdef UA_ENABLE_MULTITHREADING
     rcu_init();
     cds_wfcq_init(&server->dispatchQueue_head, &server->dispatchQueue_tail);
-    cds_lfs_init(&server->mainLoopJobs);
 #endif
 
     /* Create Namespaces 0 and 1 */
@@ -257,18 +253,15 @@ UA_Server_new(const UA_ServerConfig config) {
     UA_SecureChannelManager_init(&server->secureChannelManager, server);
     UA_SessionManager_init(&server->sessionManager, server);
 
-    /* Add a regular job for cleanup and maintenance */
-    UA_Job cleanup;
-    cleanup.type = UA_JOBTYPE_METHODCALL;
-    cleanup.job.methodCall.data = NULL;
-    cleanup.job.methodCall.method = UA_Server_cleanup;
-    UA_Server_addRepeatedJob(server, cleanup, 10000, NULL);
+    /* Add a regular callback for cleanup and maintenance */
+    UA_Server_addRepeatedCallback(server, (UA_ServerCallback)UA_Server_cleanup, NULL,
+                                  10000, NULL);
 
     /* Initialized discovery database */
 #ifdef UA_ENABLE_DISCOVERY
     LIST_INIT(&server->registeredServers);
     server->registeredServersSize = 0;
-    server->periodicServerRegisterJob = NULL;
+    server->periodicServerRegisterCallback = NULL;
     server->registerServerCallback = NULL;
     server->registerServerCallbackData = NULL;
 #endif
@@ -307,12 +300,20 @@ UA_Server_new(const UA_ServerConfig config) {
 /*****************/
 
 UA_StatusCode
-UA_Server_addRepeatedJob(UA_Server *server, UA_Job job,
-                         UA_UInt32 interval, UA_Guid *jobId) {
-    return UA_RepeatedJobsList_addRepeatedJob(&server->repeatedJobs, job, interval, jobId);
+UA_Server_addRepeatedCallback(UA_Server *server, UA_ServerCallback callback,
+                              void *data, UA_UInt32 interval,
+                              UA_UInt64 *callbackId) {
+    return UA_Timer_addRepeatedCallback(&server->timer, (UA_TimerCallback)callback,
+                                        data, interval, callbackId);
+}
+
+UA_StatusCode
+UA_Server_changeRepeatedCallbackInterval(UA_Server *server, UA_UInt64 callbackId,
+                                         UA_UInt32 interval) {
+    return UA_Timer_changeRepeatedCallbackInterval(&server->timer, callbackId, interval);
 }
 
 UA_StatusCode
-UA_Server_removeRepeatedJob(UA_Server *server, UA_Guid jobId) {
-    return UA_RepeatedJobsList_removeRepeatedJob(&server->repeatedJobs, jobId);
+UA_Server_removeRepeatedCallback(UA_Server *server, UA_UInt64 callbackId) {
+    return UA_Timer_removeRepeatedCallback(&server->timer, callbackId);
 }

+ 81 - 7
src/server/ua_server_binary.c

@@ -589,15 +589,24 @@ UA_Server_processSecureChannelMessage(UA_Server *server, UA_SecureChannel *chann
 }
 
 /* Takes the raw message from the network layer */
-void
-UA_Server_processBinaryMessage(UA_Server *server, UA_Connection *connection,
-                               const UA_ByteString *message) {
+static void
+processBinaryMessage(UA_Server *server, UA_Connection *connection,
+                     UA_ByteString *message) {
+    UA_Boolean realloced = UA_FALSE;
+    UA_StatusCode retval = UA_Connection_completeMessages(connection, message, &realloced);
+    if(retval != UA_STATUSCODE_GOOD) {
+        if(!realloced)
+            connection->releaseRecvBuffer(connection, message);
+        else
+            UA_ByteString_deleteMembers(message);
+        return;
+    }
+    
     UA_SecureChannel *channel = connection->channel;
     if(channel) {
         /* Assemble chunks in the securechannel and process complete messages */
-        UA_StatusCode retval = 
-            UA_SecureChannel_processChunks(channel, message,
-                 (UA_ProcessMessageCallback*)UA_Server_processSecureChannelMessage, server);
+        retval = UA_SecureChannel_processChunks(channel, message,
+                      (UA_ProcessMessageCallback*)UA_Server_processSecureChannelMessage, server);
         if(retval != UA_STATUSCODE_GOOD)
             UA_LOG_TRACE_CHANNEL(server->config.logger, channel, "Procesing chunks "
                                  "resulted in error code %s", UA_StatusCode_name(retval));
@@ -605,7 +614,7 @@ UA_Server_processBinaryMessage(UA_Server *server, UA_Connection *connection,
         /* Process messages without a channel and no chunking */
         size_t offset = 0;
         UA_TcpMessageHeader tcpMessageHeader;
-        UA_StatusCode retval = UA_TcpMessageHeader_decodeBinary(message, &offset, &tcpMessageHeader);
+        retval = UA_TcpMessageHeader_decodeBinary(message, &offset, &tcpMessageHeader);
         if(retval != UA_STATUSCODE_GOOD) {
             connection->close(connection);
             return;
@@ -653,4 +662,69 @@ UA_Server_processBinaryMessage(UA_Server *server, UA_Connection *connection,
             connection->close(connection);
         }
     }
+
+    if(!realloced)
+        connection->releaseRecvBuffer(connection, message);
+    else
+        UA_ByteString_deleteMembers(message);
+}
+
+#ifndef UA_ENABLE_MULTITHREADING
+
+void
+UA_Server_processBinaryMessage(UA_Server *server, UA_Connection *connection,
+                               UA_ByteString *message) {
+    processBinaryMessage(server, connection, message);
+}
+
+#else
+
+typedef struct {
+    UA_Connection *connection;
+    UA_ByteString message;
+} ConnectionMessage;
+
+static void
+workerProcessBinaryMessage(UA_Server *server, ConnectionMessage *cm) {
+    processBinaryMessage(server, cm->connection, &cm->message);
+    UA_free(cm);
+}
+
+void
+UA_Server_processBinaryMessage(UA_Server *server, UA_Connection *connection,
+                               UA_ByteString *message) {
+    /* Allocate the memory for the callback data */
+    ConnectionMessage *cm = UA_malloc(sizeof(ConnectionMessage));
+
+    /* If malloc failed, execute immediately */
+    if(!cm) {
+        processBinaryMessage(server, connection, message);
+        return;
+    }
+
+    /* Dispatch to the workers */
+    cm->connection = connection;
+    cm->message = *message;
+    UA_Server_workerCallback(server, (UA_ServerCallback)workerProcessBinaryMessage, cm);
+}
+
+#endif
+
+/* Remove a connection after it was closed in the network layer */
+#ifdef UA_ENABLE_MULTITHREADING
+static void
+deleteConnectionTrampoline(UA_Server *server, void *data) {
+    UA_Connection *connection = (UA_Connection*)data;
+    connection->free(connection);
+}
+#endif
+
+void
+UA_Server_removeConnection(UA_Server *server, UA_Connection *connection) {
+    UA_Connection_detachSecureChannel(connection);
+#ifndef UA_ENABLE_MULTITHREADING
+    connection->free(connection);
+#else
+    UA_Server_delayedCallback(server, deleteConnectionTrampoline, connection);
+#endif
 }

+ 7 - 6
src/server/ua_server_discovery.c

@@ -13,10 +13,11 @@ register_server_with_discovery_server(UA_Server *server,
                                       const char* discoveryServerUrl,
                                       const UA_Boolean isUnregister,
                                       const char* semaphoreFilePath) {
-    /* Use a fallback URL */
-    const char *url = "opc.tcp://localhost:4840";
-    if(discoveryServerUrl)
-        url = discoveryServerUrl;
+    if(!discoveryServerUrl) {
+        UA_LOG_ERROR(server->config.logger, UA_LOGCATEGORY_SERVER,
+                     "No discovery server url provided");
+        return UA_STATUSCODE_BADINTERNALERROR;
+    }
 
     /* Create the client */
     UA_Client *client = UA_Client_new(UA_ClientConfig_standard);
@@ -24,10 +25,10 @@ register_server_with_discovery_server(UA_Server *server,
         return UA_STATUSCODE_BADOUTOFMEMORY;
 
     /* Connect the client */
-    UA_StatusCode retval = UA_Client_connect(client, url);
+    UA_StatusCode retval = UA_Client_connect(client, discoveryServerUrl);
     if(retval != UA_STATUSCODE_GOOD) {
         UA_LOG_ERROR(server->config.logger, UA_LOGCATEGORY_CLIENT,
-                     "Connecting to client failed with statuscode %s",
+                     "Connecting to the discovery server failed with statuscode %s",
                      UA_StatusCode_name(retval));
         UA_Client_delete(client);
         return retval;

+ 19 - 38
src/server/ua_server_internal.h

@@ -58,30 +58,11 @@ extern "C" {
 # define UA_ASSERT_RCU_UNLOCKED()
 #endif
 
-
 #ifdef UA_ENABLE_MULTITHREADING
-typedef struct {
-    UA_Server *server;
-    pthread_t thr;
-    UA_UInt32 counter;
-    volatile UA_Boolean running;
-    char padding[64 - sizeof(void*) - sizeof(pthread_t) -
-                 sizeof(UA_UInt32) - sizeof(UA_Boolean)]; // separate cache lines
-} UA_Worker;
-
-struct MainLoopJob {
-    struct cds_lfs_node node;
-    UA_Job job;
-};
-
-void
-UA_Server_dispatchJob(UA_Server *server, const UA_Job *job);
-
+struct UA_Worker;
+typedef struct UA_Worker UA_Worker;
 #endif
 
-void
-UA_Server_processJob(UA_Server *server, UA_Job *job);
-
 #if defined(UA_ENABLE_METHODCALLS) && defined(UA_ENABLE_SUBSCRIPTIONS)
 /* Internally used context to a session 'context' of the current mehtod call */
 extern UA_THREAD_LOCAL UA_Session* methodCallSession;
@@ -133,7 +114,7 @@ struct UA_Server {
     /* Discovery */
     LIST_HEAD(registeredServer_list, registeredServer_list_entry) registeredServers; // doubly-linked list of registered servers
     size_t registeredServersSize;
-    struct PeriodicServerRegisterJob *periodicServerRegisterJob;
+    struct PeriodicServerRegisterCallback *periodicServerRegisterCallback;
     UA_Server_registerServerCallback registerServerCallback;
     void* registerServerCallbackData;
 # ifdef UA_ENABLE_DISCOVERY_MULTICAST
@@ -161,26 +142,24 @@ struct UA_Server {
     size_t namespacesSize;
     UA_String *namespaces;
 
+    /* Callbacks with a repetition interval */
+    UA_Timer timer;
 
-    /* Jobs with a repetition interval */
-    UA_RepeatedJobsList repeatedJobs;
+    /* Delayed callbacks */
+    SLIST_HEAD(DelayedCallbacksList, UA_DelayedCallback) delayedCallbacks;
 
-#ifndef UA_ENABLE_MULTITHREADING
-    SLIST_HEAD(DelayedJobsList, UA_DelayedJob) delayedCallbacks;
-#else
+    /* Worker threads */
+#ifdef UA_ENABLE_MULTITHREADING
     /* Dispatch queue head for the worker threads (the tail should not be in the same cache line) */
     struct cds_wfcq_head dispatchQueue_head;
     UA_Worker *workers; /* there are nThread workers in a running server */
-    struct cds_lfs_stack mainLoopJobs; /* Work that shall be executed only in the main loop and not
-                                          by worker threads */
-    struct DelayedJobs *delayedJobs;
     pthread_cond_t dispatchQueue_condition; /* so the workers don't spin if the queue is empty */
     pthread_mutex_t dispatchQueue_mutex; /* mutex for access to condition variable */
     struct cds_wfcq_tail dispatchQueue_tail; /* Dispatch queue tail for the worker threads */
 #endif
 
     /* Config is the last element so that MSVC allows the usernamePasswordLogins
-       field with zero-sized array */
+     * field with zero-sized array */
     UA_ServerConfig config;
 };
 
@@ -198,15 +177,17 @@ typedef UA_StatusCode (*UA_EditNodeCallback)(UA_Server*, UA_Session*, UA_Node*,
 UA_StatusCode UA_Server_editNode(UA_Server *server, UA_Session *session, const UA_NodeId *nodeId,
                                  UA_EditNodeCallback callback, const void *data);
 
-/********************/
-/* Event Processing */
-/********************/
+/*************/
+/* Callbacks */
+/*************/
 
-void UA_Server_processBinaryMessage(UA_Server *server, UA_Connection *connection,
-                                    const UA_ByteString *message);
+/* Delayed callbacks are executed when all previously dispatched callbacks are
+ * finished */
+UA_StatusCode
+UA_Server_delayedCallback(UA_Server *server, UA_ServerCallback callback, void *data);
 
-UA_StatusCode UA_Server_delayedCallback(UA_Server *server, UA_ServerCallback callback, void *data);
-UA_StatusCode UA_Server_delayedFree(UA_Server *server, void *data);
+void
+UA_Server_workerCallback(UA_Server *server, UA_ServerCallback callback, void *data);
 
 /*********************/
 /* Utility Functions */

+ 4 - 3
src/server/ua_server_utils.c

@@ -215,15 +215,16 @@ UA_Server_editNode(UA_Server *server, UA_Session *session,
     do {
         UA_RCU_LOCK();
         UA_Node *copy = UA_NodeStore_getCopy(server->nodestore, nodeId);
-        UA_RCU_UNLOCK();
-        if(!copy)
+        if(!copy) {
+            UA_RCU_UNLOCK();
             return UA_STATUSCODE_BADOUTOFMEMORY;
+        }
         retval = callback(server, session, copy, data);
         if(retval != UA_STATUSCODE_GOOD) {
             UA_NodeStore_deleteNode(copy);
+            UA_RCU_UNLOCK();
             return retval;
         }
-        UA_RCU_LOCK();
         retval = UA_NodeStore_replace(server->nodestore, copy);
         UA_RCU_UNLOCK();
     } while(retval != UA_STATUSCODE_GOOD);

+ 246 - 352
src/server/ua_server_worker.c

@@ -5,88 +5,46 @@
 #include "ua_util.h"
 #include "ua_server_internal.h"
 
+#define UA_MAXTIMEOUT 50 /* Max timeout in ms between main-loop iterations */
+
 /**
- * There are four types of job execution:
- *
- * 1. Normal jobs (dispatched to worker threads if multithreading is activated)
- *
- * 2. Repeated jobs with a repetition interval (dispatched to worker threads)
- *
- * 3. Mainloop jobs are executed (once) from the mainloop and not in the worker threads. The server
- * contains a stack structure where all threads can add mainloop jobs for the next mainloop
- * iteration. This is used e.g. to trigger adding and removing repeated jobs without blocking the
- * mainloop.
- *
- * 4. Delayed jobs are executed once in a worker thread. But only when all normal jobs that were
- * dispatched earlier have been executed. This is achieved by a counter in the worker threads. We
- * compute from the counter if all previous jobs have finished. The delay can be very long, since we
- * try to not interfere too much with normal execution. A use case is to eventually free obsolete
- * structures that _could_ still be accessed from concurrent threads.
- *
- * - Remove the entry from the list
- * - mark it as "dead" with an atomic operation
- * - add a delayed job that frees the memory when all concurrent operations have completed
- *
- * This approach to concurrently accessible memory is known as epoch based reclamation [1]. According to
- * [2], it performs competitively well on many-core systems. Our version of EBR does however not require
- * a global epoch. Instead, every worker thread has its own epoch counter that we observe for changes.
- *
- * [1] Fraser, K. 2003. Practical lock freedom. Ph.D. thesis. Computer Laboratory, University of Cambridge.
- * [2] Hart, T. E., McKenney, P. E., Brown, A. D., & Walpole, J. (2007). Performance of memory reclamation
- *     for lockless synchronization. Journal of Parallel and Distributed Computing, 67(12), 1270-1285.
+ * Worker Threads and Dispatch Queue
+ * ---------------------------------
+ * The worker threads dequeue callbacks from a central Multi-Producer
+ * Multi-Consumer Queue (MPMC). When there are no callbacks, workers go idle.
+ * The condition to wake them up is triggered whenever a callback is
+ * dispatched.
  *
  * Future Plans: Use work-stealing to load-balance between cores.
- * [3] Le, Nhat Minh, et al. "Correct and efficient work-stealing for weak
- *     memory models." ACM SIGPLAN Notices. Vol. 48. No. 8. ACM, 2013.
- */
+ * Le, Nhat Minh, et al. "Correct and efficient work-stealing for weak memory
+ * models." ACM SIGPLAN Notices. Vol. 48. No. 8. ACM, 2013. */
 
-#define UA_MAXTIMEOUT 50 // max timeout in millisec until the next main loop iteration
+#ifdef UA_ENABLE_MULTITHREADING
 
-void
-UA_Server_processJob(UA_Server *server, UA_Job *job) {
-    UA_ASSERT_RCU_UNLOCKED();
-    UA_RCU_LOCK();
-    switch(job->type) {
-    case UA_JOBTYPE_NOTHING:
-        break;
-    case UA_JOBTYPE_DETACHCONNECTION:
-        UA_Connection_detachSecureChannel(job->job.closeConnection);
-        break;
-    case UA_JOBTYPE_BINARYMESSAGE_NETWORKLAYER:
-        {
-        UA_Server_processBinaryMessage(server, job->job.binaryMessage.connection,
-                                       &job->job.binaryMessage.message);
-        UA_Connection *connection = job->job.binaryMessage.connection;
-        connection->releaseRecvBuffer(connection, &job->job.binaryMessage.message);
-        }
-        break;
-    case UA_JOBTYPE_BINARYMESSAGE_ALLOCATED:
-        UA_Server_processBinaryMessage(server, job->job.binaryMessage.connection,
-                                       &job->job.binaryMessage.message);
-        UA_ByteString_deleteMembers(&job->job.binaryMessage.message);
-        break;
-    case UA_JOBTYPE_METHODCALL:
-    case UA_JOBTYPE_METHODCALL_DELAYED:
-        job->job.methodCall.method(server, job->job.methodCall.data);
-        break;
-    default:
-        UA_LOG_WARNING(server->config.logger, UA_LOGCATEGORY_SERVER,
-                       "Trying to execute a job of unknown type");
-        break;
-    }
-    UA_RCU_UNLOCK();
-}
+struct UA_Worker {
+    UA_Server *server;
+    pthread_t thr;
+    UA_UInt32 counter;
+    volatile UA_Boolean running;
 
-/*******************************/
-/* Worker Threads and Dispatch */
-/*******************************/
+    /* separate cache lines */
+    char padding[64 - sizeof(void*) - sizeof(pthread_t) -
+                 sizeof(UA_UInt32) - sizeof(UA_Boolean)];
+};
 
-#ifdef UA_ENABLE_MULTITHREADING
+typedef struct {
+    struct cds_wfcq_node node;
+    UA_ServerCallback callback;
+    void *data;
 
-struct DispatchJob {
-    struct cds_wfcq_node node; // node for the queue
-    UA_Job job;
-};
+    UA_Boolean delayed;         /* Is it a delayed callback? */
+    UA_Boolean countersSampled; /* Have the worker counters been sampled? */
+    UA_UInt32 workerCounters[]; /* Counter value for each worker */
+} WorkerCallback; 
+
+/* Forward Declaration */
+static void
+processDelayedCallback(UA_Server *server, WorkerCallback *dc);
 
 static void *
 workerLoop(UA_Worker *worker) {
@@ -94,231 +52,236 @@ workerLoop(UA_Worker *worker) {
     UA_UInt32 *counter = &worker->counter;
     volatile UA_Boolean *running = &worker->running;
 
-    /* Initialize the (thread local) random seed with the ram address of worker */
+    /* Initialize the (thread local) random seed with the ram address
+     * of the worker. Not for security-critical entropy! */
     UA_random_seed((uintptr_t)worker);
     rcu_register_thread();
 
     while(*running) {
-        struct DispatchJob *dj = (struct DispatchJob*)
-            cds_wfcq_dequeue_blocking(&server->dispatchQueue_head, &server->dispatchQueue_tail);
-        if(dj) {
-            UA_Server_processJob(server, &dj->job);
-            UA_free(dj);
-        } else {
-            /* nothing to do. sleep until a job is dispatched (and wakes up all worker threads) */
+        UA_atomic_add(counter, 1);
+        WorkerCallback *dc = (WorkerCallback*)
+            cds_wfcq_dequeue_blocking(&server->dispatchQueue_head,
+                                      &server->dispatchQueue_tail);
+        if(!dc) {
+            /* Nothing to do. Sleep until a callback is dispatched */
             pthread_mutex_lock(&server->dispatchQueue_mutex);
-            pthread_cond_wait(&server->dispatchQueue_condition, &server->dispatchQueue_mutex);
+            pthread_cond_wait(&server->dispatchQueue_condition,
+                              &server->dispatchQueue_mutex);
             pthread_mutex_unlock(&server->dispatchQueue_mutex);
+            continue;
         }
-        UA_atomic_add(counter, 1);
+
+        if(dc->delayed) {
+            processDelayedCallback(server, dc);
+            continue;
+        }
+        
+        UA_RCU_LOCK();
+        dc->callback(server, dc->data);
+        UA_free(dc);
+        UA_RCU_UNLOCK();
     }
 
     UA_ASSERT_RCU_UNLOCKED();
-    rcu_barrier(); // wait for all scheduled call_rcu work to complete
+    rcu_barrier();
     rcu_unregister_thread();
-    UA_LOG_DEBUG(server->config.logger, UA_LOGCATEGORY_SERVER, "Worker shut down");
+    UA_LOG_DEBUG(server->config.logger, UA_LOGCATEGORY_SERVER,
+                 "Worker shut down");
     return NULL;
 }
 
-void
-UA_Server_dispatchJob(UA_Server *server, const UA_Job *job) {
-    struct DispatchJob *dj = UA_malloc(sizeof(struct DispatchJob));
-    // todo: check malloc
-    dj->job = *job;
-    cds_wfcq_node_init(&dj->node);
-    cds_wfcq_enqueue(&server->dispatchQueue_head, &server->dispatchQueue_tail, &dj->node);
-}
-
 static void
 emptyDispatchQueue(UA_Server *server) {
-    while(!cds_wfcq_empty(&server->dispatchQueue_head, &server->dispatchQueue_tail)) {
-        struct DispatchJob *dj = (struct DispatchJob*)
-            cds_wfcq_dequeue_blocking(&server->dispatchQueue_head, &server->dispatchQueue_tail);
-        UA_Server_processJob(server, &dj->job);
-        UA_free(dj);
+    while(!cds_wfcq_empty(&server->dispatchQueue_head,
+                          &server->dispatchQueue_tail)) {
+        WorkerCallback *dc = (WorkerCallback*)
+            cds_wfcq_dequeue_blocking(&server->dispatchQueue_head,
+                                      &server->dispatchQueue_tail);
+        dc->callback(server, dc->data);
+        UA_free(dc);
     }
 }
 
 #endif
 
-/****************/
-/* Delayed Jobs */
-/****************/
+/**
+ * Repeated Callbacks
+ * ------------------
+ * Repeated Callbacks are handled by UA_Timer (used in both client and server).
+ * In the multi-threaded case, callbacks are dispatched to workers. Otherwise,
+ * they are executed immediately. */
 
-static void
-delayed_free(UA_Server *server, void *data) {
-    UA_free(data);
-}
+void
+UA_Server_workerCallback(UA_Server *server, UA_ServerCallback callback,
+                         void *data) {
+#ifndef UA_ENABLE_MULTITHREADING
+    /* Execute immediately */
+    callback(server, data);
+#else
+    /* Execute immediately if memory could not be allocated */
+    WorkerCallback *dc = UA_malloc(sizeof(WorkerCallback));
+    if(!dc) {
+        callback(server, data);
+        return;
+    }
 
-UA_StatusCode UA_Server_delayedFree(UA_Server *server, void *data) {
-    return UA_Server_delayedCallback(server, delayed_free, data);
+    /* Enqueue for the worker threads */
+    dc->callback = callback;
+    dc->data = data;
+    dc->delayed = false;
+    cds_wfcq_node_init(&dc->node);
+    cds_wfcq_enqueue(&server->dispatchQueue_head,
+                     &server->dispatchQueue_tail, &dc->node);
+
+    /* Wake up sleeping workers */
+    pthread_cond_broadcast(&server->dispatchQueue_condition);
+#endif
 }
 
+/**
+ * Delayed Callbacks
+ * -----------------
+ *
+ * Delayed Callbacks are called only when all callbacks that were dispatched
+ * prior are finished. In the single-threaded case, the callback is added to a
+ * singly-linked list that is processed at the end of the server's main-loop. In
+ * the multi-threaded case, the delay is ensure by a three-step procedure:
+ *
+ * 1. The delayed callback is dispatched to the worker queue. So it is only
+ *    dequeued when all prior callbacks have been dequeued.
+ *
+ * 2. When the callback is first dequeued by a worker, sample the counter of all
+ *    workers. Once all counters have advanced, the callback is ready.
+ *
+ * 3. Check regularly if the callback is ready by adding it back to the dispatch
+ *    queue. */
+
 #ifndef UA_ENABLE_MULTITHREADING
 
-typedef struct UA_DelayedJob {
-    SLIST_ENTRY(UA_DelayedJob) next;
-    UA_Job job;
-} UA_DelayedJob;
+typedef struct UA_DelayedCallback {
+    SLIST_ENTRY(UA_DelayedCallback) next;
+    UA_ServerCallback callback;
+    void *data;
+} UA_DelayedCallback;
 
 UA_StatusCode
-UA_Server_delayedCallback(UA_Server *server, UA_ServerCallback callback, void *data) {
-    UA_DelayedJob *dj = (UA_DelayedJob *)UA_malloc(sizeof(UA_DelayedJob));
-    if(!dj)
+UA_Server_delayedCallback(UA_Server *server, UA_ServerCallback callback,
+                          void *data) {
+    UA_DelayedCallback *dc =
+        (UA_DelayedCallback*)UA_malloc(sizeof(UA_DelayedCallback));
+    if(!dc)
         return UA_STATUSCODE_BADOUTOFMEMORY;
-    dj->job.type = UA_JOBTYPE_METHODCALL;
-    dj->job.job.methodCall.data = data;
-    dj->job.job.methodCall.method = callback;
-    SLIST_INSERT_HEAD(&server->delayedCallbacks, dj, next);
+
+    dc->callback = callback;
+    dc->data = data;
+    SLIST_INSERT_HEAD(&server->delayedCallbacks, dc, next);
     return UA_STATUSCODE_GOOD;
 }
 
 static void
 processDelayedCallbacks(UA_Server *server) {
-    UA_DelayedJob *dj, *dj_tmp;
-    SLIST_FOREACH_SAFE(dj, &server->delayedCallbacks, next, dj_tmp) {
-        SLIST_REMOVE(&server->delayedCallbacks, dj, UA_DelayedJob, next);
-        UA_Server_processJob(server, &dj->job);
-        UA_free(dj);
+    UA_DelayedCallback *dc, *dc_tmp;
+    SLIST_FOREACH_SAFE(dc, &server->delayedCallbacks, next, dc_tmp) {
+        SLIST_REMOVE(&server->delayedCallbacks, dc, UA_DelayedCallback, next);
+        dc->callback(server, dc->data);
+        UA_free(dc);
     }
 }
 
-#else
-
-#define DELAYEDJOBSSIZE 100 // Collect delayed jobs until we have DELAYEDWORKSIZE items
+#else /* UA_ENABLE_MULTITHREADING */
 
-struct DelayedJobs {
-    struct DelayedJobs *next;
-    UA_UInt32 *workerCounters; // initially NULL until the counter are set
-    UA_UInt32 jobsCount; // the size of the array is DELAYEDJOBSSIZE, the count may be less
-    UA_Job jobs[DELAYEDJOBSSIZE]; // when it runs full, a new delayedJobs entry is created
-};
+UA_StatusCode
+UA_Server_delayedCallback(UA_Server *server, UA_ServerCallback callback,
+                          void *data) {
+    size_t dcsize = sizeof(WorkerCallback) +
+        (sizeof(UA_UInt32) * server->config.nThreads);
+    WorkerCallback *dc = UA_malloc(dcsize);
+    if(!dc)
+        return UA_STATUSCODE_BADOUTOFMEMORY;
 
-/* Dispatched as an ordinary job when the DelayedJobs list is full */
-static void getCounters(UA_Server *server, struct DelayedJobs *delayed) {
-    UA_UInt32 *counters = UA_malloc(server->config.nThreads * sizeof(UA_UInt32));
-    for(UA_UInt16 i = 0; i < server->config.nThreads; ++i)
-        counters[i] = server->workers[i].counter;
-    delayed->workerCounters = counters;
+    /* Enqueue for the worker threads */
+    dc->callback = callback;
+    dc->data = data;
+    dc->delayed = true;
+    dc->countersSampled = false;
+    cds_wfcq_node_init(&dc->node);
+    cds_wfcq_enqueue(&server->dispatchQueue_head,
+                     &server->dispatchQueue_tail, &dc->node);
+
+    /* Wake up sleeping workers */
+    pthread_cond_broadcast(&server->dispatchQueue_condition);
+    return UA_STATUSCODE_GOOD;
 }
 
-/* Call from the main thread only. This is the only function that modifies */
-/* server->delayedWork. processDelayedWorkQueue modifies the "next" (after the */
-/* head). */
+/* Called from the worker loop */
 static void
-addDelayedJob(UA_Server *server, UA_Job *job) {
-    struct DelayedJobs *dj = server->delayedJobs;
-    if(!dj || dj->jobsCount >= DELAYEDJOBSSIZE) {
-        /* create a new DelayedJobs and add it to the linked list */
-        dj = UA_malloc(sizeof(struct DelayedJobs));
-        if(!dj) {
-            UA_LOG_ERROR(server->config.logger, UA_LOGCATEGORY_SERVER,
-                         "Not enough memory to add a delayed job");
-            return;
-        }
-        dj->jobsCount = 0;
-        dj->workerCounters = NULL;
-        dj->next = server->delayedJobs;
-        server->delayedJobs = dj;
-
-        /* dispatch a method that sets the counter for the full list that comes afterwards */
-        if(dj->next) {
-            UA_Job setCounter = (UA_Job){
-                .type = UA_JOBTYPE_METHODCALL, .job.methodCall =
-                {.method = (void (*)(UA_Server*, void*))getCounters, .data = dj->next}};
-            UA_Server_dispatchJob(server, &setCounter);
-        }
-    }
-    dj->jobs[dj->jobsCount] = *job;
-    ++dj->jobsCount;
-}
+processDelayedCallback(UA_Server *server, WorkerCallback *dc) {
+    /* Set the worker counters */
+    if(!dc->countersSampled) {
+        for(size_t i = 0; i < server->config.nThreads; ++i)
+            dc->workerCounters[i] = server->workers[i].counter;
+        dc->countersSampled = true;
 
-static void
-addDelayedJobAsync(UA_Server *server, UA_Job *job) {
-    addDelayedJob(server, job);
-    UA_free(job);
-}
+        /* Re-add to the dispatch queue */
+        cds_wfcq_node_init(&dc->node);
+        cds_wfcq_enqueue(&server->dispatchQueue_head,
+                         &server->dispatchQueue_tail, &dc->node);
 
-UA_StatusCode
-UA_Server_delayedCallback(UA_Server *server, UA_ServerCallback callback, void *data) {
-    UA_Job *j = UA_malloc(sizeof(UA_Job));
-    if(!j)
-        return UA_STATUSCODE_BADOUTOFMEMORY;
-    j->type = UA_JOBTYPE_METHODCALL;
-    j->job.methodCall.data = data;
-    j->job.methodCall.method = callback;
-    struct MainLoopJob *mlw = UA_malloc(sizeof(struct MainLoopJob));
-    mlw->job = (UA_Job) {.type = UA_JOBTYPE_METHODCALL, .job.methodCall =
-                         {.data = j, .method = (UA_ServerCallback)addDelayedJobAsync}};
-    cds_lfs_push(&server->mainLoopJobs, &mlw->node);
-    return UA_STATUSCODE_GOOD;
-}
+        /* Wake up sleeping workers */
+        pthread_cond_broadcast(&server->dispatchQueue_condition);
+        return;
+    }
 
-/* Find out which delayed jobs can be executed now */
-static void
-dispatchDelayedJobs(UA_Server *server, void *_) {
-    /* start at the second */
-    struct DelayedJobs *dw = server->delayedJobs, *beforedw = dw;
-    if(dw)
-        dw = dw->next;
-
-    /* find the first delayedwork where the counters have been set and have moved */
-    while(dw) {
-        if(!dw->workerCounters) {
-            beforedw = dw;
-            dw = dw->next;
-            continue;
-        }
-        UA_Boolean allMoved = true;
-        for(size_t i = 0; i < server->config.nThreads; ++i) {
-            if(dw->workerCounters[i] == server->workers[i].counter) {
-                allMoved = false;
-                break;
-            }
-        }
-        if(allMoved)
+    /* Have all other jobs finished? */
+    UA_Boolean ready = true;
+    for(size_t i = 0; i < server->config.nThreads; ++i) {
+        if(dc->workerCounters[i] == server->workers[i].counter) {
+            ready = false;
             break;
-        beforedw = dw;
-        dw = dw->next;
+        }
     }
 
-    /* process and free all delayed jobs from here on */
-    while(dw) {
-        for(size_t i = 0; i < dw->jobsCount; ++i)
-            UA_Server_processJob(server, &dw->jobs[i]);
-        struct DelayedJobs *next = UA_atomic_xchg((void**)&beforedw->next, NULL);
-        UA_free(dw->workerCounters);
-        UA_free(dw);
-        dw = next;
+    /* Re-add to the dispatch queue.
+     * TODO: What is the impact of this loop?
+     * Can we add a small delay here? */
+    if(!ready) {
+        cds_wfcq_node_init(&dc->node);
+        cds_wfcq_enqueue(&server->dispatchQueue_head,
+                         &server->dispatchQueue_tail, &dc->node);
+
+        /* Wake up sleeping workers */
+        pthread_cond_broadcast(&server->dispatchQueue_condition);
+        return;
     }
+        
+    /* Execute the callback */
+    dc->callback(server, dc->data);
+    UA_free(dc);
 }
 
 #endif
 
-/********************/
-/* Main Server Loop */
-/********************/
+/**
+ * Main Server Loop
+ * ----------------
+ * Start: Spin up the workers and the network layer
+ * Iterate: Process repeated callbacks and events in the network layer.
+ *          This part can be driven from an external main-loop in an
+ *          event-driven single-threaded architecture.
+ * Stop: Stop workers, finish all callbacks, stop the network layer,
+ *       clean up */
 
-#ifdef UA_ENABLE_MULTITHREADING
-static void processMainLoopJobs(UA_Server *server) {
-    /* no synchronization required if we only use push and pop_all */
-    struct cds_lfs_head *head = __cds_lfs_pop_all(&server->mainLoopJobs);
-    if(!head)
-        return;
-    struct MainLoopJob *mlw = (struct MainLoopJob*)&head->node;
-    struct MainLoopJob *next;
-    do {
-        UA_Server_processJob(server, &mlw->job);
-        next = (struct MainLoopJob*)mlw->node.next;
-        UA_free(mlw);
-        //cppcheck-suppress unreadVariable
-    } while((mlw = next));
-}
-#endif
+UA_StatusCode
+UA_Server_run_startup(UA_Server *server) {
+    /* Start the networklayers */
+    UA_StatusCode result = UA_STATUSCODE_GOOD;
+    for(size_t i = 0; i < server->config.networkLayersSize; ++i) {
+        UA_ServerNetworkLayer *nl = &server->config.networkLayers[i];
+        result |= nl->start(nl, server->config.logger);
+    }
 
-UA_StatusCode UA_Server_run_startup(UA_Server *server) {
-#ifdef UA_ENABLE_MULTITHREADING
     /* Spin up the worker threads */
+#ifdef UA_ENABLE_MULTITHREADING
     UA_LOG_INFO(server->config.logger, UA_LOGCATEGORY_SERVER,
                 "Spinning up %u worker thread(s)", server->config.nThreads);
     pthread_cond_init(&server->dispatchQueue_condition, 0);
@@ -333,66 +296,26 @@ UA_StatusCode UA_Server_run_startup(UA_Server *server) {
         worker->running = true;
         pthread_create(&worker->thr, NULL, (void* (*)(void*))workerLoop, worker);
     }
-
-    /* Try to execute delayed callbacks every 10 sec */
-    UA_Job processDelayed = {.type = UA_JOBTYPE_METHODCALL,
-                             .job.methodCall = {.method = dispatchDelayedJobs, .data = NULL} };
-    UA_RepeatedJobsList_addRepeatedJob(&server->repeatedJobs, processDelayed, 10000, NULL);
 #endif
 
-    /* Start the networklayers */
-    UA_StatusCode result = UA_STATUSCODE_GOOD;
-    for(size_t i = 0; i < server->config.networkLayersSize; ++i) {
-        UA_ServerNetworkLayer *nl = &server->config.networkLayers[i];
-        result |= nl->start(nl, server->config.logger);
-    }
-
-
+    /* Start the multicast discovery server */
 #ifdef UA_ENABLE_DISCOVERY_MULTICAST
-    if(server->config.applicationDescription.applicationType == UA_APPLICATIONTYPE_DISCOVERYSERVER)
+    if(server->config.applicationDescription.applicationType ==
+       UA_APPLICATIONTYPE_DISCOVERYSERVER)
         startMulticastDiscoveryServer(server);
 #endif
 
     return result;
 }
 
-/* completeMessages is run synchronous on the jobs returned from the network
-   layer, so that the order for processing TCP packets is never mixed up. */
-static void
-completeMessages(UA_Server *server, UA_Job *job) {
-    UA_Boolean realloced = UA_FALSE;
-    UA_StatusCode retval = UA_Connection_completeMessages(job->job.binaryMessage.connection,
-                                                          &job->job.binaryMessage.message, &realloced);
-    if(retval != UA_STATUSCODE_GOOD) {
-        if(retval == UA_STATUSCODE_BADOUTOFMEMORY)
-            UA_LOG_WARNING(server->config.logger, UA_LOGCATEGORY_NETWORK,
-                           "Lost message(s) from Connection %i as memory could not be allocated",
-                           job->job.binaryMessage.connection->sockfd);
-        else if(retval != UA_STATUSCODE_GOOD)
-            UA_LOG_INFO(server->config.logger, UA_LOGCATEGORY_NETWORK,
-                        "Could not merge half-received messages on Connection %i with error 0x%08x",
-                        job->job.binaryMessage.connection->sockfd, retval);
-        job->type = UA_JOBTYPE_NOTHING;
-        return;
-    }
-    if(realloced)
-        job->type = UA_JOBTYPE_BINARYMESSAGE_ALLOCATED;
-
-    /* discard the job if message is empty - also no leak is possible here */
-    if(job->job.binaryMessage.message.length == 0)
-        job->type = UA_JOBTYPE_NOTHING;
-}
-
-UA_UInt16 UA_Server_run_iterate(UA_Server *server, UA_Boolean waitInternal) {
-#ifdef UA_ENABLE_MULTITHREADING
-    /* Run work assigned for the main thread */
-    processMainLoopJobs(server);
-#endif
+UA_UInt16
+UA_Server_run_iterate(UA_Server *server, UA_Boolean waitInternal) {
     /* Process repeated work */
     UA_DateTime now = UA_DateTime_nowMonotonic();
-    UA_Boolean dispatched = false; /* to wake up worker threads */
     UA_DateTime nextRepeated =
-        UA_RepeatedJobsList_process(&server->repeatedJobs, now, &dispatched);
+        UA_Timer_process(&server->timer, now,
+                         (UA_TimerDispatchCallback)UA_Server_workerCallback,
+                         server);
     UA_DateTime latest = now + (UA_MAXTIMEOUT * UA_MSEC_TO_DATETIME);
     if(nextRepeated > latest)
         nextRepeated = latest;
@@ -401,63 +324,30 @@ UA_UInt16 UA_Server_run_iterate(UA_Server *server, UA_Boolean waitInternal) {
     if(waitInternal)
         timeout = (UA_UInt16)((nextRepeated - now) / UA_MSEC_TO_DATETIME);
 
-    /* Get work from the networklayer */
+    /* Listen on the networklayer */
     for(size_t i = 0; i < server->config.networkLayersSize; ++i) {
         UA_ServerNetworkLayer *nl = &server->config.networkLayers[i];
-        UA_Job *jobs = NULL;
-        size_t jobsSize;
-        /* only the last networklayer waits on the tieout */
-        if(i == server->config.networkLayersSize-1)
-            jobsSize = nl->getJobs(nl, &jobs, timeout);
-        else
-            jobsSize = nl->getJobs(nl, &jobs, 0);
-
-        for(size_t k = 0; k < jobsSize; ++k) {
-#ifdef UA_ENABLE_MULTITHREADING
-            /* Filter out delayed work */
-            if(jobs[k].type == UA_JOBTYPE_METHODCALL_DELAYED) {
-                addDelayedJob(server, &jobs[k]);
-                jobs[k].type = UA_JOBTYPE_NOTHING;
-                continue;
-            }
-#endif
-            /* Merge half-received messages */
-            if(jobs[k].type == UA_JOBTYPE_BINARYMESSAGE_NETWORKLAYER)
-                completeMessages(server, &jobs[k]);
-        }
-
-        /* Dispatch/process jobs */
-        for(size_t j = 0; j < jobsSize; ++j) {
-#ifdef UA_ENABLE_MULTITHREADING
-            UA_Server_dispatchJob(server, &jobs[j]);
-            dispatched = true;
-#else
-            UA_Server_processJob(server, &jobs[j]);
-#endif
-        }
-
-        /* Clean up jobs list */
-        if(jobsSize > 0)
-            UA_free(jobs);
+        nl->listen(nl, server, timeout);
     }
 
-#ifdef UA_ENABLE_MULTITHREADING
-    /* Wake up worker threads */
-    if(dispatched)
-        pthread_cond_broadcast(&server->dispatchQueue_condition);
-#else
+#ifndef UA_ENABLE_MULTITHREADING
+    /* Process delayed callbacks when all callbacks and
+     * network events are done */
     processDelayedCallbacks(server);
 #endif
 
 #if defined(UA_ENABLE_DISCOVERY_MULTICAST) && !defined(UA_ENABLE_MULTITHREADING)
-    if(server->config.applicationDescription.applicationType == UA_APPLICATIONTYPE_DISCOVERYSERVER) {
-        UA_DateTime multicastNextRepeat = 0;
+    if(server->config.applicationDescription.applicationType ==
+       UA_APPLICATIONTYPE_DISCOVERYSERVER) {
         // TODO multicastNextRepeat does not consider new input data (requests)
         // on the socket. It will be handled on the next call. if needed, we
         // need to use select with timeout on the multicast socket
         // server->mdnsSocket (see example in mdnsd library) on higher level.
-        if(iterateMulticastDiscoveryServer(server, &multicastNextRepeat, UA_TRUE) &&
-           multicastNextRepeat < nextRepeated)
+        UA_DateTime multicastNextRepeat = 0;
+        UA_Boolean hasNext =
+            iterateMulticastDiscoveryServer(server, &multicastNextRepeat,
+                                            UA_TRUE);
+        if(hasNext && multicastNextRepeat < nextRepeated)
             nextRepeated = multicastNextRepeat;
     }
 #endif
@@ -469,49 +359,53 @@ UA_UInt16 UA_Server_run_iterate(UA_Server *server, UA_Boolean waitInternal) {
     return timeout;
 }
 
-UA_StatusCode UA_Server_run_shutdown(UA_Server *server) {
+UA_StatusCode
+UA_Server_run_shutdown(UA_Server *server) {
+    /* Stop the netowrk layer */
     for(size_t i = 0; i < server->config.networkLayersSize; ++i) {
         UA_ServerNetworkLayer *nl = &server->config.networkLayers[i];
-        UA_Job *stopJobs = NULL;
-        size_t stopJobsSize = nl->stop(nl, &stopJobs);
-        for(size_t j = 0; j < stopJobsSize; ++j)
-            UA_Server_processJob(server, &stopJobs[j]);
-        UA_free(stopJobs);
+        nl->stop(nl, server);
     }
 
-#ifdef UA_ENABLE_MULTITHREADING
-    /* Ensure that run_shutdown can be called multiple times */
+#ifndef UA_ENABLE_MULTITHREADING
+    /* Process remaining delayed callbacks */
+    processDelayedCallbacks(server);
+#else
+    /* Shut down the workers */
     if(server->workers) {
         UA_LOG_INFO(server->config.logger, UA_LOGCATEGORY_SERVER,
-                    "Shutting down %u worker thread(s)", server->config.nThreads);
-        /* Wait for all worker threads to finish */
+                    "Shutting down %u worker thread(s)",
+                    server->config.nThreads);
         for(size_t i = 0; i < server->config.nThreads; ++i)
             server->workers[i].running = false;
         pthread_cond_broadcast(&server->dispatchQueue_condition);
         for(size_t i = 0; i < server->config.nThreads; ++i)
             pthread_join(server->workers[i].thr, NULL);
-        /* Free the worker structures */
         UA_free(server->workers);
         server->workers = NULL;
     }
 
-    /* Manually finish the work still enqueued */
+    /* Execute the remaining callbacks in the dispatch queue.
+     * This also executes the delayed callbacks. */
     emptyDispatchQueue(server);
+    
+    /* Wait for all scheduled call_rcu work to complete */
     UA_ASSERT_RCU_UNLOCKED();
-    rcu_barrier(); // wait for all scheduled call_rcu work to complete
-#else
-    processDelayedCallbacks(server);
+    rcu_barrier();
 #endif
 
+    /* Stop multicast discovery */
 #ifdef UA_ENABLE_DISCOVERY_MULTICAST
-    if(server->config.applicationDescription.applicationType == UA_APPLICATIONTYPE_DISCOVERYSERVER)
+    if(server->config.applicationDescription.applicationType ==
+       UA_APPLICATIONTYPE_DISCOVERYSERVER)
         stopMulticastDiscoveryServer(server);
 #endif
 
     return UA_STATUSCODE_GOOD;
 }
 
-UA_StatusCode UA_Server_run(UA_Server *server, volatile UA_Boolean *running) {
+UA_StatusCode
+UA_Server_run(UA_Server *server, volatile UA_Boolean *running) {
     UA_StatusCode retval = UA_Server_run_startup(server);
     if(retval != UA_STATUSCODE_GOOD)
         return retval;

+ 0 - 2
src/server/ua_services_attribute.c

@@ -1221,11 +1221,9 @@ Service_Write(UA_Server *server, UA_Session *session,
 
 UA_StatusCode
 UA_Server_write(UA_Server *server, const UA_WriteValue *value) {
-    UA_RCU_LOCK();
     UA_StatusCode retval =
         UA_Server_editNode(server, &adminSession, &value->nodeId,
                            (UA_EditNodeCallback)copyAttributeIntoNode, value);
-    UA_RCU_UNLOCK();
     return retval;
 }
 

+ 96 - 110
src/server/ua_services_discovery.c

@@ -16,6 +16,7 @@
 #endif
 
 #ifdef UA_ENABLE_DISCOVERY
+
 static UA_StatusCode
 setApplicationDescriptionFromRegisteredServer(const UA_FindServersRequest *request,
                                               UA_ApplicationDescription *target,
@@ -174,7 +175,8 @@ void Service_FindServers(UA_Server *server, UA_Session *session,
         }
 
         if(addSelf) {
-            response->responseHeader.serviceResult = setApplicationDescriptionFromServer(&foundServers[0], server);
+            response->responseHeader.serviceResult =
+                setApplicationDescriptionFromServer(&foundServers[0], server);
             if(response->responseHeader.serviceResult != UA_STATUSCODE_GOOD) {
                 UA_free(foundServers);
                 if (foundServerFilteredPointer)
@@ -314,6 +316,11 @@ void Service_GetEndpoints(UA_Server *server, UA_Session *session,
 
 #ifdef UA_ENABLE_DISCOVERY
 
+static void
+freeEntry(UA_Server *server, void *entry) {
+    UA_free(entry);
+}
+
 static void
 process_RegisterServer(UA_Server *server, UA_Session *session,
                        const UA_RequestHeader* requestHeader,
@@ -429,7 +436,7 @@ process_RegisterServer(UA_Server *server, UA_Session *session,
         server->registeredServersSize--;
 #else
         server->registeredServersSize = uatomic_add_return(&server->registeredServersSize, -1);
-        UA_Server_delayedFree(server, registeredServer_entry);
+        UA_Server_delayedCallback(server, freeEntry, registeredServer_entry);
 #endif
         responseHeader->serviceResult = UA_STATUSCODE_GOOD;
         return;
@@ -545,21 +552,21 @@ void UA_Discovery_cleanupTimedOut(UA_Server *server, UA_DateTime nowMonotonic) {
             server->registeredServersSize--;
 #else
             server->registeredServersSize = uatomic_add_return(&server->registeredServersSize, -1);
-            UA_Server_delayedFree(server, current);
+            UA_Server_delayedCallback(server, freeEntry, current);
 #endif
         }
     }
 }
 
-struct PeriodicServerRegisterJob {
-    UA_UInt32 default_interval;
-    UA_Guid job_id;
-    UA_Job *job;
+struct PeriodicServerRegisterCallback {
+    UA_UInt64 id;
     UA_UInt32 this_interval;
+    UA_UInt32 default_interval;
+    UA_Boolean registered;
     const char* discovery_server_url;
 };
 
-/* Called by the UA_Server job. The OPC UA specification says:
+/* Called by the UA_Server callback. The OPC UA specification says:
  *
  * > If an error occurs during registration (e.g. the Discovery Server is not running) then the Server
  * > must periodically re-attempt registration. The frequency of these attempts should start at 1 second
@@ -570,144 +577,123 @@ struct PeriodicServerRegisterJob {
  * if the next interval is default or if it is a repeaded call. */
 static void
 periodicServerRegister(UA_Server *server, void *data) {
-    if(!data) {
-        UA_LOG_ERROR(server->config.logger, UA_LOGCATEGORY_SERVER,
-                     "Data parameter must be not NULL for periodic server register");
-        return;
-    }
+    UA_assert(data != NULL);
 
-    struct PeriodicServerRegisterJob *retryJob = (struct PeriodicServerRegisterJob *)data;
-    if(retryJob->job != NULL) {
-        // remove the retry job because we don't want to fire it again.
-        UA_Server_removeRepeatedJob(server, retryJob->job_id);
-    }
+    struct PeriodicServerRegisterCallback *cb = (struct PeriodicServerRegisterCallback *)data;
 
-    // fixme: remove magic urls
+    /* Which URL to register on */
+    // fixme: remove magic url
     const char * server_url;
-    if(retryJob->discovery_server_url != NULL)
-        server_url = retryJob->discovery_server_url;
+    if(cb->discovery_server_url != NULL)
+        server_url = cb->discovery_server_url;
     else
         server_url = "opc.tcp://localhost:4840";
+
+    /* Register
+       You can also use a semaphore file. That file must exist. When the file is
+       deleted, the server is automatically unregistered. The semaphore file has
+       to be accessible by the discovery server
+    
+       UA_StatusCode retval = UA_Server_register_discovery(server,
+       "opc.tcp://localhost:4840", "/path/to/some/file");
+    */
     UA_StatusCode retval = UA_Server_register_discovery(server, server_url, NULL);
 
-    // You can also use a semaphore file. That file must exist. When the file is
-    // deleted, the server is automatically unregistered. The semaphore file has
-    // to be accessible by the discovery server
-    //
-    // UA_StatusCode retval = UA_Server_register_discovery(server,
-    // "opc.tcp://localhost:4840", "/path/to/some/file");
+    /* Registering failed */
     if(retval != UA_STATUSCODE_GOOD) {
         UA_LOG_ERROR(server->config.logger, UA_LOGCATEGORY_SERVER,
                      "Could not register server with discovery server. "
-                     "Is the discovery server started? StatusCode %s", UA_StatusCode_name(retval));
+                     "Is the discovery server started? StatusCode %s",
+                     UA_StatusCode_name(retval));
 
-        // first retry in 1 second
-        UA_UInt32 nextInterval = 1;
+        /* If the server was previously registered, retry in one second,
+         * else, double the previous interval */
+        UA_UInt32 nextInterval = 1000;
+        if(!cb->registered)
+            nextInterval = cb->this_interval * 2;
 
-        if (retryJob->job != NULL)
-            // double the interval for the next retry
-            nextInterval = retryJob->this_interval*2;
+        /* The interval should be smaller than the default interval */
+        if(nextInterval > cb->default_interval)
+            nextInterval = cb->default_interval;
 
-        // as long as next retry is smaller than default interval, retry
-        if (nextInterval < retryJob->default_interval) {
-            UA_LOG_INFO(server->config.logger, UA_LOGCATEGORY_SERVER,
-                        "Retrying registration in %d seconds", nextInterval);
-            // todo: malloc may fail: return a statuscode
-            struct PeriodicServerRegisterJob *newRetryJob =
-                (struct PeriodicServerRegisterJob *)UA_malloc(sizeof(struct PeriodicServerRegisterJob));
-            newRetryJob->job = (UA_Job *)UA_malloc(sizeof(UA_Job));
-            newRetryJob->default_interval = retryJob->default_interval;
-            newRetryJob->this_interval = nextInterval;
-            newRetryJob->discovery_server_url = retryJob->discovery_server_url;
-
-            newRetryJob->job->type = UA_JOBTYPE_METHODCALL;
-            newRetryJob->job->job.methodCall.method = periodicServerRegister;
-            newRetryJob->job->job.methodCall.data = newRetryJob;
-
-            UA_Server_addRepeatedJob(server, *newRetryJob->job,
-                                     nextInterval*1000, &newRetryJob->job_id);
-        }
-    } else {
-        UA_LOG_DEBUG(server->config.logger, UA_LOGCATEGORY_SERVER,
-                    "Server successfully registered. Next periodical register will be in %d seconds",
-                    (int)(retryJob->default_interval/1000));
+        cb->this_interval = nextInterval;
+        UA_Server_changeRepeatedCallbackInterval(server, cb->id, nextInterval);
+        return;
     }
 
-    if(retryJob->job) {
-        UA_free(retryJob->job);
-        UA_free(retryJob);
-    }
+    /* Registering succeeded */
+    UA_LOG_DEBUG(server->config.logger, UA_LOGCATEGORY_SERVER,
+                 "Server successfully registered. Next periodical register will be in %d seconds",
+                 (int)(cb->default_interval/1000));
 
+    if(!cb->registered) {
+        retval = UA_Server_changeRepeatedCallbackInterval(server, cb->id, cb->default_interval);
+        /* If changing the interval fails, try again after the next registering */
+        if(retval == UA_STATUSCODE_GOOD)
+            cb->registered = true;
+    }
 }
 
 UA_StatusCode
-UA_Server_addPeriodicServerRegisterJob(UA_Server *server,
-                                       const char* discoveryServerUrl,
-                                       const UA_UInt32 intervalMs,
-                                       const UA_UInt32 delayFirstRegisterMs,
-                                       UA_Guid* periodicJobId) {
-    if(server->periodicServerRegisterJob != NULL)
+UA_Server_addPeriodicServerRegisterCallback(UA_Server *server,
+                                            const char* discoveryServerUrl,
+                                            UA_UInt32 intervalMs,
+                                            UA_UInt32 delayFirstRegisterMs,
+                                            UA_UInt64 *periodicCallbackId) {
+    /* There can be only one callback atm */
+    if(server->periodicServerRegisterCallback) {
+        UA_LOG_ERROR(server->config.logger, UA_LOGCATEGORY_SERVER,
+                     "There is already a register callback in place");
+        return UA_STATUSCODE_BADINTERNALERROR;
+    }
+
+    /* No valid server URL */
+    if(!discoveryServerUrl) {
+        UA_LOG_ERROR(server->config.logger, UA_LOGCATEGORY_SERVER,
+                     "No discovery server URL provided");
         return UA_STATUSCODE_BADINTERNALERROR;
+    }
+
+    /* Allocate and initialize */
+    struct PeriodicServerRegisterCallback* cb =
+        (struct PeriodicServerRegisterCallback*)
+        UA_malloc(sizeof(struct PeriodicServerRegisterCallback));
+    if(!cb)
+        return UA_STATUSCODE_BADOUTOFMEMORY;
+    server->periodicServerRegisterCallback = cb;
 
-    // registering the server should be done periodically. Approx. every 10
-    // minutes. The first call will be in 10 Minutes.
-    UA_Job job;
-    job.type = UA_JOBTYPE_METHODCALL;
-    job.job.methodCall.method = periodicServerRegister;
-    job.job.methodCall.data = NULL;
-
-    server->periodicServerRegisterJob =
-        (struct PeriodicServerRegisterJob *)UA_malloc(sizeof(struct PeriodicServerRegisterJob));
-    server->periodicServerRegisterJob->job = NULL;
-    server->periodicServerRegisterJob->this_interval = 0;
-    server->periodicServerRegisterJob->default_interval = intervalMs;
-    server->periodicServerRegisterJob->discovery_server_url = discoveryServerUrl;
-    job.job.methodCall.data = server->periodicServerRegisterJob;
+    /* Start repeating a failed register after 1s, then increase the delay. Set
+     * to 500ms, as the delay is doubled before changing the callback
+     * interval.*/
+    cb->this_interval = 500;
+    cb->default_interval = intervalMs;
+    cb->registered = false;
+    cb->discovery_server_url = discoveryServerUrl;
 
+    /* Add the callback */
     UA_StatusCode retval =
-        UA_Server_addRepeatedJob(server, job,
-                                 intervalMs, &server->periodicServerRegisterJob->job_id);
+        UA_Server_addRepeatedCallback(server, periodicServerRegister,
+                                      cb, delayFirstRegisterMs, &cb->id);
     if(retval != UA_STATUSCODE_GOOD) {
         UA_LOG_ERROR(server->config.logger, UA_LOGCATEGORY_SERVER,
                      "Could not create periodic job for server register. "
                      "StatusCode %s", UA_StatusCode_name(retval));
+        UA_free(server->periodicServerRegisterCallback);
+        server->periodicServerRegisterCallback = NULL;
         return retval;
     }
 
-    if(periodicJobId)
-        UA_Guid_copy(&server->periodicServerRegisterJob->job_id, periodicJobId);
-
-    if(delayFirstRegisterMs > 0) {
-        // Register the server with the discovery server.
-        // Delay this first registration until the server is fully initialized
-        // will be freed in the callback
-        // todo: malloc may fail: return a statuscode
-        struct PeriodicServerRegisterJob *newRetryJob =
-            (struct PeriodicServerRegisterJob *)UA_malloc(sizeof(struct PeriodicServerRegisterJob));
-        newRetryJob->job = (UA_Job*)UA_malloc(sizeof(UA_Job));
-        newRetryJob->this_interval = 1;
-        newRetryJob->default_interval = intervalMs;
-        newRetryJob->job->type = UA_JOBTYPE_METHODCALL;
-        newRetryJob->job->job.methodCall.method = periodicServerRegister;
-        newRetryJob->job->job.methodCall.data = newRetryJob;
-        newRetryJob->discovery_server_url = discoveryServerUrl;
-        retval = UA_Server_addRepeatedJob(server, *newRetryJob->job,
-                                          delayFirstRegisterMs, &newRetryJob->job_id);
-        if (retval != UA_STATUSCODE_GOOD) {
-            UA_LOG_ERROR(server->config.logger, UA_LOGCATEGORY_SERVER,
-                         "Could not create first job for server register. "
-                         "StatusCode %s", UA_StatusCode_name(retval));
-            return retval;
-        }
-    }
+    if(periodicCallbackId)
+        *periodicCallbackId = cb->id;
     return UA_STATUSCODE_GOOD;
 }
 
 void
-UA_Server_setRegisterServerCallback(UA_Server *server, UA_Server_registerServerCallback cb,
+UA_Server_setRegisterServerCallback(UA_Server *server,
+                                    UA_Server_registerServerCallback cb,
                                     void* data) {
     server->registerServerCallback = cb;
     server->registerServerCallbackData = data;
 }
 
-#endif // UA_ENABLE_DISCOVERY
+#endif /* UA_ENABLE_DISCOVERY */

+ 9 - 7
src/server/ua_services_nodemanagement.c

@@ -1390,7 +1390,9 @@ removeReferences(UA_Server *server, UA_Session *session,
 static UA_StatusCode
 deleteNode(UA_Server *server, UA_Session *session,
            const UA_NodeId *nodeId, UA_Boolean deleteReferences) {
+    UA_RCU_LOCK();
     const UA_Node *node = UA_NodeStore_get(server->nodestore, nodeId);
+    UA_RCU_UNLOCK();
     if(!node)
         return UA_STATUSCODE_BADNODEIDUNKNOWN;
 
@@ -1400,8 +1402,10 @@ deleteNode(UA_Server *server, UA_Session *session,
     /* Destroy an object before removing it */
     if(node->nodeClass == UA_NODECLASS_OBJECT) {
         /* Call the destructor from the object type */
+        UA_RCU_LOCK();
         const UA_ObjectTypeNode *typenode =
             getObjectNodeType(server, (const UA_ObjectNode*)node);
+        UA_RCU_UNLOCK();
         if(typenode && typenode->lifecycleManagement.destructor) {
             const UA_ObjectNode *on = (const UA_ObjectNode*)node;
             typenode->lifecycleManagement.destructor(*nodeId, on->instanceHandle);
@@ -1413,7 +1417,11 @@ deleteNode(UA_Server *server, UA_Session *session,
     if(deleteReferences)
         removeReferences(server, session, node);
 
-    return UA_NodeStore_remove(server->nodestore, nodeId);
+    UA_RCU_LOCK();
+    UA_StatusCode retval = UA_NodeStore_remove(server->nodestore, nodeId);
+    UA_RCU_UNLOCK();
+
+    return retval;
 }
 
 void Service_DeleteNodes(UA_Server *server, UA_Session *session,
@@ -1444,10 +1452,8 @@ void Service_DeleteNodes(UA_Server *server, UA_Session *session,
 UA_StatusCode
 UA_Server_deleteNode(UA_Server *server, const UA_NodeId nodeId,
                      UA_Boolean deleteReferences) {
-    UA_RCU_LOCK();
     UA_StatusCode retval = deleteNode(server, &adminSession,
                                       &nodeId, deleteReferences);
-    UA_RCU_UNLOCK();
     return retval;
 }
 
@@ -1604,11 +1610,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;
 }
 
@@ -1628,10 +1632,8 @@ setOLM(UA_Server *server, UA_Session *session,
 UA_StatusCode
 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;
 }
 

+ 16 - 15
src/server/ua_services_subscription.c

@@ -20,12 +20,12 @@ setSubscriptionSettings(UA_Server *server, UA_Subscription *subscription,
                         UA_UInt32 requestedLifetimeCount,
                         UA_UInt32 requestedMaxKeepAliveCount,
                         UA_UInt32 maxNotificationsPerPublish, UA_Byte priority) {
-    /* deregister the job if required */
-    UA_StatusCode retval = Subscription_unregisterPublishJob(server, subscription);
+    /* deregister the callback if required */
+    UA_StatusCode retval = Subscription_unregisterPublishCallback(server, subscription);
     if(retval != UA_STATUSCODE_GOOD)
         UA_LOG_DEBUG_SESSION(server->config.logger, subscription->session, "Subscription %u | "
-                             "Could not unregister publish job with error code 0x%08x",
-                             subscription->subscriptionID, retval);
+                             "Could not unregister publish callback with error code %s",
+                             subscription->subscriptionID, UA_StatusCode_name(retval));
 
     /* re-parameterize the subscription */
     subscription->publishingInterval = requestedPublishingInterval;
@@ -46,11 +46,11 @@ setSubscriptionSettings(UA_Server *server, UA_Subscription *subscription,
         subscription->notificationsPerPublish = server->config.maxNotificationsPerPublish;
     subscription->priority = priority;
 
-    retval = Subscription_registerPublishJob(server, subscription);
+    retval = Subscription_registerPublishCallback(server, subscription);
     if(retval != UA_STATUSCODE_GOOD)
         UA_LOG_DEBUG_SESSION(server->config.logger, subscription->session, "Subscription %u | "
-                             "Could not register publish job with error code 0x%08x",
-                             subscription->subscriptionID, retval);
+                             "Could not register publish callback with error code %s",
+                             subscription->subscriptionID, UA_StatusCode_name(retval));
 }
 
 void
@@ -145,7 +145,7 @@ static void
 setMonitoredItemSettings(UA_Server *server, UA_MonitoredItem *mon,
                          UA_MonitoringMode monitoringMode,
                          const UA_MonitoringParameters *params) {
-    MonitoredItem_unregisterSampleJob(server, mon);
+    MonitoredItem_unregisterSampleCallback(server, mon);
     mon->monitoringMode = monitoringMode;
 
     /* ClientHandle */
@@ -186,9 +186,9 @@ setMonitoredItemSettings(UA_Server *server, UA_MonitoredItem *mon,
     /* DiscardOldest */
     mon->discardOldest = params->discardOldest;
 
-    /* Register sample job if reporting is enabled */
+    /* Register sample callback if reporting is enabled */
     if(monitoringMode == UA_MONITORINGMODE_REPORTING)
-        MonitoredItem_registerSampleJob(server, mon);
+        MonitoredItem_registerSampleCallback(server, mon);
 }
 
 static const UA_String binaryEncoding = {sizeof("Default Binary")-1, (UA_Byte*)"Default Binary"};
@@ -391,9 +391,9 @@ void Service_SetMonitoringMode(UA_Server *server, UA_Session *session,
             continue;
         mon->monitoringMode = request->monitoringMode;
         if(mon->monitoringMode == UA_MONITORINGMODE_REPORTING)
-            MonitoredItem_registerSampleJob(server, mon);
+            MonitoredItem_registerSampleCallback(server, mon);
         else
-            MonitoredItem_unregisterSampleJob(server, mon);
+            MonitoredItem_unregisterSampleCallback(server, mon);
     }
 }
 
@@ -511,15 +511,16 @@ Service_DeleteSubscriptions(UA_Server *server, UA_Session *session,
         }
     }
 
-    /* Send dangling publish responses in a delayed job if the last subscription
-       was removed */
+    /* Send dangling publish responses in a delayed callback if the last
+     * subscription was removed */
     if(LIST_FIRST(&session->serverSubscriptions))
         return;
     UA_NodeId *sessionToken = UA_NodeId_new();
     if(!sessionToken)
         return;
     UA_NodeId_copy(&session->authenticationToken, sessionToken);
-    UA_Server_delayedCallback(server, (UA_ServerCallback)UA_Subscription_answerPublishRequestsNoSubscription,
+    UA_Server_delayedCallback(server,
+                              (UA_ServerCallback)UA_Subscription_answerPublishRequestsNoSubscription,
                               sessionToken);
 }
 

+ 24 - 31
src/server/ua_subscription.c

@@ -33,8 +33,8 @@ UA_MonitoredItem_new(void) {
 
 void
 MonitoredItem_delete(UA_Server *server, UA_MonitoredItem *monitoredItem) {
-    /* Remove the sampling job */
-    MonitoredItem_unregisterSampleJob(server, monitoredItem);
+    /* Remove the sampling callback */
+    MonitoredItem_unregisterSampleCallback(server, monitoredItem);
 
     /* Clear the queued samples */
     MonitoredItem_queuedValue *val, *val_tmp;
@@ -251,25 +251,21 @@ UA_MoniteredItem_SampleCallback(UA_Server *server,
 }
 
 UA_StatusCode
-MonitoredItem_registerSampleJob(UA_Server *server, UA_MonitoredItem *mon) {
-    UA_Job job;
-    job.type = UA_JOBTYPE_METHODCALL;
-    job.job.methodCall.method = (UA_ServerCallback)UA_MoniteredItem_SampleCallback;
-    job.job.methodCall.data = mon;
-    UA_StatusCode retval = UA_Server_addRepeatedJob(server, job,
-                                                    (UA_UInt32)mon->samplingInterval,
-                                                    &mon->sampleJobGuid);
+MonitoredItem_registerSampleCallback(UA_Server *server, UA_MonitoredItem *mon) {
+    UA_StatusCode retval =
+        UA_Server_addRepeatedCallback(server, (UA_ServerCallback)UA_MoniteredItem_SampleCallback,
+                                      mon, (UA_UInt32)mon->samplingInterval, &mon->sampleCallbackId);
     if(retval == UA_STATUSCODE_GOOD)
-        mon->sampleJobIsRegistered = true;
+        mon->sampleCallbackIsRegistered = true;
     return retval;
 }
 
 UA_StatusCode
-MonitoredItem_unregisterSampleJob(UA_Server *server, UA_MonitoredItem *mon) {
-    if(!mon->sampleJobIsRegistered)
+MonitoredItem_unregisterSampleCallback(UA_Server *server, UA_MonitoredItem *mon) {
+    if(!mon->sampleCallbackIsRegistered)
         return UA_STATUSCODE_GOOD;
-    mon->sampleJobIsRegistered = false;
-    return UA_Server_removeRepeatedJob(server, mon->sampleJobGuid);
+    mon->sampleCallbackIsRegistered = false;
+    return UA_Server_removeRepeatedCallback(server, mon->sampleCallbackId);
 }
 
 /****************/
@@ -293,9 +289,8 @@ UA_Subscription_new(UA_Session *session, UA_UInt32 subscriptionID) {
 }
 
 void
-UA_Subscription_deleteMembers(UA_Subscription *subscription,
-                              UA_Server *server) {
-    Subscription_unregisterPublishJob(server, subscription);
+UA_Subscription_deleteMembers(UA_Subscription *subscription, UA_Server *server) {
+    Subscription_unregisterPublishCallback(server, subscription);
 
     /* Delete monitored Items */
     UA_MonitoredItem *mon, *tmp_mon;
@@ -580,34 +575,32 @@ UA_Subscription_publishCallback(UA_Server *server, UA_Subscription *sub) {
 }
 
 UA_StatusCode
-Subscription_registerPublishJob(UA_Server *server, UA_Subscription *sub) {
-    if(sub->publishJobIsRegistered)
+Subscription_registerPublishCallback(UA_Server *server, UA_Subscription *sub) {
+    if(sub->publishCallbackIsRegistered)
         return UA_STATUSCODE_GOOD;
 
     UA_LOG_DEBUG_SESSION(server->config.logger, sub->session,
                          "Subscription %u | Register subscription publishing callback",
                          sub->subscriptionID);
-    UA_Job job;
-    job.type = UA_JOBTYPE_METHODCALL;
-    job.job.methodCall.method = (UA_ServerCallback)UA_Subscription_publishCallback;
-    job.job.methodCall.data = sub;
     UA_StatusCode retval =
-        UA_Server_addRepeatedJob(server, job, (UA_UInt32)sub->publishingInterval,
-                                 &sub->publishJobGuid);
+        UA_Server_addRepeatedCallback(server,
+                                 (UA_ServerCallback)UA_Subscription_publishCallback,
+                                 sub, (UA_UInt32)sub->publishingInterval,
+                                 &sub->publishCallbackId);
     if(retval == UA_STATUSCODE_GOOD)
-        sub->publishJobIsRegistered = true;
+        sub->publishCallbackIsRegistered = true;
     return retval;
 }
 
 UA_StatusCode
-Subscription_unregisterPublishJob(UA_Server *server, UA_Subscription *sub) {
-    if(!sub->publishJobIsRegistered)
+Subscription_unregisterPublishCallback(UA_Server *server, UA_Subscription *sub) {
+    if(!sub->publishCallbackIsRegistered)
         return UA_STATUSCODE_GOOD;
     UA_LOG_DEBUG_SESSION(server->config.logger, sub->session,
                          "Subscription %u | Unregister subscription "
                          "publishing callback", sub->subscriptionID);
-    sub->publishJobIsRegistered = false;
-    return UA_Server_removeRepeatedJob(server, sub->publishJobGuid);
+    sub->publishCallbackIsRegistered = false;
+    return UA_Server_removeRepeatedCallback(server, sub->publishCallbackId);
 }
 
 /* When the session has publish requests stored but the last subscription is

+ 10 - 10
src/server/ua_subscription.h

@@ -49,9 +49,9 @@ typedef struct UA_MonitoredItem {
     // TODO: dataEncoding is hardcoded to UA binary
     UA_DataChangeTrigger trigger;
 
-    /* Sample Job */
-    UA_Guid sampleJobGuid;
-    UA_Boolean sampleJobIsRegistered;
+    /* Sample Callback */
+    UA_UInt64 sampleCallbackId;
+    UA_Boolean sampleCallbackIsRegistered;
 
     /* Sample Queue */
     UA_ByteString lastSampledValue;
@@ -61,8 +61,8 @@ typedef struct UA_MonitoredItem {
 UA_MonitoredItem * UA_MonitoredItem_new(void);
 void MonitoredItem_delete(UA_Server *server, UA_MonitoredItem *monitoredItem);
 void UA_MoniteredItem_SampleCallback(UA_Server *server, UA_MonitoredItem *monitoredItem);
-UA_StatusCode MonitoredItem_registerSampleJob(UA_Server *server, UA_MonitoredItem *mon);
-UA_StatusCode MonitoredItem_unregisterSampleJob(UA_Server *server, UA_MonitoredItem *mon);
+UA_StatusCode MonitoredItem_registerSampleCallback(UA_Server *server, UA_MonitoredItem *mon);
+UA_StatusCode MonitoredItem_unregisterSampleCallback(UA_Server *server, UA_MonitoredItem *mon);
 
 /****************/
 /* Subscription */
@@ -104,9 +104,9 @@ struct UA_Subscription {
     UA_UInt32 currentLifetimeCount;
     UA_UInt32 lastMonitoredItemId;
 
-    /* Publish Job */
-    UA_Guid publishJobGuid;
-    UA_Boolean publishJobIsRegistered;
+    /* Publish Callback */
+    UA_UInt64 publishCallbackId;
+    UA_Boolean publishCallbackIsRegistered;
 
     /* MonitoredItems */
     LIST_HEAD(UA_ListOfUAMonitoredItems, UA_MonitoredItem) monitoredItems;
@@ -118,8 +118,8 @@ struct UA_Subscription {
 
 UA_Subscription * UA_Subscription_new(UA_Session *session, UA_UInt32 subscriptionID);
 void UA_Subscription_deleteMembers(UA_Subscription *subscription, UA_Server *server);
-UA_StatusCode Subscription_registerPublishJob(UA_Server *server, UA_Subscription *sub);
-UA_StatusCode Subscription_unregisterPublishJob(UA_Server *server, UA_Subscription *sub);
+UA_StatusCode Subscription_registerPublishCallback(UA_Server *server, UA_Subscription *sub);
+UA_StatusCode Subscription_unregisterPublishCallback(UA_Server *server, UA_Subscription *sub);
 
 UA_StatusCode
 UA_Subscription_deleteMonitoredItem(UA_Server *server, UA_Subscription *sub,

+ 222 - 145
src/ua_timer.c

@@ -6,187 +6,260 @@
 #include "ua_timer.h"
 
 /* Only one thread operates on the repeated jobs. This is usually the "main"
- * thread with the event loop. All other threads may add changes to the repeated
- * jobs to a multi-producer single-consumer queue. The queue is based on a
- * design by Dmitry Vyukov.
- * http://www.1024cores.net/home/lock-free-algorithms/queues/intrusive-mpsc-node-based-queue */
-
-struct UA_RepeatedJob {
-    SLIST_ENTRY(UA_RepeatedJob) next; /* Next element in the list */
-    UA_DateTime nextTime;             /* The next time when the jobs are to be executed */
-    UA_UInt64 interval;               /* Interval in 100ns resolution */
-    UA_Guid id;                       /* Id of the repeated job */
-    UA_Job job;                       /* The job description itself */
+ * thread with the event loop. All other threads introduce changes via a
+ * multi-producer single-consumer (MPSC) queue. The queue is based on a design
+ * by Dmitry Vyukov.
+ * http://www.1024cores.net/home/lock-free-algorithms/queues/intrusive-mpsc-node-based-queue
+ *
+ * The RepeatedCallback structure is used both in the sorted list of callbacks
+ * and in the MPSC changes queue. For the changes queue, we differentiate
+ * between three cases encoded in the callback pointer.
+ *
+ * callback > 0x01: add the new repeated callback to the sorted list
+ * callback == 0x00: remove the callback with the same id
+ * callback == 0x01: change the interval of the existing callback */
+
+#define REMOVE_SENTINEL 0x00
+#define CHANGE_SENTINEL 0x01
+
+struct UA_TimerCallbackEntry {
+    SLIST_ENTRY(UA_TimerCallbackEntry) next; /* Next element in the list */
+    UA_DateTime nextTime;                    /* The next time when the callbacks
+                                              * are to be executed */
+    UA_UInt64 interval;                      /* Interval in 100ns resolution */
+    UA_UInt64 id;                            /* Id of the repeated callback */
+
+    UA_TimerCallback callback;
+    void *data;
 };
 
 void
-UA_RepeatedJobsList_init(UA_RepeatedJobsList *rjl,
-                         UA_RepeatedJobsListProcessCallback processCallback,
-                         void *processContext) {
-    SLIST_INIT(&rjl->repeatedJobs);
-    rjl->changes_head = (UA_RepeatedJob*)&rjl->changes_stub;
-    rjl->changes_tail = (UA_RepeatedJob*)&rjl->changes_stub;
-    rjl->changes_stub = NULL;
-    rjl->processCallback = processCallback;
-    rjl->processContext = processContext;
+UA_Timer_init(UA_Timer *t) {
+    SLIST_INIT(&t->repeatedCallbacks);
+    t->changes_head = (UA_TimerCallbackEntry*)&t->changes_stub;
+    t->changes_tail = (UA_TimerCallbackEntry*)&t->changes_stub;
+    t->changes_stub = NULL;
+    t->idCounter = 0;
 }
 
 static void
-enqueueChange(UA_RepeatedJobsList *rjl, UA_RepeatedJob *rj) {
-    rj->next.sle_next = NULL;
-    UA_RepeatedJob *prev = (UA_RepeatedJob *)UA_atomic_xchg((void* volatile *)&rjl->changes_head, rj);
+enqueueChange(UA_Timer *t, UA_TimerCallbackEntry *tc) {
+    tc->next.sle_next = NULL;
+    UA_TimerCallbackEntry *prev = (UA_TimerCallbackEntry*)
+        UA_atomic_xchg((void * volatile *)&t->changes_head, tc);
     /* Nothing can be dequeued while the producer is blocked here */
-    prev->next.sle_next = rj; /* Once this change is visible in the consumer,
+    prev->next.sle_next = tc; /* Once this change is visible in the consumer,
                                * the node is dequeued in the following
                                * iteration */
 }
 
-static UA_RepeatedJob *
-dequeueChange(UA_RepeatedJobsList *rjl) {
-    UA_RepeatedJob *tail = rjl->changes_tail;
-    UA_RepeatedJob *next = tail->next.sle_next;
-    if(tail == (UA_RepeatedJob*)&rjl->changes_stub) {
+static UA_TimerCallbackEntry *
+dequeueChange(UA_Timer *t) {
+    UA_TimerCallbackEntry *tail = t->changes_tail;
+    UA_TimerCallbackEntry *next = tail->next.sle_next;
+    if(tail == (UA_TimerCallbackEntry*)&t->changes_stub) {
         if(!next)
             return NULL;
-        rjl->changes_tail = next;
+        t->changes_tail = next;
         tail = next;
         next = next->next.sle_next;
     }
     if(next) {
-        rjl->changes_tail = next;
+        t->changes_tail = next;
         return tail;
     }
-    UA_RepeatedJob* head = rjl->changes_head;
+    UA_TimerCallbackEntry* head = t->changes_head;
     if(tail != head)
         return NULL;
-    enqueueChange(rjl, (UA_RepeatedJob*)&rjl->changes_stub);
+    enqueueChange(t, (UA_TimerCallbackEntry*)&t->changes_stub);
     next = tail->next.sle_next;
     if(next) {
-        rjl->changes_tail = next;
+        t->changes_tail = next;
         return tail;
     }
     return NULL;
 }
 
-/* Adding repeated jobs: Add an entry with the "nextTime" timestamp in the
+/* Adding repeated callbacks: Add an entry with the "nextTime" timestamp in the
  * future. This will be picked up in the next iteration and inserted at the
  * correct place. So that the next execution takes place ät "nextTime". */
 UA_StatusCode
-UA_RepeatedJobsList_addRepeatedJob(UA_RepeatedJobsList *rjl, const UA_Job job,
-                                   const UA_UInt32 interval, UA_Guid *jobId) {
+UA_Timer_addRepeatedCallback(UA_Timer *t, UA_TimerCallback callback,
+                             void *data, UA_UInt32 interval,
+                             UA_UInt64 *callbackId) {
+    /* A callback method needs to be present */
+    if(!callback)
+        return UA_STATUSCODE_BADINTERNALERROR;
+
     /* The interval needs to be at least 5ms */
     if(interval < 5)
         return UA_STATUSCODE_BADINTERNALERROR;
 
-    /* Allocate the repeated job structure */
-    UA_RepeatedJob *rj = (UA_RepeatedJob*)UA_malloc(sizeof(UA_RepeatedJob));
-    if(!rj)
+    /* Allocate the repeated callback structure */
+    UA_TimerCallbackEntry *tc =
+        (UA_TimerCallbackEntry*)UA_malloc(sizeof(UA_TimerCallbackEntry));
+    if(!tc)
         return UA_STATUSCODE_BADOUTOFMEMORY;
 
-    /* Set the repeated job */
-    rj->interval = (UA_UInt64)interval * (UA_UInt64)UA_MSEC_TO_DATETIME;
-    rj->id = UA_Guid_random();
-    rj->job = job;
-    rj->nextTime = UA_DateTime_nowMonotonic() + (UA_DateTime)rj->interval;
+    /* Set the repeated callback */
+    tc->interval = (UA_UInt64)interval * (UA_UInt64)UA_MSEC_TO_DATETIME;
+    tc->id = ++t->idCounter;
+    tc->callback = callback;
+    tc->data = data;
+    tc->nextTime = UA_DateTime_nowMonotonic() + (UA_DateTime)tc->interval;
 
-    /* Set the output guid */
-    if(jobId)
-        *jobId = rj->id;
+    /* Set the output identifier */
+    if(callbackId)
+        *callbackId = tc->id;
 
     /* Enqueue the changes in the MPSC queue */
-    enqueueChange(rjl, rj);
+    enqueueChange(t, tc);
     return UA_STATUSCODE_GOOD;
 }
 
 static void
-addRepeatedJob(UA_RepeatedJobsList *rjl,
-               UA_RepeatedJob * UA_RESTRICT rj,
-               UA_DateTime nowMonotonic) {
-    /* The latest time for the first execution */
-    rj->nextTime = nowMonotonic + (UA_Int64)rj->interval;
-
-    /* Find the last entry before this job */
-    UA_RepeatedJob *tmpRj, *afterRj = NULL;
-    SLIST_FOREACH(tmpRj, &rjl->repeatedJobs, next) {
-        if(tmpRj->nextTime >= rj->nextTime)
+addTimerCallbackEntry(UA_Timer *t, UA_TimerCallbackEntry * UA_RESTRICT tc) {
+    /* Find the last entry before this callback */
+    UA_TimerCallbackEntry *tmpTc, *afterTc = NULL;
+    SLIST_FOREACH(tmpTc, &t->repeatedCallbacks, next) {
+        if(tmpTc->nextTime >= tc->nextTime)
             break;
-        afterRj = tmpRj;
+        afterTc = tmpTc;
 
-        /* The goal is to have many repeated jobs with the same repetition
+        /* The goal is to have many repeated callbacks with the same repetition
          * interval in a "block" in order to reduce linear search for re-entry
          * to the sorted list after processing. Allow the first execution to lie
-         * between "nextTime - 1s" and "nextTime" if this adjustment groups jobs
-         * with the same repetition interval. */
-        if(tmpRj->interval == rj->interval &&
-           tmpRj->nextTime > (rj->nextTime - UA_SEC_TO_DATETIME))
-            rj->nextTime = tmpRj->nextTime;
+         * between "nextTime - 1s" and "nextTime" if this adjustment groups
+         * callbacks with the same repetition interval. */
+        if(tmpTc->interval == tc->interval &&
+           tmpTc->nextTime > (tc->nextTime - UA_SEC_TO_DATETIME))
+            tc->nextTime = tmpTc->nextTime;
     }
 
-    /* Add the repeated job */
-    if(afterRj)
-        SLIST_INSERT_AFTER(afterRj, rj, next);
+    /* Add the repeated callback */
+    if(afterTc)
+        SLIST_INSERT_AFTER(afterTc, tc, next);
     else
-        SLIST_INSERT_HEAD(&rjl->repeatedJobs, rj, next);
+        SLIST_INSERT_HEAD(&t->repeatedCallbacks, tc, next);
 }
 
-/* Removing a repeated job: Add an entry with the "nextTime" timestamp set to
- * UA_INT64_MAX. The next iteration picks this up and removes the repated job
- * from the linked list. */
 UA_StatusCode
-UA_RepeatedJobsList_removeRepeatedJob(UA_RepeatedJobsList *rjl, const UA_Guid jobId) {
-    /* Allocate the repeated job structure */
-    UA_RepeatedJob *rj = (UA_RepeatedJob*)UA_malloc(sizeof(UA_RepeatedJob));
-    if(!rj)
+UA_Timer_changeRepeatedCallbackInterval(UA_Timer *t, UA_UInt64 callbackId,
+                                        UA_UInt32 interval) {
+    /* The interval needs to be at least 5ms */
+    if(interval < 5)
+        return UA_STATUSCODE_BADINTERNALERROR;
+
+    /* Allocate the repeated callback structure */
+    UA_TimerCallbackEntry *tc =
+        (UA_TimerCallbackEntry*)UA_malloc(sizeof(UA_TimerCallbackEntry));
+    if(!tc)
         return UA_STATUSCODE_BADOUTOFMEMORY;
 
-    /* Set the repeated job with the sentinel nextTime */
-    rj->id = jobId;
-    rj->nextTime = UA_INT64_MAX;
+    /* Set the repeated callback */
+    tc->interval = (UA_UInt64)interval * (UA_UInt64)UA_MSEC_TO_DATETIME;
+    tc->id = callbackId;
+    tc->nextTime = UA_DateTime_nowMonotonic() + (UA_DateTime)tc->interval;
+    tc->callback = (UA_TimerCallback)CHANGE_SENTINEL;
 
     /* Enqueue the changes in the MPSC queue */
-    enqueueChange(rjl, rj);
+    enqueueChange(t, tc);
     return UA_STATUSCODE_GOOD;
 }
 
 static void
-removeRepeatedJob(UA_RepeatedJobsList *rjl, const UA_Guid *jobId) {
-    UA_RepeatedJob *rj, *prev = NULL;
-    SLIST_FOREACH(rj, &rjl->repeatedJobs, next) {
-        if(UA_Guid_equal(jobId, &rj->id)) {
+changeTimerCallbackEntryInterval(UA_Timer *t, UA_UInt64 callbackId,
+                                 UA_UInt64 interval, UA_DateTime nextTime) {
+    /* Remove from the sorted list */
+    UA_TimerCallbackEntry *tc, *prev = NULL;
+    SLIST_FOREACH(tc, &t->repeatedCallbacks, next) {
+        if(callbackId == tc->id) {
             if(prev)
                 SLIST_REMOVE_AFTER(prev, next);
             else
-                SLIST_REMOVE_HEAD(&rjl->repeatedJobs, next);
-            UA_free(rj);
+                SLIST_REMOVE_HEAD(&t->repeatedCallbacks, next);
             break;
         }
-        prev = rj;
+        prev = tc;
     }
+    if(!tc)
+        return;
+
+    /* Adjust settings */
+    tc->interval = interval;
+    tc->nextTime = nextTime;
+
+    /* Reinsert at the new position */
+    addTimerCallbackEntry(t, tc);
+}
+
+/* Removing a repeated callback: Add an entry with the "nextTime" timestamp set
+ * to UA_INT64_MAX. The next iteration picks this up and removes the repated
+ * callback from the linked list. */
+UA_StatusCode
+UA_Timer_removeRepeatedCallback(UA_Timer *t, UA_UInt64 callbackId) {
+    /* Allocate the repeated callback structure */
+    UA_TimerCallbackEntry *tc =
+        (UA_TimerCallbackEntry*)UA_malloc(sizeof(UA_TimerCallbackEntry));
+    if(!tc)
+        return UA_STATUSCODE_BADOUTOFMEMORY;
+
+    /* Set the repeated callback with the sentinel nextTime */
+    tc->id = callbackId;
+    tc->callback = (UA_TimerCallback)REMOVE_SENTINEL;
+
+    /* Enqueue the changes in the MPSC queue */
+    enqueueChange(t, tc);
+    return UA_STATUSCODE_GOOD;
 }
 
 static void
-processChanges(UA_RepeatedJobsList *rjl, UA_DateTime nowMonotonic) {
-    UA_RepeatedJob *change;
-    while((change = dequeueChange(rjl))) {
-        if(change->nextTime < UA_INT64_MAX) {
-            addRepeatedJob(rjl, change, nowMonotonic);
-        } else {
-            removeRepeatedJob(rjl, &change->id);
+removeRepeatedCallback(UA_Timer *t, UA_UInt64 callbackId) {
+    UA_TimerCallbackEntry *tc, *prev = NULL;
+    SLIST_FOREACH(tc, &t->repeatedCallbacks, next) {
+        if(callbackId == tc->id) {
+            if(prev)
+                SLIST_REMOVE_AFTER(prev, next);
+            else
+                SLIST_REMOVE_HEAD(&t->repeatedCallbacks, next);
+            UA_free(tc);
+            break;
+        }
+        prev = tc;
+    }
+}
+
+/* Process the changes that were added to the MPSC queue (by other threads) */
+static void
+processChanges(UA_Timer *t) {
+    UA_TimerCallbackEntry *change;
+    while((change = dequeueChange(t))) {
+        switch((uintptr_t)change->callback) {
+        case REMOVE_SENTINEL:
+            removeRepeatedCallback(t, change->id);
+            UA_free(change);
+            break;
+        case CHANGE_SENTINEL:
+            changeTimerCallbackEntryInterval(t, change->id, change->interval,
+                                           change->nextTime);
             UA_free(change);
+            break;
+        default:
+            addTimerCallbackEntry(t, change);
         }
     }
 }
 
 UA_DateTime
-UA_RepeatedJobsList_process(UA_RepeatedJobsList *rjl,
-                            UA_DateTime nowMonotonic,
-                            UA_Boolean *dispatched) {
-    /* Insert and remove jobs */
-    processChanges(rjl, nowMonotonic);
-
-    /* Find the last job to be executed now */
-    UA_RepeatedJob *firstAfter, *lastNow = NULL;
-    SLIST_FOREACH(firstAfter, &rjl->repeatedJobs, next) {
+UA_Timer_process(UA_Timer *t, UA_DateTime nowMonotonic,
+                 UA_TimerDispatchCallback dispatchCallback,
+                 void *application) {
+    /* Insert and remove callbacks */
+    processChanges(t);
+
+    /* Find the last callback to be executed now */
+    UA_TimerCallbackEntry *firstAfter, *lastNow = NULL;
+    SLIST_FOREACH(firstAfter, &t->repeatedCallbacks, next) {
         if(firstAfter->nextTime > nowMonotonic)
             break;
         lastNow = firstAfter;
@@ -199,79 +272,83 @@ UA_RepeatedJobsList_process(UA_RepeatedJobsList *rjl,
         return UA_INT64_MAX;
     }
 
-    /* Put the jobs that are executed now in a separate list */
-    struct memberstruct(UA_RepeatedJobsList,RepeatedJobsSList) executedNowList;
-    executedNowList.slh_first = SLIST_FIRST(&rjl->repeatedJobs);
+    /* Put the callbacks that are executed now in a separate list */
+    UA_TimerCallbackList executedNowList;
+    executedNowList.slh_first = SLIST_FIRST(&t->repeatedCallbacks);
     lastNow->next.sle_next = NULL;
 
     /* Fake entry to represent the first element in the newly-sorted list */
-    UA_RepeatedJob tmp_first;
+    UA_TimerCallbackEntry tmp_first;
     tmp_first.nextTime = nowMonotonic - 1; /* never matches for last_dispatched */
     tmp_first.next.sle_next = firstAfter;
-    UA_RepeatedJob *last_dispatched = &tmp_first;
+    UA_TimerCallbackEntry *last_dispatched = &tmp_first;
 
-    /* Iterate over the list of jobs to process now */
-    UA_RepeatedJob *rj;
-    while((rj = SLIST_FIRST(&executedNowList))) {
+    /* Iterate over the list of callbacks to process now */
+    UA_TimerCallbackEntry *tc;
+    while((tc = SLIST_FIRST(&executedNowList))) {
         /* Remove from the list */
         SLIST_REMOVE_HEAD(&executedNowList, next);
 
-        /* Dispatch/process job */
-        rjl->processCallback(rjl->processContext, &rj->job);
-        *dispatched = true;
+        /* Dispatch/process callback */
+        dispatchCallback(application, tc->callback, tc->data);
 
         /* Set the time for the next execution. Prevent an infinite loop by
          * forcing the next processing into the next iteration. */
-        rj->nextTime += (UA_Int64)rj->interval;
-        if(rj->nextTime < nowMonotonic)
-            rj->nextTime = nowMonotonic + 1;
-
-        /* Find the new position for rj to keep the list sorted */
-        UA_RepeatedJob *prev_rj;
-        if(last_dispatched->nextTime == rj->nextTime) {
-            /* We "batch" repeatedJobs with the same interval in
-             * addRepeatedJobs. So this might occur quite often. */
+        tc->nextTime += (UA_Int64)tc->interval;
+        if(tc->nextTime < nowMonotonic)
+            tc->nextTime = nowMonotonic + 1;
+
+        /* Find the new position for tc to keep the list sorted */
+        UA_TimerCallbackEntry *prev_tc;
+        if(last_dispatched->nextTime == tc->nextTime) {
+            /* We try to "batch" repeatedCallbacks with the same interval. This
+             * saves a linear search when the last dispatched entry has the same
+             * nextTime timestamp as this entry. */
             UA_assert(last_dispatched != &tmp_first);
-            prev_rj = last_dispatched;
+            prev_tc = last_dispatched;
         } else {
             /* Find the position for the next execution by a linear search
-             * starting at the first possible job */
-            prev_rj = &tmp_first;
+             * starting at last_dispatched or the first element */
+            if(last_dispatched->nextTime < tc->nextTime)
+                prev_tc = last_dispatched;
+            else
+                prev_tc = &tmp_first;
+
             while(true) {
-                UA_RepeatedJob *n = SLIST_NEXT(prev_rj, next);
-                if(!n || n->nextTime >= rj->nextTime)
+                UA_TimerCallbackEntry *n = SLIST_NEXT(prev_tc, next);
+                if(!n || n->nextTime >= tc->nextTime)
                     break;
-                prev_rj = n;
+                prev_tc = n;
             }
 
             /* Update last_dispatched */
-            last_dispatched = rj;
+            last_dispatched = tc;
         }
 
         /* Add entry to the new position in the sorted list */
-        SLIST_INSERT_AFTER(prev_rj, rj, next);
+        SLIST_INSERT_AFTER(prev_tc, tc, next);
     }
 
     /* Set the entry-point for the newly sorted list */
-    rjl->repeatedJobs.slh_first = tmp_first.next.sle_next;
+    t->repeatedCallbacks.slh_first = tmp_first.next.sle_next;
 
-    /* Re-repeat processAddRemoved since one of the jobs might have removed or
-     * added a job. So we get the returned timeout right. */
-    processChanges(rjl, nowMonotonic);
+    /* Re-repeat processAddRemoved since one of the callbacks might have removed
+     * or added a callback. So we return a correct timeout. */
+    processChanges(t);
 
     /* Return timestamp of next repetition */
-    return SLIST_FIRST(&rjl->repeatedJobs)->nextTime;
+    return SLIST_FIRST(&t->repeatedCallbacks)->nextTime;
 }
 
 void
-UA_RepeatedJobsList_deleteMembers(UA_RepeatedJobsList *rjl) {
-    /* Process changes to empty the queue */
-    processChanges(rjl, 0);
-
-    /* Remove repeated jobs */
-    UA_RepeatedJob *current;
-    while((current = SLIST_FIRST(&rjl->repeatedJobs))) {
-        SLIST_REMOVE_HEAD(&rjl->repeatedJobs, next);
+UA_Timer_deleteMembers(UA_Timer *t) {
+    /* Process changes to empty the MPSC queue */
+    processChanges(t);
+
+    /* Remove repeated callbacks */
+    UA_TimerCallbackEntry *current;
+    while((current = SLIST_FIRST(&t->repeatedCallbacks))) {
+        SLIST_REMOVE_HEAD(&t->repeatedCallbacks, next);
         UA_free(current);
     }
 }

+ 51 - 38
src/ua_timer.h

@@ -10,54 +10,67 @@ extern "C" {
 #endif
 
 #include "ua_util.h"
-#include "ua_job.h"
 
-typedef void
-(*UA_RepeatedJobsListProcessCallback)(void *processContext, UA_Job *job);
+/* An (event) timer triggers callbacks with a recurring interval. Adding,
+ * removing and changing repeated callbacks can be done from independent
+ * threads. Processing the changes and dispatching callbacks must be done by a
+ * single "mainloop" process. */
 
-struct UA_RepeatedJob;
-typedef struct UA_RepeatedJob UA_RepeatedJob;
+/* Forward declaration */
+struct UA_TimerCallbackEntry;
+typedef struct UA_TimerCallbackEntry UA_TimerCallbackEntry;
+
+/* Linked-list definition */
+typedef SLIST_HEAD(UA_TimerCallbackList, UA_TimerCallbackEntry) UA_TimerCallbackList;
 
 typedef struct {
-    /* The linked list of jobs is sorted according to the execution timestamp. */
-    SLIST_HEAD(RepeatedJobsSList, UA_RepeatedJob) repeatedJobs;
-
-    /* Changes to the repeated jobs in a multi-producer single-consumer queue */
-    UA_RepeatedJob * volatile changes_head;
-    UA_RepeatedJob *changes_tail;
-    UA_RepeatedJob *changes_stub;
-
-    /* The callback to process jobs that have timed out */
-    UA_RepeatedJobsListProcessCallback processCallback;
-    void *processContext;
-} UA_RepeatedJobsList;
-
-/* Initialize the RepeatedJobsSList. Not thread-safe. */
-void
-UA_RepeatedJobsList_init(UA_RepeatedJobsList *rjl,
-                         UA_RepeatedJobsListProcessCallback processCallback,
-                         void *processContext);
-
-/* Add a repated job. Thread-safe, can be used in parallel and in parallel with
- * UA_RepeatedJobsList_process. */
+    /* The linked list of callbacks is sorted according to the execution timestamp. */
+    UA_TimerCallbackList repeatedCallbacks;
+
+    /* Changes to the repeated callbacks in a multi-producer single-consumer queue */
+    UA_TimerCallbackEntry * volatile changes_head;
+    UA_TimerCallbackEntry *changes_tail;
+    UA_TimerCallbackEntry *changes_stub;
+
+    UA_UInt64 idCounter;
+} UA_Timer;
+
+/* Initialize the Timer. Not thread-safe. */
+void UA_Timer_init(UA_Timer *t);
+
+/* Add a repated callback. Thread-safe, can be used in parallel and in parallel
+ * with UA_Timer_process. */
+typedef void (*UA_TimerCallback)(void *application, void *data);
+
+UA_StatusCode
+UA_Timer_addRepeatedCallback(UA_Timer *t, UA_TimerCallback callback, void *data,
+                             UA_UInt32 interval, UA_UInt64 *callbackId);
+
+/* Change the callback interval. If this is called from within the callback. The
+ * adjustment is made during the next _process call. */
 UA_StatusCode
-UA_RepeatedJobsList_addRepeatedJob(UA_RepeatedJobsList *rjl, const UA_Job job,
-                                   const UA_UInt32 interval, UA_Guid *jobId);
+UA_Timer_changeRepeatedCallbackInterval(UA_Timer *t, UA_UInt64 callbackId,
+                                        UA_UInt32 interval);
 
-/* Remove a repated job. Thread-safe, can be used in parallel and in parallel
- * with UA_RepeatedJobsList_process. */
+/* Remove a repated callback. Thread-safe, can be used in parallel and in
+ * parallel with UA_Timer_process. */
 UA_StatusCode
-UA_RepeatedJobsList_removeRepeatedJob(UA_RepeatedJobsList *rjl, const UA_Guid jobId);
+UA_Timer_removeRepeatedCallback(UA_Timer *t, UA_UInt64 callbackId);
+
+/* Process (dispatch) the repeated callbacks that have timed out. Returns the
+ * timestamp of the next scheduled repeated callback. Not thread-safe.
+ * Application is a pointer to the client / server environment for the callback.
+ * Dispatched is set to true when at least one callback was run / dispatched. */
+typedef void (*UA_TimerDispatchCallback)(void *application, UA_TimerCallback callback,
+                                         void *data);
 
-/* Process the repeated jobs that have timed out. Returns the timestamp of the
- * next scheduled repeated job. Not thread-safe. */
 UA_DateTime
-UA_RepeatedJobsList_process(UA_RepeatedJobsList *rjl, UA_DateTime nowMonotonic,
-                            UA_Boolean *dispatched);
+UA_Timer_process(UA_Timer *t, UA_DateTime nowMonotonic,
+                 UA_TimerDispatchCallback dispatchCallback,
+                 void *application);
 
-/* Remove all repeated jobs. Not thread-safe. */
-void
-UA_RepeatedJobsList_deleteMembers(UA_RepeatedJobsList *rjl);
+/* Remove all repeated callbacks. Not thread-safe. */
+void UA_Timer_deleteMembers(UA_Timer *t);
 
 #ifdef __cplusplus
 } // extern "C"

+ 190 - 177
tests/check_discovery.c

@@ -31,7 +31,6 @@
 #include "check.h"
 #include "testing_clock.h"
 
-
 // set register timeout to 1 second so we are able to test it.
 #define registerTimeout 1
 // cleanup is only triggered every 10 seconds, thus wait a bit longer to check
@@ -54,7 +53,8 @@ static void setup_lds(void) {
     *running_lds = true;
     UA_ServerConfig config_lds = UA_ServerConfig_standard;
     config_lds.applicationDescription.applicationType = UA_APPLICATIONTYPE_DISCOVERYSERVER;
-    config_lds.applicationDescription.applicationUri = UA_String_fromChars("urn:open62541.test.local_discovery_server");
+    config_lds.applicationDescription.applicationUri =
+        UA_String_fromChars("urn:open62541.test.local_discovery_server");
     config_lds.applicationDescription.applicationName.locale = UA_String_fromChars("en");
     config_lds.applicationDescription.applicationName.text = UA_String_fromChars("LDS Server");
     config_lds.mdnsServerName = UA_String_fromChars("LDS_test");
@@ -82,18 +82,19 @@ static void teardown_lds(void) {
     UA_String_deleteMembers(&server_lds->config.applicationDescription.applicationUri);
     UA_LocalizedText_deleteMembers(&server_lds->config.applicationDescription.applicationName);
     UA_String_deleteMembers(&server_lds->config.mdnsServerName);
-    UA_Array_delete(server_lds->config.serverCapabilities, server_lds->config.serverCapabilitiesSize, &UA_TYPES[UA_TYPES_STRING]);
+    UA_Array_delete(server_lds->config.serverCapabilities,
+                    server_lds->config.serverCapabilitiesSize,
+                    &UA_TYPES[UA_TYPES_STRING]);
     UA_Server_delete(server_lds);
     nl_lds.deleteMembers(&nl_lds);
 }
 
-
 UA_Server *server_register;
 UA_Boolean *running_register;
 UA_ServerNetworkLayer nl_register;
 pthread_t server_thread_register;
 
-UA_Guid periodicRegisterJobId;
+UA_UInt64 periodicRegisterCallbackId;
 
 static void * serverloop_register(void *_) {
     while(*running_register)
@@ -106,7 +107,8 @@ static void setup_register(void) {
     running_register = UA_Boolean_new();
     *running_register = true;
     UA_ServerConfig config_register = UA_ServerConfig_standard;
-    config_register.applicationDescription.applicationUri = UA_String_fromChars("urn:open62541.test.server_register");
+    config_register.applicationDescription.applicationUri =
+        UA_String_fromChars("urn:open62541.test.server_register");
     config_register.applicationDescription.applicationName.locale = UA_String_fromChars("de");
     config_register.applicationDescription.applicationName.text = UA_String_fromChars("Anmeldungsserver");
     config_register.mdnsServerName = UA_String_fromChars("Register_test");
@@ -130,63 +132,65 @@ static void teardown_register(void) {
     nl_register.deleteMembers(&nl_register);
 }
 
-
-static char* UA_String_to_char_alloc(const UA_String *str) {
-    char* ret = malloc(sizeof(char)*str->length+1);
-    memcpy( ret, str->data, str->length );
-    ret[str->length] = '\0';
-    return ret;
-}
-
 START_TEST(Server_register) {
-        UA_StatusCode retval = UA_Server_register_discovery(server_register, "opc.tcp://localhost:4840", NULL);
-        ck_assert_uint_eq(retval, UA_STATUSCODE_GOOD);
-    }
+    UA_StatusCode retval =
+        UA_Server_register_discovery(server_register, "opc.tcp://localhost:4840", NULL);
+    ck_assert_uint_eq(retval, UA_STATUSCODE_GOOD);
+}
 END_TEST
 
 START_TEST(Server_unregister) {
-        UA_StatusCode retval = UA_Server_unregister_discovery(server_register, "opc.tcp://localhost:4840");
-        ck_assert_uint_eq(retval, UA_STATUSCODE_GOOD);
-    }
+    UA_StatusCode retval =
+        UA_Server_unregister_discovery(server_register, "opc.tcp://localhost:4840");
+    ck_assert_uint_eq(retval, UA_STATUSCODE_GOOD);
+}
 END_TEST
 
 START_TEST(Server_register_semaphore) {
-        // create the semaphore
-        int fd = open("/tmp/open62541-unit-test-semaphore", O_RDWR|O_CREAT, S_IRWXU | S_IRWXG | S_IRWXO);
-        ck_assert_int_ne(fd, -1);
-        close(fd);
-
-        UA_StatusCode retval = UA_Server_register_discovery(server_register, "opc.tcp://localhost:4840", "/tmp/open62541-unit-test-semaphore");
-        ck_assert_uint_eq(retval, UA_STATUSCODE_GOOD);
-    }
+    // create the semaphore
+    int fd = open("/tmp/open62541-unit-test-semaphore", O_RDWR|O_CREAT, S_IRWXU | S_IRWXG | S_IRWXO);
+    ck_assert_int_ne(fd, -1);
+    close(fd);
+
+    UA_StatusCode retval =
+        UA_Server_register_discovery(server_register, "opc.tcp://localhost:4840",
+                                     "/tmp/open62541-unit-test-semaphore");
+    ck_assert_uint_eq(retval, UA_STATUSCODE_GOOD);
+}
 END_TEST
 
 START_TEST(Server_unregister_semaphore) {
-        // delete the semaphore, this should remove the registration automatically on next check
-        ck_assert_int_eq(remove("/tmp/open62541-unit-test-semaphore"), 0);
-    }
+    // delete the semaphore, this should remove the registration automatically on next check
+    ck_assert_int_eq(remove("/tmp/open62541-unit-test-semaphore"), 0);
+}
 END_TEST
 
 START_TEST(Server_register_periodic) {
-        // periodic register every minute, first register immediately
-        UA_StatusCode retval = UA_Server_addPeriodicServerRegisterJob(server_register, NULL, 60*1000, 100, &periodicRegisterJobId);
-        ck_assert_uint_eq(retval, UA_STATUSCODE_GOOD);
-    }
+    // periodic register every minute, first register immediately
+    UA_StatusCode retval =
+        UA_Server_addPeriodicServerRegisterCallback(server_register, "opc.tcp://localhost:4840",
+                                                    60*1000, 100, &periodicRegisterCallbackId);
+    ck_assert_uint_eq(retval, UA_STATUSCODE_GOOD);
+}
 END_TEST
 
 START_TEST(Server_unregister_periodic) {
-        // wait for first register delay
-        UA_sleep(1000);
-        sleep(1);
-        UA_Server_removeRepeatedJob(server_register, periodicRegisterJobId);
-        UA_sleep(1000);
-        sleep(1);
-        UA_StatusCode retval = UA_Server_unregister_discovery(server_register, NULL);
-        ck_assert_uint_eq(retval, UA_STATUSCODE_GOOD);
-    }
+    // wait for first register delay
+    UA_sleep(1000);
+    sleep(1);
+    UA_Server_removeRepeatedCallback(server_register, periodicRegisterCallbackId);
+    UA_StatusCode retval = UA_Server_unregister_discovery(server_register,
+                                                          "opc.tcp://localhost:4840");
+    ck_assert_uint_eq(retval, UA_STATUSCODE_GOOD);
+}
 END_TEST
 
-static void FindAndCheck(const char* expectedUris[], size_t expectedUrisSize, const char* expectedLocales[], const char* expectedNames[], const char *filterUri, const char *filterLocale) {
+static void
+FindAndCheck(const UA_String expectedUris[], size_t expectedUrisSize,
+             const UA_String expectedLocales[],
+             const UA_String expectedNames[],
+             const char *filterUri,
+             const char *filterLocale) {
     UA_Client *client = UA_Client_new(UA_ClientConfig_standard);
 
     UA_ApplicationDescription* applicationDescriptionArray = NULL;
@@ -195,7 +199,7 @@ static void FindAndCheck(const char* expectedUris[], size_t expectedUrisSize, co
     size_t serverUrisSize = 0;
     UA_String *serverUris = NULL;
 
-    if (filterUri) {
+    if(filterUri) {
         serverUrisSize = 1;
         serverUris = UA_malloc(sizeof(UA_String));
         serverUris[0] = UA_String_fromChars(filterUri);
@@ -204,23 +208,22 @@ static void FindAndCheck(const char* expectedUris[], size_t expectedUrisSize, co
     size_t localeIdsSize = 0;
     UA_String *localeIds = NULL;
 
-    if (filterLocale) {
+    if(filterLocale) {
         localeIdsSize = 1;
         localeIds = UA_malloc(sizeof(UA_String));
         localeIds[0] = UA_String_fromChars(filterLocale);
     }
 
-    UA_StatusCode retval = UA_Client_findServers(client, "opc.tcp://localhost:4840",
-                                                 serverUrisSize, serverUris, localeIdsSize, localeIds,
-                                                 &applicationDescriptionArraySize, &applicationDescriptionArray);
+    UA_StatusCode retval =
+        UA_Client_findServers(client, "opc.tcp://localhost:4840",
+                              serverUrisSize, serverUris, localeIdsSize, localeIds,
+                              &applicationDescriptionArraySize, &applicationDescriptionArray);
 
-    if (filterUri) {
+    if(filterUri)
         UA_Array_delete(serverUris, serverUrisSize, &UA_TYPES[UA_TYPES_STRING]);
-    }
 
-    if (filterLocale) {
+    if(filterLocale)
         UA_Array_delete(localeIds, localeIdsSize, &UA_TYPES[UA_TYPES_STRING]);
-    }
 
     ck_assert_uint_eq(retval, UA_STATUSCODE_GOOD);
 
@@ -228,32 +231,30 @@ static void FindAndCheck(const char* expectedUris[], size_t expectedUrisSize, co
     ck_assert_uint_eq(applicationDescriptionArraySize, expectedUrisSize);
     assert(applicationDescriptionArray != NULL);
 
-    for (size_t i=0; i < expectedUrisSize; ++i) {
-        char* serverUri = UA_String_to_char_alloc(&applicationDescriptionArray[i].applicationUri);
-        ck_assert_str_eq(serverUri, expectedUris[i]);
-        free(serverUri);
-
-        if (expectedNames && expectedNames[i] != NULL) {
-            char *name = UA_String_to_char_alloc(&applicationDescriptionArray[i].applicationName.text);
-            ck_assert_str_eq(name, expectedNames[i]);
-            free(name);
-        }
-        if (expectedLocales && expectedLocales[i] != NULL) {
-            char *locale = UA_String_to_char_alloc(&applicationDescriptionArray[i].applicationName.locale);
-            ck_assert_str_eq(locale, expectedLocales[i]);
-            free(locale);
-        }
-    }
+    for(size_t i = 0; i < expectedUrisSize; ++i) {
+        ck_assert(UA_String_equal(&applicationDescriptionArray[i].applicationUri,
+                                  &expectedUris[i]));
 
+        if(expectedNames)
+            ck_assert(UA_String_equal(&applicationDescriptionArray[i].applicationName.text,
+                                      &expectedNames[i]));
 
-    UA_Array_delete(applicationDescriptionArray, applicationDescriptionArraySize, &UA_TYPES[UA_TYPES_APPLICATIONDESCRIPTION]);
+        if (expectedLocales)
+            ck_assert(UA_String_equal(&applicationDescriptionArray[i].applicationName.locale,
+                                      &expectedLocales[i]));
+    }
+
+    UA_Array_delete(applicationDescriptionArray, applicationDescriptionArraySize,
+                    &UA_TYPES[UA_TYPES_APPLICATIONDESCRIPTION]);
 
     UA_Client_disconnect(client);
     UA_Client_delete(client);
 }
 
-static void FindOnNetworkAndCheck(char* expectedServerNames[], size_t expectedServerNamesSize, const char *filterUri, const char *filterLocale,
-                                  const char** filterCapabilities, size_t filterCapabilitiesSize) {
+static void
+FindOnNetworkAndCheck(UA_String expectedServerNames[], size_t expectedServerNamesSize,
+                      const char *filterUri, const char *filterLocale,
+                      const char** filterCapabilities, size_t filterCapabilitiesSize) {
     UA_Client *client = UA_Client_new(UA_ClientConfig_standard);
 
     UA_ServerOnNetwork* serverOnNetwork = NULL;
@@ -263,40 +264,35 @@ static void FindOnNetworkAndCheck(char* expectedServerNames[], size_t expectedSe
     size_t  serverCapabilityFilterSize = 0;
     UA_String *serverCapabilityFilter = NULL;
 
-    if (filterCapabilitiesSize) {
+    if(filterCapabilitiesSize) {
         serverCapabilityFilterSize = filterCapabilitiesSize;
         serverCapabilityFilter = UA_malloc(sizeof(UA_String) * filterCapabilitiesSize);
-        for (size_t i=0; i<filterCapabilitiesSize; i++) {
+        for(size_t i = 0; i < filterCapabilitiesSize; i++)
             serverCapabilityFilter[i] = UA_String_fromChars(filterCapabilities[i]);
-        }
     }
 
 
-    UA_StatusCode retval = UA_Client_findServersOnNetwork(client, "opc.tcp://localhost:4840", 0, 0,
-                                                          serverCapabilityFilterSize, serverCapabilityFilter,
-                                                          &serverOnNetworkSize, &serverOnNetwork);
+    UA_StatusCode retval =
+        UA_Client_findServersOnNetwork(client, "opc.tcp://localhost:4840", 0, 0,
+                                       serverCapabilityFilterSize, serverCapabilityFilter,
+                                       &serverOnNetworkSize, &serverOnNetwork);
 
-    if (serverCapabilityFilterSize) {
-        UA_Array_delete(serverCapabilityFilter, serverCapabilityFilterSize, &UA_TYPES[UA_TYPES_STRING]);
-    }
+    if(serverCapabilityFilterSize)
+        UA_Array_delete(serverCapabilityFilter, serverCapabilityFilterSize,
+                        &UA_TYPES[UA_TYPES_STRING]);
 
     ck_assert_uint_eq(retval, UA_STATUSCODE_GOOD);
 
     // only the discovery server is expected
     ck_assert_uint_eq(serverOnNetworkSize , expectedServerNamesSize);
 
-    if (expectedServerNamesSize > 0) {
+    if(expectedServerNamesSize > 0)
         ck_assert_ptr_ne(serverOnNetwork, NULL);
-    }
 
-    if (serverOnNetwork != NULL) {
-        for (size_t i=0; i<expectedServerNamesSize; i++) {
-            char* serverName = malloc(sizeof(char) * (serverOnNetwork[i].serverName.length+1));
-            memcpy( serverName, serverOnNetwork[i].serverName.data, serverOnNetwork[i].serverName.length );
-            serverName[serverOnNetwork[i].serverName.length] = '\0';
-            ck_assert_str_eq(serverName, expectedServerNames[i]);
-            free(serverName);
-        }
+    if(serverOnNetwork != NULL) {
+        for(size_t i = 0; i < expectedServerNamesSize; i++)
+            ck_assert(UA_String_equal(&serverOnNetwork[i].serverName,
+                                      &expectedServerNames[i]));
     }
 
     UA_Array_delete(serverOnNetwork, serverOnNetworkSize, &UA_TYPES[UA_TYPES_SERVERONNETWORK]);
@@ -305,9 +301,11 @@ static void FindOnNetworkAndCheck(char* expectedServerNames[], size_t expectedSe
     UA_Client_delete(client);
 }
 
-static UA_StatusCode GetEndpoints(UA_Client *client, const UA_String* endpointUrl, size_t* endpointDescriptionsSize, UA_EndpointDescription** endpointDescriptions,
-                                  const char* filterTransportProfileUri
-) {
+static UA_StatusCode
+GetEndpoints(UA_Client *client, const UA_String* endpointUrl,
+             size_t* endpointDescriptionsSize,
+             UA_EndpointDescription** endpointDescriptions,
+             const char* filterTransportProfileUri) {
     UA_GetEndpointsRequest request;
     UA_GetEndpointsRequest_init(&request);
     //request.requestHeader.authenticationToken = client->authenticationToken;
@@ -332,7 +330,9 @@ static UA_StatusCode GetEndpoints(UA_Client *client, const UA_String* endpointUr
     ck_assert_uint_eq(response.responseHeader.serviceResult, UA_STATUSCODE_GOOD);
 
     *endpointDescriptionsSize = response.endpointsSize;
-    *endpointDescriptions = (UA_EndpointDescription*)UA_Array_new(response.endpointsSize, &UA_TYPES[UA_TYPES_ENDPOINTDESCRIPTION]);
+    *endpointDescriptions =
+        (UA_EndpointDescription*)UA_Array_new(response.endpointsSize,
+                                              &UA_TYPES[UA_TYPES_ENDPOINTDESCRIPTION]);
     for(size_t i=0;i<response.endpointsSize;i++) {
         UA_EndpointDescription_init(&(*endpointDescriptions)[i]);
         UA_EndpointDescription_copy(&response.endpoints[i], &(*endpointDescriptions)[i]);
@@ -342,8 +342,9 @@ static UA_StatusCode GetEndpoints(UA_Client *client, const UA_String* endpointUr
 }
 
 
-static void GetEndpointsAndCheck(const char* discoveryUrl, const char* filterTransportProfileUri, const char* expectedEndpointUrls[], size_t expectedEndpointUrlsSize) {
-
+static void
+GetEndpointsAndCheck(const char* discoveryUrl, const char* filterTransportProfileUri,
+                     const UA_String expectedEndpointUrls[], size_t expectedEndpointUrlsSize) {
     UA_Client *client = UA_Client_new(UA_ClientConfig_standard);
 
     ck_assert_uint_eq(UA_Client_connect(client, discoveryUrl), UA_STATUSCODE_GOOD);
@@ -351,7 +352,8 @@ static void GetEndpointsAndCheck(const char* discoveryUrl, const char* filterTra
     UA_EndpointDescription* endpointArray = NULL;
     size_t endpointArraySize = 0;
     UA_String discoveryUrlUA = UA_String_fromChars(discoveryUrl);
-    UA_StatusCode retval = GetEndpoints(client, &discoveryUrlUA, &endpointArraySize, &endpointArray, filterTransportProfileUri);
+    UA_StatusCode retval = GetEndpoints(client, &discoveryUrlUA, &endpointArraySize,
+                                        &endpointArray, filterTransportProfileUri);
     ck_assert_uint_eq(retval, UA_STATUSCODE_GOOD);
     UA_String_deleteMembers(&discoveryUrlUA);
 
@@ -359,10 +361,7 @@ static void GetEndpointsAndCheck(const char* discoveryUrl, const char* filterTra
 
     for(size_t j = 0; j < endpointArraySize && j < expectedEndpointUrlsSize; j++) {
         UA_EndpointDescription* endpoint = &endpointArray[j];
-        char *eu = UA_String_to_char_alloc(&endpoint->endpointUrl);
-        ck_assert_ptr_ne(eu, NULL); // clang static analysis fix
-        ck_assert_str_eq(eu, expectedEndpointUrls[j]);
-        free(eu);
+        ck_assert(UA_String_equal(&endpoint->endpointUrl, &expectedEndpointUrls[j]));
     }
 
     UA_Array_delete(endpointArray, endpointArraySize, &UA_TYPES[UA_TYPES_ENDPOINTDESCRIPTION]);
@@ -371,122 +370,136 @@ static void GetEndpointsAndCheck(const char* discoveryUrl, const char* filterTra
 
 // Test if discovery server lists himself as registered server, before any other registration.
 START_TEST(Client_find_discovery) {
-        const char* expectedUris[] ={"urn:open62541.test.local_discovery_server"};
-        FindAndCheck(expectedUris, 1,NULL, NULL, NULL, NULL);
-    }
+    const UA_String expectedUris[] = {UA_STRING("urn:open62541.test.local_discovery_server")};
+    FindAndCheck(expectedUris, 1, NULL, NULL, NULL, NULL);
+}
 END_TEST
 
 // Test if discovery server lists himself as registered server if it is filtered by his uri
 START_TEST(Client_filter_discovery) {
-        const char* expectedUris[] ={"urn:open62541.test.local_discovery_server"};
-        FindAndCheck(expectedUris, 1,NULL, NULL, "urn:open62541.test.local_discovery_server", NULL);
-    }
+    const UA_String expectedUris[] = {UA_STRING("urn:open62541.test.local_discovery_server")};
+    FindAndCheck(expectedUris, 1, NULL, NULL, "urn:open62541.test.local_discovery_server", NULL);
+}
 END_TEST
 
 // Test if server filters locale
 START_TEST(Client_filter_locale) {
-        const char* expectedUris[] ={"urn:open62541.test.local_discovery_server", "urn:open62541.test.server_register"};
-        const char* expectedNames[] ={"LDS Server", "Anmeldungsserver"};
-        const char* expectedLocales[] ={"en", "de"};
-        // even if we request en_US, the server will return de_DE because it only has that name.
-        FindAndCheck(expectedUris, 2,expectedLocales, expectedNames, NULL, "en");
+    const UA_String expectedUris[] = {
+        UA_STRING("urn:open62541.test.local_discovery_server"),
+        UA_STRING("urn:open62541.test.server_register")
+    };
+    const UA_String expectedNames[] = {
+        UA_STRING("LDS Server"),
+        UA_STRING("Anmeldungsserver")
+    };
+    const UA_String expectedLocales[] = {UA_STRING("en"), UA_STRING("de")};
+    // even if we request en_US, the server will return de_DE because it only has that name.
+    FindAndCheck(expectedUris, 2, expectedLocales, expectedNames, NULL, "en");
 
-    }
+}
 END_TEST
 
 // Test if registered server is returned from LDS
 START_TEST(Client_find_registered) {
-        const char* expectedUris[] ={"urn:open62541.test.local_discovery_server", "urn:open62541.test.server_register"};
-        FindAndCheck(expectedUris, 2, NULL, NULL, NULL, NULL);
-    }
+    const UA_String expectedUris[] = {
+        UA_STRING("urn:open62541.test.local_discovery_server"),
+        UA_STRING("urn:open62541.test.server_register")
+    };
+    FindAndCheck(expectedUris, 2, NULL, NULL, NULL, NULL);
+}
 END_TEST
 
 // Test if registered server is returned from LDS using FindServersOnNetwork
 START_TEST(Client_find_on_network_registered) {
-        char *expectedUris[2];
-        char hostname[256];
-
-        ck_assert_uint_eq(gethostname(hostname, 255), 0);
-
-        //DNS limits name to max 63 chars (+ \0)
-        expectedUris[0] = malloc(64);
-        snprintf(expectedUris[0], 64, "LDS_test-%s", hostname);
-        expectedUris[1] = malloc(64);
-        snprintf(expectedUris[1], 64, "Register_test-%s", hostname);
-        FindOnNetworkAndCheck(expectedUris, 2, NULL, NULL, NULL, 0);
-
-
-        // filter by Capabilities
-        const char* capsLDS[] ={"LDS"};
-        const char* capsNA[] ={"NA"};
-        const char* capsMultiple[] ={"LDS", "NA"};
-
-        // only LDS expected
-        FindOnNetworkAndCheck(expectedUris, 1, NULL, NULL, capsLDS, 1);
-        // only register server expected
-        FindOnNetworkAndCheck(&expectedUris[1], 1, NULL, NULL, capsNA, 1);
-        // no server expected
-        FindOnNetworkAndCheck(NULL, 0, NULL, NULL, capsMultiple, 2);
-
-        free(expectedUris[0]);
-        free(expectedUris[1]);
-
-    }
+    char urls[2][64];
+    UA_String expectedUris[2];
+    char hostname[256];
+
+    ck_assert_uint_eq(gethostname(hostname, 255), 0);
+
+    //DNS limits name to max 63 chars (+ \0)
+    snprintf(urls[0], 64, "LDS_test-%s", hostname);
+    snprintf(urls[1], 64, "Register_test-%s", hostname);
+    expectedUris[0] = UA_STRING(urls[0]);
+    expectedUris[1] = UA_STRING(urls[1]);
+    FindOnNetworkAndCheck(expectedUris, 2, NULL, NULL, NULL, 0);
+
+    // filter by Capabilities
+    const char* capsLDS[] = {"LDS"};
+    const char* capsNA[] = {"NA"};
+    const char* capsMultiple[] = {"LDS", "NA"};
+
+    // only LDS expected
+    FindOnNetworkAndCheck(expectedUris, 1, NULL, NULL, capsLDS, 1);
+    // only register server expected
+    FindOnNetworkAndCheck(&expectedUris[1], 1, NULL, NULL, capsNA, 1);
+    // no server expected
+    FindOnNetworkAndCheck(NULL, 0, NULL, NULL, capsMultiple, 2);
+}
 END_TEST
 
 // Test if filtering with uris works
 START_TEST(Client_find_filter) {
-        const char* expectedUris[] ={"urn:open62541.test.server_register"};
-        FindAndCheck(expectedUris, 1,NULL, NULL, "urn:open62541.test.server_register", NULL);
-    }
+    const UA_String expectedUris[] = {
+        UA_STRING("urn:open62541.test.server_register")
+    };
+    FindAndCheck(expectedUris, 1, NULL, NULL, "urn:open62541.test.server_register", NULL);
+}
 END_TEST
 
-
 START_TEST(Client_get_endpoints) {
-        const char* expectedEndpoints[] ={"opc.tcp://localhost:4840"};
-        // general check if expected endpoints are returned
-        GetEndpointsAndCheck("opc.tcp://localhost:4840", NULL,expectedEndpoints, 1);
-        // check if filtering transport profile still returns the endpoint
-        GetEndpointsAndCheck("opc.tcp://localhost:4840", "http://opcfoundation.org/UA-Profile/Transport/uatcp-uasc-uabinary", expectedEndpoints, 1);
-        // filter transport profily by HTTPS, which should return no endpoint
-        GetEndpointsAndCheck("opc.tcp://localhost:4840", "http://opcfoundation.org/UA-Profile/Transport/https-uabinary", NULL, 0);
-    }
+    const UA_String  expectedEndpoints[] ={
+        UA_STRING("opc.tcp://localhost:4840")
+    };
+
+    // general check if expected endpoints are returned
+    GetEndpointsAndCheck("opc.tcp://localhost:4840", NULL,expectedEndpoints, 1);
+
+    // check if filtering transport profile still returns the endpoint
+    GetEndpointsAndCheck("opc.tcp://localhost:4840",
+                         "http://opcfoundation.org/UA-Profile/Transport/uatcp-uasc-uabinary",
+                         expectedEndpoints, 1);
+
+    // filter transport profily by HTTPS, which should return no endpoint
+    GetEndpointsAndCheck("opc.tcp://localhost:4840",
+                         "http://opcfoundation.org/UA-Profile/Transport/https-uabinary", NULL, 0);
+}
 END_TEST
 
 START_TEST(Util_start_lds) {
-        setup_lds();
-    }
+    setup_lds();
+}
 END_TEST
 
 START_TEST(Util_stop_lds) {
-        teardown_lds();
-    }
+    teardown_lds();
+}
 END_TEST
 
 START_TEST(Util_wait_timeout) {
-        // wait until server is removed by timeout. Additionally wait a few seconds more to be sure.
-        UA_sleep(100000 * checkWait);
-        sleep(1);
-    }
+    // wait until server is removed by timeout. Additionally wait a few seconds more to be sure.
+    UA_sleep(100000 * checkWait);
+    sleep(1);
+}
 END_TEST
 
 START_TEST(Util_wait_mdns) {
-        UA_sleep(1000);
-        sleep(1);
-    }
+    UA_sleep(1000);
+    sleep(1);
+}
 END_TEST
 
 START_TEST(Util_wait_startup) {
-        UA_sleep(1000);
-        sleep(1);
-    }
+    UA_sleep(1000);
+    sleep(1);
+}
 END_TEST
 
 START_TEST(Util_wait_retry) {
-        // first retry is after 2 seconds, then 4, so it should be enough to wait 3 seconds
-        UA_sleep(3000);
-        sleep(3);
-    }
+    // first retry is after 2 seconds, then 4, so it should be enough to wait 3 seconds
+    UA_sleep(3000);
+    sleep(3);
+}
 END_TEST
 
 static Suite* testSuite_Client(void) {

+ 20 - 28
tests/check_server_jobs.c

@@ -24,62 +24,54 @@ static void teardown(void) {
 UA_Boolean *executed;
 
 static void
-dummyJob(UA_Server *serverPtr, void *data) {
+dummyCallback(UA_Server *serverPtr, void *data) {
     *executed = true;
 }
 
-START_TEST(Server_addRemoveRepeatedJob) {
+START_TEST(Server_addRemoveRepeatedCallback) {
     executed = UA_Boolean_new();
-    UA_Guid id;
-    UA_Job rj = (UA_Job){
-        .type = UA_JOBTYPE_METHODCALL,
-        .job.methodCall = {.data = NULL, .method = dummyJob}
-    };
-    /* The job is added to the main queue only upon the next run_iterate */
-    UA_Server_addRepeatedJob(server, rj, 10, &id);
-    UA_Server_run_iterate(server, false);
 
-    /* Wait until the job has surely timed out */
+    /* The callback is added to the main queue only upon the next run_iterate */
+    UA_UInt64 id;
+    UA_Server_addRepeatedCallback(server, dummyCallback, NULL, 10, &id);
+
+    /* Wait until the callback has surely timed out */
     UA_sleep(15);
     UA_Server_run_iterate(server, false);
 
-    /* Wait a bit longer until the workers have picked up the dispatched job */
+    /* Wait a bit longer until the workers have picked up the dispatched callback */
     UA_sleep(15);
     ck_assert_uint_eq(*executed, true);
 
-    UA_Server_removeRepeatedJob(server, id);
+    UA_Server_removeRepeatedCallback(server, id);
     UA_Boolean_delete(executed);
 }
 END_TEST
 
-UA_Guid *jobId;
+UA_UInt64 *cbId;
 
 static void
-removeItselfJob(UA_Server *serverPtr, void *data) {
-    UA_Server_removeRepeatedJob(serverPtr, *jobId);
+removeItselfCallback(UA_Server *serverPtr, void *data) {
+    UA_Server_removeRepeatedCallback(serverPtr, *cbId);
 }
 
-START_TEST(Server_repeatedJobRemoveItself) {
-    jobId = UA_Guid_new();
-    UA_Job rj = (UA_Job){
-        .type = UA_JOBTYPE_METHODCALL,
-        .job.methodCall = {.data = NULL, .method = removeItselfJob}
-    };
-    UA_Server_addRepeatedJob(server, rj, 10, jobId);
+START_TEST(Server_repeatedCallbackRemoveItself) {
+    cbId = UA_UInt64_new();
+    UA_Server_addRepeatedCallback(server, removeItselfCallback, NULL, 10, cbId);
 
     UA_sleep(15);
     UA_Server_run_iterate(server, false);
 
-    UA_Guid_delete(jobId);
+    UA_UInt64_delete(cbId);
 }
 END_TEST
 
 static Suite* testSuite_Client(void) {
-    Suite *s = suite_create("Server Jobs");
-    TCase *tc_server = tcase_create("Server Repeated Jobs");
+    Suite *s = suite_create("Server Callbacks");
+    TCase *tc_server = tcase_create("Server Repeated Callbacks");
     tcase_add_checked_fixture(tc_server, setup, teardown);
-    tcase_add_test(tc_server, Server_addRemoveRepeatedJob);
-    tcase_add_test(tc_server, Server_repeatedJobRemoveItself);
+    tcase_add_test(tc_server, Server_addRemoveRepeatedCallback);
+    tcase_add_test(tc_server, Server_repeatedCallbackRemoveItself);
     suite_add_tcase(s, tc_server);
     return s;
 }

+ 0 - 5
tests/check_services_subscriptions.c

@@ -161,19 +161,14 @@ START_TEST(Server_publishCallback) {
     ck_assert(publishingInterval > 0.0f);
     UA_CreateSubscriptionResponse_deleteMembers(&response);
 
-    /* Sleep until the publishing interval times out */
-    UA_sleep((UA_UInt32)publishingInterval + 1);
-
     /* Keepalive is set to max initially */
     UA_Subscription *sub;
     LIST_FOREACH(sub, &adminSession.serverSubscriptions, listEntry)
         ck_assert_uint_eq(sub->currentKeepAliveCount, sub->maxKeepAliveCount);
 
     /* Sleep until the publishing interval times out */
-    UA_Server_run_iterate(server, false);
     UA_sleep((UA_UInt32)publishingInterval + 1);
     UA_Server_run_iterate(server, false);
-    UA_sleep((UA_UInt32)publishingInterval + 1);
 
     LIST_FOREACH(sub, &adminSession.serverSubscriptions, listEntry)
         ck_assert_uint_eq(sub->currentKeepAliveCount, sub->maxKeepAliveCount+1);