MDEV-9994 - Aria service threads are not "joined"

Aria service threads are created "joinable", but they're not "joined" on
completion. This causes memory leaks around thread local storage.

Fixed by joining service thread. Simplified relevant code and cleaned up
relevant valgrind suppressions.

4 files changed, 26 insertions, 61 deletions

diff --git a/storage/maria/ma_checkpoint.c b/storage/maria/ma_checkpoint.c
index 1e8adb64981..f01af164aa6 100644
--- a/storage/maria/ma_checkpoint.c
+++ b/storage/maria/ma_checkpoint.c
@@ -46,7 +46,7 @@ static mysql_mutex_t LOCK_checkpoint;
 static mysql_cond_t  COND_checkpoint;
 /** @brief control structure for checkpoint background thread */
 static MA_SERVICE_THREAD_CONTROL checkpoint_control=
-  {THREAD_DEAD, FALSE, &LOCK_checkpoint, &COND_checkpoint};
+  {0, FALSE, FALSE, &LOCK_checkpoint, &COND_checkpoint};
 /* is ulong like pagecache->blocks_changed */
 static ulong pages_to_flush_before_next_checkpoint;
 static PAGECACHE_FILE *dfiles, /**< data files to flush in background */
@@ -326,7 +326,6 @@ end:
 
 int ma_checkpoint_init(ulong interval)
 {
-  pthread_t th;
   int res= 0;
   DBUG_ENTER("ma_checkpoint_init");
   if (ma_service_thread_control_init(&checkpoint_control))
@@ -334,14 +333,14 @@ int ma_checkpoint_init(ulong interval)
   else if (interval > 0)
   {
     compile_time_assert(sizeof(void *) >= sizeof(ulong));
-    if (!(res= mysql_thread_create(key_thread_checkpoint,
-                                   &th, NULL, ma_checkpoint_background,
-                                   (void *)interval)))
-    {
-      /* thread lives, will have to be killed */
-      checkpoint_control.status= THREAD_RUNNING;
-    }
+    if ((res= mysql_thread_create(key_thread_checkpoint,
+                                  &checkpoint_control.thread, NULL,
+                                  ma_checkpoint_background,
+                                  (void*) interval)))
+      checkpoint_control.killed= TRUE;
   }
+  else
+    checkpoint_control.killed= TRUE;
   DBUG_RETURN(res);
 }
 
@@ -717,7 +716,6 @@ pthread_handler_t ma_checkpoint_background(void *arg)
     DBUG_EXECUTE_IF("maria_checkpoint_indirect", level= CHECKPOINT_INDIRECT;);
     ma_checkpoint_execute(level, FALSE);
   }
-  my_service_thread_signal_end(&checkpoint_control);
   my_thread_end();
   return 0;
 }
diff --git a/storage/maria/ma_loghandler.c b/storage/maria/ma_loghandler.c
index 54f67eb1ab3..bf2205f5a4c 100644
--- a/storage/maria/ma_loghandler.c
+++ b/storage/maria/ma_loghandler.c
@@ -54,7 +54,7 @@ static mysql_mutex_t LOCK_soft_sync;
 static mysql_cond_t  COND_soft_sync;
 /** @brief control structure for checkpoint background thread */
 static MA_SERVICE_THREAD_CONTROL soft_sync_control=
-  {THREAD_DEAD, FALSE, &LOCK_soft_sync, &COND_soft_sync};
+  {0, FALSE, FALSE, &LOCK_soft_sync, &COND_soft_sync};
 
 
 /* transaction log file descriptor */
@@ -8819,7 +8819,6 @@ ma_soft_sync_background( void *arg __attribute__((unused)))
       if (my_service_thread_sleep(&soft_sync_control, sleep))
         break;
     }
-    my_service_thread_signal_end(&soft_sync_control);
     my_thread_end();
     DBUG_RETURN(0);
   }
@@ -8832,7 +8831,6 @@ ma_soft_sync_background( void *arg __attribute__((unused)))
 
 int translog_soft_sync_start(void)
 {
-  pthread_t th;
   int res= 0;
   uint32 min, max;
   DBUG_ENTER("translog_soft_sync_start");
@@ -8847,9 +8845,10 @@ int translog_soft_sync_start(void)
   soft_need_sync= 1;
 
   if (!(res= ma_service_thread_control_init(&soft_sync_control)))
-    if (!(res= mysql_thread_create(key_thread_soft_sync,
-                                   &th, NULL, ma_soft_sync_background, NULL)))
-      soft_sync_control.status= THREAD_RUNNING;
+    if ((res= mysql_thread_create(key_thread_soft_sync,
+                                  &soft_sync_control.thread, NULL,
+                                  ma_soft_sync_background, NULL)))
+      soft_sync_control.killed= TRUE;
   DBUG_RETURN(res);
 }
 
