From: Florin Coras <fcoras@cisco.com>
Date: Mon, 2 Jun 2025 04:31:22 +0000 (-0400)
Subject: vcl: handle pthread cleanups in vls
X-Git-Url: https://gerrit.fd.io/r/gitweb?a=commitdiff_plain;h=refs%2Fchanges%2F63%2F43063%2F3;p=vpp.git

vcl: handle pthread cleanups in vls

For historic reasons vcl registers for atexit and pthread exit cleanup
callback functions. Since vls is the one handling thread detection, move
thread cleanup logic there as well.

This also avoids potential issues if main/first pthreads exists before
the subsequent spawned ones as it may lead to premature vcl worker
cleanup.

Type: improvement

Change-Id: Id7a5c186b48f1e4c60ced635d918b5d4b4143fa6
Signed-off-by: Florin Coras <fcoras@cisco.com>
---

diff --git a/src/vcl/vcl_locked.c b/src/vcl/vcl_locked.c
index 7ba9fab25fa..aaf47c8f9ef 100644
--- a/src/vcl/vcl_locked.c
+++ b/src/vcl/vcl_locked.c
@@ -156,6 +156,8 @@ typedef struct vls_main_
 
 vls_main_t *vlsm;
 
+static pthread_key_t vls_mt_pthread_stop_key;
+
 typedef enum
 {
   VLS_RPC_STATE_INIT,
@@ -323,6 +325,37 @@ vls_mt_add (void)
   /* Only allow new pthread to be cancled in vls_mt_mq_lock */
   if (vlsl->vls_mt_n_threads >= 2)
     pthread_setcancelstate (PTHREAD_CANCEL_DISABLE, NULL);
+
+  if (pthread_setspecific (vls_mt_pthread_stop_key, vcl_worker_get_current ()))
+    VDBG (0, "failed to setup key value");
+}
+
+static void
+vls_mt_del (void *arg)
+{
+  vcl_worker_t *wrk = (vcl_worker_t *) arg;
+
+  VDBG (0, "vls worker %u vcl worker %u nthreads %u cleaning up pthread",
+	vlsl->vls_wrk_index, vcl_get_worker_index (), vlsl->vls_mt_n_threads);
+
+  if (wrk != vcl_worker_get_current ())
+    {
+      VDBG (0, "vls_mt_del called with wrong worker %u != %u", wrk->wrk_index,
+	    vcl_get_worker_index ());
+      return;
+    }
+
+  vlsl->vls_mt_n_threads -= 1;
+
+  if (vls_mt_wrk_supported ())
+    {
+      vppcom_worker_unregister ();
+    }
+  else
+    {
+      if (!vlsl->vls_mt_n_threads)
+	vppcom_worker_unregister ();
+    }
 }
 
 static inline void
@@ -2138,6 +2171,12 @@ vls_app_create (char *app_name)
 {
   int rv;
 
+  if (pthread_key_create (&vls_mt_pthread_stop_key, vls_mt_del))
+    {
+      VDBG (0, "failed to add pthread cleanup function");
+      return -1;
+    }
+
   if ((rv = vppcom_app_create (app_name)))
     return rv;
 
@@ -2153,6 +2192,8 @@ vls_app_create (char *app_name)
   vls_worker_alloc ();
   vlsl->vls_wrk_index = vcl_get_worker_index ();
   vlsl->vls_mt_n_threads = 1;
+  if (pthread_setspecific (vls_mt_pthread_stop_key, vcl_worker_get_current ()))
+    VDBG (0, "failed to setup key value");
   clib_rwlock_init (&vlsl->vls_pool_lock);
   vls_mt_locks_init ();
   vcm->wrk_rpc_fn = vls_rpc_handler;
diff --git a/src/vcl/vcl_private.c b/src/vcl/vcl_private.c
index 33ce7eb6c7a..ea82c26803c 100644
--- a/src/vcl/vcl_private.c
+++ b/src/vcl/vcl_private.c
@@ -15,8 +15,6 @@
 
 #include <vcl/vcl_private.h>
 
-static pthread_key_t vcl_worker_stop_key;
-
 vcl_mq_evt_conn_t *
 vcl_mq_evt_conn_alloc (vcl_worker_t * wrk)
 {
@@ -172,22 +170,6 @@ vcl_worker_cleanup (vcl_worker_t * wrk, u8 notify_vpp)
   clib_spinlock_unlock (&vcm->workers_lock);
 }
 
-static void
-vcl_worker_cleanup_cb (void *arg)
-{
-  vcl_worker_t *wrk;
-  u32 wrk_index;
-
-  wrk_index = vcl_get_worker_index ();
-  wrk = vcl_worker_get_if_valid (wrk_index);
-  if (!wrk)
-    return;
-
-  vcl_worker_cleanup (wrk, 1 /* notify vpp */ );
-  vcl_set_worker_index (~0);
-  VDBG (0, "cleaned up worker %u", wrk_index);
-}
-
 void
 vcl_worker_detached_start_signal_mq (vcl_worker_t *wrk)
 {
@@ -379,10 +361,6 @@ vcl_worker_register_with_vpp (void)
       clib_spinlock_unlock (&vcm->workers_lock);
       return -1;
     }
-  if (pthread_key_create (&vcl_worker_stop_key, vcl_worker_cleanup_cb))
-    VDBG (0, "failed to add pthread cleanup function");
-  if (pthread_setspecific (vcl_worker_stop_key, &wrk->thread_id))
-    VDBG (0, "failed to setup key value");
 
   clib_spinlock_unlock (&vcm->workers_lock);