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