always remove sessions and securechannels with a delayed callback

Otherwise, a subscription callbacks that is already scheduled may hold a
pointer to the session / channel.

src/server/ua_securechannel_manager.c

@@ -28,25 +28,42 @@ void UA_SecureChannelManager_deleteMembers(UA_SecureChannelManager *cm) {
     }
 }
 
-static void removeSecureChannel(UA_SecureChannelManager *cm, channel_list_entry *entry){
+static void
+removeSecureChannelCallback(UA_Server *server, void *entry) {
+    channel_list_entry *centry = (channel_list_entry*)entry;
+    UA_SecureChannel_deleteMembersCleanup(&centry->channel);
+    UA_free(entry);
+}
+
+static UA_StatusCode
+removeSecureChannel(UA_SecureChannelManager *cm, channel_list_entry *entry){
+    UA_LOG_INFO_CHANNEL(cm->server->config.logger, &entry->channel,
+                         "SecureChannel has timed out");
+
+    /* Add a delayed callback to remove the channel when the currently
+     * scheduled jobs have completed */
+    UA_StatusCode retval = UA_Server_delayedCallback(cm->server, removeSecureChannelCallback, entry);
+    if(retval != UA_STATUSCODE_GOOD) {
+        UA_LOG_WARNING(cm->server->config.logger, UA_LOGCATEGORY_SESSION,
+                       "Could not remove the secure channel with error code %s",
+                       UA_StatusCode_name(retval));
+        return retval; /* Try again next time */
+    }
+
+    /* Detach the channel and make the capacity available */
     LIST_REMOVE(entry, pointers);
     UA_atomic_add(&cm->currentChannelCount, (UA_UInt32)-1);
-    UA_SecureChannel_deleteMembersCleanup(&entry->channel);
-#ifndef UA_ENABLE_MULTITHREADING
-    UA_free(entry);
-#else
-    UA_Server_delayedFree(cm->server, entry);
-#endif
+    return UA_STATUSCODE_GOOD;
 }
 
 /* remove channels that were not renewed or who have no connection attached */
-void UA_SecureChannelManager_cleanupTimedOut(UA_SecureChannelManager *cm, UA_DateTime nowMonotonic) {
+void
+UA_SecureChannelManager_cleanupTimedOut(UA_SecureChannelManager *cm, UA_DateTime nowMonotonic) {
     channel_list_entry *entry, *temp;
     LIST_FOREACH_SAFE(entry, &cm->channels, pointers, temp) {
         UA_DateTime timeout = entry->channel.securityToken.createdAt +
             (UA_DateTime)(entry->channel.securityToken.revisedLifetime * UA_MSEC_TO_DATETIME);
         if(timeout < nowMonotonic || !entry->channel.connection) {
-            UA_LOG_DEBUG_CHANNEL(cm->server->config.logger, &entry->channel, "SecureChannel has timed out");
             removeSecureChannel(cm, entry);
         } else if(entry->channel.nextSecurityToken.tokenId > 0) {
             UA_SecureChannel_revolveTokens(&entry->channel);
@@ -60,7 +77,8 @@ static UA_Boolean purgeFirstChannelWithoutSession(UA_SecureChannelManager *cm) {
     LIST_FOREACH(entry, &cm->channels, pointers) {
         if(LIST_EMPTY(&(entry->channel.sessions))){
             UA_LOG_DEBUG_CHANNEL(cm->server->config.logger, &entry->channel,
-                                 "Channel was purged since maxSecureChannels was reached and channel had no session attached");
+                                 "Channel was purged since maxSecureChannels was "
+                                 "reached and channel had no session attached");
             removeSecureChannel(cm, entry);
             UA_assert(entry != LIST_FIRST(&cm->channels));
             return true;
@@ -169,6 +187,5 @@ UA_SecureChannelManager_close(UA_SecureChannelManager *cm, UA_UInt32 channelId)
     }
     if(!entry)
         return UA_STATUSCODE_BADINTERNALERROR;
-    removeSecureChannel(cm, entry);
-    return UA_STATUSCODE_GOOD;
+    return removeSecureChannel(cm, entry);
 }

src/server/ua_securechannel_manager.h

@@ -26,9 +26,15 @@ typedef struct UA_SecureChannelManager {
 UA_StatusCode
 UA_SecureChannelManager_init(UA_SecureChannelManager *cm, UA_Server *server);
 
-void UA_SecureChannelManager_deleteMembers(UA_SecureChannelManager *cm);
-
-void UA_SecureChannelManager_cleanupTimedOut(UA_SecureChannelManager *cm, UA_DateTime nowMonotonic);
+/* Remove a all securechannels */
+void
+UA_SecureChannelManager_deleteMembers(UA_SecureChannelManager *cm);
+
+/* Remove timed out securechannels with a delayed callback. So all currently
+ * scheduled jobs with a pointer to a securechannel can finish first. */
+void
+UA_SecureChannelManager_cleanupTimedOut(UA_SecureChannelManager *cm,
+                                        UA_DateTime nowMonotonic);
 
 UA_StatusCode
 UA_SecureChannelManager_open(UA_SecureChannelManager *cm, UA_Connection *conn,

src/server/ua_server_worker.c

@@ -360,6 +360,15 @@ void UA_Server_deleteAllRepeatedJobs(UA_Server *server) {
 /* Delayed Jobs */
 /****************/
 
+static void
+delayed_free(UA_Server *server, void *data) {
+    UA_free(data);
+}
+
+UA_StatusCode UA_Server_delayedFree(UA_Server *server, void *data) {
+    return UA_Server_delayedCallback(server, delayed_free, data);
+}
+
 #ifndef UA_ENABLE_MULTITHREADING
 
 typedef struct UA_DelayedJob {
@@ -411,7 +420,8 @@ static void getCounters(UA_Server *server, struct DelayedJobs *delayed) {
 /* Call from the main thread only. This is the only function that modifies */
 /* server->delayedWork. processDelayedWorkQueue modifies the "next" (after the */
 /* head). */
-static void addDelayedJob(UA_Server *server, UA_Job *job) {
+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 */
@@ -438,15 +448,6 @@ static void addDelayedJob(UA_Server *server, UA_Job *job) {
     ++dj->jobsCount;
 }
 
-static void
-delayed_free(UA_Server *server, void *data) {
-    UA_free(data);
-}
-
-UA_StatusCode UA_Server_delayedFree(UA_Server *server, void *data) {
-    return UA_Server_delayedCallback(server, delayed_free, data);
-}
-
 static void
 addDelayedJobAsync(UA_Server *server, UA_Job *job) {
     addDelayedJob(server, job);

src/server/ua_session_manager.c

@@ -22,27 +22,47 @@ void UA_SessionManager_deleteMembers(UA_SessionManager *sm) {
     }
 }
 
+/* Delayed callback to free the session memory */
 static void
-removeSessionEntry(UA_SessionManager *sm, session_list_entry *sentry) {
+removeSessionCallback(UA_Server *server, void *entry) {
+    session_list_entry *sentry = (session_list_entry*)entry;
+    UA_Session_deleteMembersCleanup(&sentry->session, server);
+    UA_free(sentry);
+}
+
+static UA_StatusCode
+removeSession(UA_SessionManager *sm, session_list_entry *sentry) {
+    /* Deactivate the session */
+    UA_LOG_INFO(sm->server->config.logger, UA_LOGCATEGORY_SESSION,
+                "Session with token %i has timed out and is removed",
+                sentry->session.sessionId.identifier.numeric);
+    sentry->session.activated = false;
+
+    /* Add a delayed callback to remove the session when the currently
+     * scheduled jobs have completed */
+    UA_StatusCode retval = UA_Server_delayedCallback(sm->server, removeSessionCallback, sentry);
+    if(retval != UA_STATUSCODE_GOOD) {
+        UA_LOG_WARNING(sm->server->config.logger, UA_LOGCATEGORY_SESSION,
+                       "Could not remove session with error code %s",
+                       UA_StatusCode_name(retval));
+        return retval; /* Try again next time */
+    }
+
+    /* Detach the session and make the capacity available */
     LIST_REMOVE(sentry, pointers);
     UA_atomic_add(&sm->currentSessionCount, (UA_UInt32)-1);
-    UA_Session_deleteMembersCleanup(&sentry->session, sm->server);
-#ifndef UA_ENABLE_MULTITHREADING
-    UA_free(sentry);
-#else
-    UA_Server_delayedFree(sm->server, sentry);
-#endif
+    return UA_STATUSCODE_GOOD;
 }
 
-void UA_SessionManager_cleanupTimedOut(UA_SessionManager *sm, UA_DateTime nowMonotonic) {
+void
+UA_SessionManager_cleanupTimedOut(UA_SessionManager *sm,
+                                  UA_DateTime nowMonotonic) {
     session_list_entry *sentry, *temp;
     LIST_FOREACH_SAFE(sentry, &sm->sessions, pointers, temp) {
-        if(sentry->session.validTill < nowMonotonic) {
-            UA_LOG_DEBUG(sm->server->config.logger, UA_LOGCATEGORY_SESSION,
-                         "Session with token %i has timed out and is removed",
-                         sentry->session.sessionId.identifier.numeric);
-            removeSessionEntry(sm, sentry);
-        }
+        /* Session has timed out? */
+        if(sentry->session.validTill >= nowMonotonic)
+            continue;
+        removeSession(sm, sentry);
     }
 }
 
@@ -103,6 +123,5 @@ UA_SessionManager_removeSession(UA_SessionManager *sm, const UA_NodeId *token) {
     }
     if(!current)
         return UA_STATUSCODE_BADSESSIONIDINVALID;
-    removeSessionEntry(sm, current);
-    return UA_STATUSCODE_GOOD;
+    return removeSession(sm, current);
 }

src/server/ua_session_manager.h

@@ -24,9 +24,14 @@ typedef struct UA_SessionManager {
 UA_StatusCode
 UA_SessionManager_init(UA_SessionManager *sm, UA_Server *server);
 
+/* Deletes all sessions */
 void UA_SessionManager_deleteMembers(UA_SessionManager *sm);
 
-void UA_SessionManager_cleanupTimedOut(UA_SessionManager *sm, UA_DateTime nowMonotonic);
+/* Deletes all sessions that have timed out. Deletion is implemented via a
+ * delayed callback. So all currently scheduled jobs with a pointer to the
+ * session can complete. */
+void UA_SessionManager_cleanupTimedOut(UA_SessionManager *sm,
+                                       UA_DateTime nowMonotonic);
 
 UA_StatusCode
 UA_SessionManager_createSession(UA_SessionManager *sm, UA_SecureChannel *channel,