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.

mysql-test/valgrind.supp

@@ -19,36 +19,6 @@
 # Suppress some common (not fatal) errors in system libraries found by valgrind
 #
 
-#
-# Pthread doesn't free all thread specific memory before program exists
-#
-{
-   pthread allocate_tls memory loss
-   Memcheck:Leak
-   fun:calloc
-   fun:_dl_allocate_tls
-   fun:allocate_stack
-   fun:pthread_create*
-}
-
-{
-   pthread allocate_tls memory loss
-   Memcheck:Leak
-   fun:calloc
-   fun:_dl_allocate_tls
-   fun:pthread_create*
-
-}
-
-{
-   pthread allocate_tls memory loss
-   Memcheck:Leak
-   fun:calloc
-   obj:/lib*/ld*.so
-   fun:_dl_allocate_tls
-   fun:pthread_create*
-}
-
 {
    pthead_exit memory loss 1
    Memcheck:Leak
@@ -89,34 +59,6 @@
    fun:_dl_map_object_from_fd
 }
 
-{
-   pthread allocate_dtv memory loss
-   Memcheck:Leak
-   fun:calloc
-   fun:allocate_dtv
-   fun:_dl_allocate_tls_storage
-   fun:__GI__dl_allocate_tls
-   fun:pthread_create
-}
-
-{
-   pthread allocate_dtv memory loss second
-   Memcheck:Leak
-   fun:calloc
-   fun:allocate_dtv
-   fun:_dl_allocate_tls
-   fun:pthread_create*
-}
-
-{
-   pthread memalign memory loss
-   Memcheck:Leak
-   fun:memalign
-   fun:_dl_allocate_tls_storage
-   fun:__GI__dl_allocate_tls
-   fun:pthread_create
-}
-
 {
    pthread pthread_key_create
    Memcheck:Leak
@@ -972,18 +914,6 @@
    fun:nptl_pthread_exit_hack_handler
 }
 
-#
-# Pthread doesn't free all thread specific memory before program exists
-#
-{
-   pthread allocate_tls memory loss in 2.6.1.
-   Memcheck:Leak 
-   fun:calloc
-   obj:*/ld-*.so
-   fun:_dl_allocate_tls
-   fun:pthread_create*
-}
-
 {
    memory "leak" in backtrace() of glibc 2.9 (not present in 2.13)
    Memcheck:Leak

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 @@ static int really_execute_checkpoint(void)
 
 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;
 }

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);
 }
 

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,19 +60,16 @@ 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 */
+    control->killed= TRUE; /* kill it */
     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);
+    mysql_mutex_unlock(control->LOCK_control);
+    DBUG_PRINT("info", ("waiting for Maria background thread to die"));
+    pthread_join(control->thread, NULL);
   }
+  else
     mysql_mutex_unlock(control->LOCK_control);
   mysql_mutex_destroy(control->LOCK_control);
   mysql_cond_destroy(control->COND_control);
@@ -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;
-}

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);