Bug #66740 Fix the mysql_thread_end cleanup bug in libmysql
Submitted: 8 Sep 2012 6:53 Modified: 9 Jan 2015 16:23
Reporter: Stephane Carrez Email Updates:
Status: Unsupported Impact on me:
None 
Category:MySQL Server: C API (client library) Severity:S3 (Non-critical)
Version:5.1, 5.5 OS:Any (All POSIX systems)
Assigned to: CPU Architecture:Any
Tags: Contribution, mysql_thread_end

[8 Sep 2012 6:53] Stephane Carrez
Description:
There is a long standing bug in libmysql for multi-threaded applications:

it does not release the thread-specific data storage allocated by my_thread_init.

MySQL shooted on this for years arguing it is the developers responsibility
to call 'mysql_thread_end'.  This makes the developers' life difficult and in
some cases this is just not possible.

See https://bugzilla.redhat.com/show_bug.cgi?id=846602
Bug 846602 - MySQL C API missing THR_KEY_mysys
as an example of nasty workarounds that developers had to implement.

The root cause of the issue is a miss-use of the POSIX 1003.1c standard API.
Indeed, the thread data storage can be reclaimed automatically by adding
a cleanup handler in the thread data key creation.  This cleanup handler
will be called before the thread exits and it can do the necessary house keeping
to cleanup the MySQL library.  This is pure C and again POSIX 1003.1c standard.
(See http://linux.die.net/man/3/pthread_key_create)

It is not acceptable to ask developers to call 'mysql_thread_end' before a thread stops.  The MySQL per-thread data storage MUST be reclaimed automatically (See fix below).

How to repeat:
- Compile the following short multi-threaded client
  gcc -o thr_bug thr_bug.c -lpthread -lmysqlclient

- Run it through valgrind to see the memory leaks produced by my_thread_init

==32450== 12,672 bytes in 99 blocks are definitely lost in loss record 4 of 6
==32450==    at 0x402A5E6: calloc (in /usr/lib/valgrind/vgpreload_memcheck-x86-linux.so)
==32450==    by 0x40B07D3: my_thread_init (in /usr/lib/i386-linux-gnu/libmysqlclient.so.18.0.0)
==32450==    by 0x40887D4: mysql_server_init (in /usr/lib/i386-linux-gnu/libmysqlclient.so.18.0.0)
==32450==    by 0x408F268: mysql_init (in /usr/lib/i386-linux-gnu/libmysqlclient.so.18.0.0)
==32450==    by 0x8048657: test_thread (in /ext/databases/mysql-connector-c-6.0.2/thr_bug)
==32450==    by 0x4058D4B: start_thread (pthread_create.c:308)
==32450==    by 0x4470ACD: clone (clone.S:130)

- Apply the fix proposed below
- Rebuild the below test and run it through valgrind again.
  Memory leaks are fixed.

#include <pthread.h>
#include <stdio.h>
#include <mysql/mysql.h>

const char* host = "localhost";
const char* user = "root";
const char* password = "";
const char* database = "test";
int tcp_port = 3306;

void* test_thread(void* arg)
{
  MYSQL *mysql;

  mysql = mysql_init(NULL);
  if (mysql_real_connect(mysql, host, user, password, database, tcp_port, 0, 0))
  {
    printf("Thread %d connected\n", (int) arg);
    
    mysql_close(mysql);
  }
  /* mysql_thread_end is not called (some applications cannot do that) */
  return 0;
}

int main(int argc, char** argv)
{
  int i;
  pthread_t tids[100];
  
  for (i = 0; i < 100; i++) {
    pthread_t tid;
    
    if (pthread_create(&tids[i], NULL, test_thread, (void*) i) != 0) {
      perror("pthread_create");
    }
  }
  for (i = 0; i < 100; i++) {
    void* ret;
    
    pthread_join(tids[i], &ret);
  }
  return 0;
}

Suggested fix:
I've fixed the MySQL connector C library to use the POSIX 1003.1c thread key cleanup handler.

- the fix is POSIX 1003.1c compliant,
- the fix is pure C,
- the fix is backward compatible with application that called mysql_thread_end
- the fix solves the memory leaks introduced by my_thread_init

http://blog.vacs.fr/index.php?post/2012/09/07/Fixing-the-mysql_thread_end-
cleanup-bug-in-libmysql

--- mysql-connector-c-6.0.2/mysys/my_thr_init.c.orig    2009-08-07 14:31:12.000000000 +0200
+++ mysql-connector-c-6.0.2/mysys/my_thr_init.c 2012-09-07 21:37:39.840742330 +0200
@@ -47,6 +47,7 @@ pthread_mutexattr_t my_fast_mutexattr;
 #ifdef PTHREAD_ERRORCHECK_MUTEX_INITIALIZER_NP
 pthread_mutexattr_t my_errorcheck_mutexattr;
 #endif
+static void thread_cleanup(void* data);
 #ifdef _MSC_VER
 static void install_sigabrt_handler();
 #endif
@@ -118,7 +119,7 @@ my_bool my_thread_global_init(void)
   int pth_ret;
   thd_lib_detected= get_thread_lib();
 
-  if ((pth_ret= pthread_key_create(&THR_KEY_mysys, NULL)) != 0)
+  if ((pth_ret= pthread_key_create(&THR_KEY_mysys, thread_cleanup)) != 0)
   {
     fprintf(stderr,"Can't initialize threads: error %d\n", pth_ret);
     return 1;
@@ -256,9 +257,9 @@ void my_thread_global_end(void)
 
 void my_thread_destroy_mutex(void)
 {
+#ifdef SAFE_MUTEX
   struct st_my_thread_var *tmp;
   tmp= my_pthread_getspecific(struct st_my_thread_var*,THR_KEY_mysys);
-#ifdef SAFE_MUTEX
   if (tmp)
   {
     safe_mutex_free_deadlock_data(&tmp->mutex);
@@ -377,6 +378,20 @@ void my_thread_end(void)
   fprintf(stderr,"my_thread_end(): tmp: 0x%lx  pthread_self: 0x%lx  thread_id: %ld\n",
          (long) tmp, (long) pthread_self(), tmp ? (long) tmp->id : 0L);
 #endif
+  pthread_setspecific(THR_KEY_mysys,0);
+
+  /* Cleanup using the Posix 1003.1 standard mechanism with pthread_key_create */
+  thread_cleanup(tmp);
+}
+
+/*
+  Do the real thread memory cleanup.  This is called explictly by
+  my_thread_end() or by the Posix 1003.1 thread cleanup mechanism.
+*/
+static void thread_cleanup(void* data)
+{
+  struct st_my_thread_var *tmp = (struct st_my_thread_var*) data;
+
   if (tmp && tmp->init)
   {
 
@@ -404,7 +419,7 @@ void my_thread_end(void)
     bfill(tmp, sizeof(tmp), 0x8F);
 #endif
     free(tmp);
-    pthread_setspecific(THR_KEY_mysys,0);
+
     /*
       Decrement counter for number of running threads. We are using this
       in my_thread_global_end() to wait until all threads have called
@@ -415,11 +430,7 @@ void my_thread_end(void)
     DBUG_ASSERT(THR_thread_count != 0);
     if (--THR_thread_count == 0)
       pthread_cond_signal(&THR_COND_threads);
-   pthread_mutex_unlock(&THR_LOCK_threads);
-  }
-  else
-  {
-    pthread_setspecific(THR_KEY_mysys,0);
+    pthread_mutex_unlock(&THR_LOCK_threads);
   }
 }
[11 Sep 2012 22:13] Sveta Smirnova
Thank you for the report.

Verified as described. To notice leaks one need to run valgrind with option --leak-check=full
[8 Apr 2014 10:09] Rafal Somla
Posted by developer:
 
The relevant ODBC bug is bug#18534035.
[10 Jan 2015 13:35] Hartmut Holzgraefe
Can some explain *why* this is unsupported?

(there may be some extra info in bug#18534035 that isn't publicly accessible?)
[26 May 2015 15:47] Clark Dunson
Yes, biting us too.  In particular using mYm mySql connector v1.36 and Matlab R2013b.

We can use this in a production environment, because closing the mySql session does not cleanup the thread, and we get the dread:

Can't initialize threads: error 11

Will attempt to fix it in mYm, and post over there.  I'll come back and give results.