diff --git a/storage/maria/ma_servicethread.c b/storage/maria/ma_servicethread.c
index e5c949a7571..d92c5315933 100644
--- a/storage/maria/ma_servicethread.c
+++ b/storage/maria/ma_servicethread.c
@@ -33,7 +33,7 @@ int ma_service_thread_control_init(MA_SERVICE_THREAD_CONTROL *control)
   DBUG_ENTER("ma_service_thread_control_init");
   DBUG_PRINT("init", ("control 0x%lx", (ulong) control));
   control->inited= TRUE;
-  control->status= THREAD_DEAD; /* not yet born == dead */
+  control->killed= FALSE;
   res= (mysql_mutex_init(key_SERVICE_THREAD_CONTROL_lock,
                          control->LOCK_control, MY_MUTEX_INIT_SLOW) ||
         mysql_cond_init(key_SERVICE_THREAD_CONTROL_cond,
@@ -60,20 +60,17 @@ void ma_service_thread_control_end(MA_SERVICE_THREAD_CONTROL *control)
   DBUG_PRINT("init", ("control 0x%lx", (ulong) control));
   DBUG_ASSERT(control->inited);
   mysql_mutex_lock(control->LOCK_control);
-  if (control->status != THREAD_DEAD) /* thread was started OK */
+  if (!control->killed)
   {
     DBUG_PRINT("info",("killing Maria background thread"));
-    control->status= THREAD_DYING; /* kill it */
-    do /* and wait for it to be dead */
-    {
-      /* wake it up if it was in a sleep */
-      mysql_cond_broadcast(control->COND_control);
-      DBUG_PRINT("info",("waiting for Maria background thread to die"));
-      mysql_cond_wait(control->COND_control, control->LOCK_control);
-    }
-    while (control->status != THREAD_DEAD);
+    control->killed= TRUE; /* kill it */
+    mysql_cond_broadcast(control->COND_control);
+    mysql_mutex_unlock(control->LOCK_control);
+    DBUG_PRINT("info", ("waiting for Maria background thread to die"));
+    pthread_join(control->thread, NULL);
   }
-  mysql_mutex_unlock(control->LOCK_control);
+  else
+    mysql_mutex_unlock(control->LOCK_control);
   mysql_mutex_destroy(control->LOCK_control);
   mysql_cond_destroy(control->COND_control);
   control->inited= FALSE;
@@ -100,7 +97,7 @@ my_bool my_service_thread_sleep(MA_SERVICE_THREAD_CONTROL *control,
   DBUG_ENTER("my_service_thread_sleep");
   DBUG_PRINT("init", ("control 0x%lx", (ulong) control));
   mysql_mutex_lock(control->LOCK_control);
-  if (control->status == THREAD_DYING)
+  if (control->killed)
   {
     mysql_mutex_unlock(control->LOCK_control);
     DBUG_RETURN(TRUE);
@@ -119,34 +116,8 @@ my_bool my_service_thread_sleep(MA_SERVICE_THREAD_CONTROL *control,
                            control->LOCK_control, &abstime);
   }
 #endif
-  if (control->status == THREAD_DYING)
+  if (control->killed)
     res= TRUE;
   mysql_mutex_unlock(control->LOCK_control);
   DBUG_RETURN(res);
 }
-
-
-/**
-  inform about thread exiting
-
-  @param control        control block
-*/
-
-void my_service_thread_signal_end(MA_SERVICE_THREAD_CONTROL *control)
-{
-  DBUG_ENTER("my_service_thread_signal_end");
-  DBUG_PRINT("init", ("control 0x%lx", (ulong) control));
-  mysql_mutex_lock(control->LOCK_control);
-  control->status = THREAD_DEAD; /* indicate that we are dead */
-  /*
-    wake up ma_service_thread_control_end which may be waiting for
-    our death
-  */
-  mysql_cond_broadcast(control->COND_control);
-  /*
-    broadcast was inside unlock because ma_service_thread_control_end
-    destroys mutex
-  */
-  mysql_mutex_unlock(control->LOCK_control);
-  DBUG_VOID_RETURN;
-}
diff --git a/storage/maria/ma_servicethread.h b/storage/maria/ma_servicethread.h
index ed578d93c24..254225bd608 100644
--- a/storage/maria/ma_servicethread.h
+++ b/storage/maria/ma_servicethread.h
@@ -16,12 +16,10 @@
 
 #include <my_pthread.h>
 
-enum ma_service_thread_state {THREAD_RUNNING, THREAD_DYING, THREAD_DEAD};
-
 typedef struct st_ma_service_thread_control
 {
-  /** 'kill' flag for the background thread */
-  enum ma_service_thread_state status;
+  pthread_t thread;
+  my_bool killed;
   /** if thread module was inited or not */
   my_bool inited;
   /** for killing the background thread */
@@ -35,4 +33,3 @@ int ma_service_thread_control_init(MA_SERVICE_THREAD_CONTROL *control);
 void ma_service_thread_control_end(MA_SERVICE_THREAD_CONTROL *control);
 my_bool my_service_thread_sleep(MA_SERVICE_THREAD_CONTROL *control,
                                 ulonglong sleep_time);
-void my_service_thread_signal_end(MA_SERVICE_THREAD_CONTROL *control);