Merge pull request #82 from ADTRAN/call-home-client-id

Fix call home client thread cleanup

Author: michalvasko

CMakeLists.txt

@@ -24,7 +24,7 @@ if(NOT CMAKE_BUILD_TYPE)
     set(CMAKE_BUILD_TYPE debug)
 endif()
 
-set(CMAKE_C_FLAGS         "${CMAKE_C_FLAGS} -Wall -Wextra -fvisibility=hidden")
+set(CMAKE_C_FLAGS         "${CMAKE_C_FLAGS} -Wall -Wextra -fvisibility=hidden -std=gnu11")
 set(CMAKE_C_FLAGS_RELEASE "-O2 -DNDEBUG")
 set(CMAKE_C_FLAGS_PACKAGE "-g -O2 -DNDEBUG")
 set(CMAKE_C_FLAGS_DEBUG   "-g -O0")

src/session_p.h

@@ -18,6 +18,7 @@
 
 #include <stdint.h>
 #include <pthread.h>
+#include <stdatomic.h>
 
 #include <libyang/libyang.h>
 
@@ -268,14 +269,15 @@ struct nc_server_opts {
         } conn;
         NC_CH_START_WITH start_with;
         uint8_t max_attempts;
+        uint32_t id;
         pthread_mutex_t lock;
     } *ch_clients;
     uint16_t ch_client_count;
     pthread_rwlock_t ch_client_lock;
 
-    /* ACCESS locked with sid_lock */
-    uint32_t new_session_id;
-    pthread_spinlock_t sid_lock;
+    /* Atomic IDs */
+    atomic_uint_fast32_t new_session_id;
+    atomic_uint_fast32_t new_client_id;
 };
 
 /**

src/session_server.c

@@ -515,7 +515,7 @@ nc_server_init(struct ly_ctx *ctx)
     server_opts.ctx = ctx;
 
     server_opts.new_session_id = 1;
-    pthread_spin_init(&server_opts.sid_lock, PTHREAD_PROCESS_PRIVATE);
+    server_opts.new_client_id = 1;
 
     errno=0;
 
@@ -549,8 +549,6 @@ nc_server_destroy(void)
     server_opts.capabilities = NULL;
     server_opts.capabilities_count = 0;
 
-    pthread_spin_destroy(&server_opts.sid_lock);
-
 #if defined(NC_ENABLED_SSH) || defined(NC_ENABLED_TLS)
     nc_server_del_endpt(NULL, 0);
 #endif
@@ -703,9 +701,7 @@ nc_accept_inout(int fdin, int fdout, const char *username, struct nc_session **s
     (*session)->ctx = server_opts.ctx;
 
     /* assign new SID atomically */
-    pthread_spin_lock(&server_opts.sid_lock);
-    (*session)->id = server_opts.new_session_id++;
-    pthread_spin_unlock(&server_opts.sid_lock);
+    (*session)->id = atomic_fetch_add(&server_opts.new_session_id, 1);
 
     /* NETCONF handshake */
     msgtype = nc_handshake_io(*session);
@@ -2015,11 +2011,7 @@ nc_accept(int timeout, struct nc_session **session)
     pthread_rwlock_unlock(&server_opts.endpt_lock);
 
     /* assign new SID atomically */
-    /* LOCK */
-    pthread_spin_lock(&server_opts.sid_lock);
-    (*session)->id = server_opts.new_session_id++;
-    /* UNLOCK */
-    pthread_spin_unlock(&server_opts.sid_lock);
+    (*session)->id = atomic_fetch_add(&server_opts.new_session_id, 1);
 
     /* NETCONF handshake */
     msgtype = nc_handshake_io(*session);
@@ -2081,6 +2073,7 @@ nc_server_ch_add_client(const char *name, NC_TRANSPORT_IMPL ti)
         return -1;
     }
     server_opts.ch_clients[server_opts.ch_client_count - 1].name = lydict_insert(server_opts.ctx, name, 0);
+    server_opts.ch_clients[server_opts.ch_client_count - 1].id = atomic_fetch_add(&server_opts.new_client_id, 1);
     server_opts.ch_clients[server_opts.ch_client_count - 1].ti = ti;
     server_opts.ch_clients[server_opts.ch_client_count - 1].ch_endpts = NULL;
     server_opts.ch_clients[server_opts.ch_client_count - 1].ch_endpt_count = 0;
@@ -2758,11 +2751,7 @@ nc_connect_ch_client_endpt(struct nc_ch_client *client, struct nc_ch_endpt *endp
     }
 
     /* assign new SID atomically */
-    /* LOCK */
-    pthread_spin_lock(&server_opts.sid_lock);
-    (*session)->id = server_opts.new_session_id++;
-    /* UNLOCK */
-    pthread_spin_unlock(&server_opts.sid_lock);
+    (*session)->id = atomic_fetch_add(&server_opts.new_session_id, 1);
 
     /* NETCONF handshake */
     msgtype = nc_handshake_io(*session);
@@ -2909,12 +2898,14 @@ nc_ch_client_thread(void *arg)
     struct nc_ch_endpt *cur_endpt;
     struct nc_session *session;
     struct nc_ch_client *client;
+    uint32_t client_id;
 
     /* LOCK */
     client = nc_server_ch_client_with_endpt_lock(data->client_name);
     if (!client) {
         goto cleanup;
     }
+    client_id = client->id;
 
     cur_endpt = &client->ch_endpts[0];
     cur_endpt_name = strdup(cur_endpt->name);
@@ -2938,6 +2929,10 @@ nc_ch_client_thread(void *arg)
             if (!client) {
                 goto cleanup;
             }
+            if (client->id != client_id) {
+                nc_server_ch_client_unlock(client);
+                goto cleanup;
+            }
 
             /* session changed status -> it was disconnected for whatever reason,
              * persistent connection immediately tries to reconnect, periodic waits some first */
@@ -2953,6 +2948,10 @@ nc_ch_client_thread(void *arg)
                 if (!client) {
                     goto cleanup;
                 }
+                if (client->id != client_id) {
+                    nc_server_ch_client_unlock(client);
+                    goto cleanup;
+                }
             }
 
             /* set next endpoint to try */
@@ -2983,6 +2982,10 @@ nc_ch_client_thread(void *arg)
             if (!client) {
                 goto cleanup;
             }
+            if (client->id != client_id) {
+                nc_server_ch_client_unlock(client);
+                goto cleanup;
+            }
 
             ++cur_attempts;
 

src/session_server_ssh.c

@@ -1555,9 +1555,7 @@ nc_session_accept_ssh_channel(struct nc_session *orig_session, struct nc_session
     }
 
     /* assign new SID atomically */
-    pthread_spin_lock(&server_opts.sid_lock);
-    new_session->id = server_opts.new_session_id++;
-    pthread_spin_unlock(&server_opts.sid_lock);
+    new_session->id = atomic_fetch_add(&server_opts.new_session_id, 1);
 
     /* NETCONF handshake */
     msgtype = nc_handshake_io(new_session);
@@ -1628,9 +1626,7 @@ nc_ps_accept_ssh_channel(struct nc_pollsession *ps, struct nc_session **session)
     }
 
     /* assign new SID atomically */
-    pthread_spin_lock(&server_opts.sid_lock);
-    new_session->id = server_opts.new_session_id++;
-    pthread_spin_unlock(&server_opts.sid_lock);
+    new_session->id = atomic_fetch_add(&server_opts.new_session_id, 1);
 
     /* NETCONF handshake */
     msgtype = nc_handshake_io(new_session);