Bug #11014 Memory leak in threaded applications

• Submitted: 1 Jun 2005 10:59
• Modified: 2 Jun 2005 13:46
• Reporter: Shane Kerr
• Status: Not a Bug
• Category: MySQL Server
• Severity: S3 (Non-critical)
• Version: 4.0.24
• OS: Linux (Linux 2.6 with NPTL)
• CPU Architecture: Any

[1 Jun 2005 10:59] Shane Kerr

Description:
Each thread that invokes mysql_init() leaks memory. The memory is fixed per thread. This is especially bad for servers that create many threads, each of which connects to the MySQL server.

This has been verified on both Debian and Gentoo distributions.

How to repeat:
If you run this program under valgrind, you will see the memory leak. You can also change it to use pthread_create() and pthread_join() in a loop, and wait for your computer to run out of memory. :)

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

void*
foo (void *dummy)
{
        MYSQL *m;

        m = mysql_init(NULL);
        if (m == NULL) {
                fprintf(stderr, "Error in mysql_init()\n");
                exit(1);
        }
        mysql_close(m);
        return NULL;
}

int
main ()
{
        pthread_t tid;
        int ret;

        ret = pthread_create(&tid, NULL, foo, NULL);
        if (ret != 0) {
                fprintf(stderr, "pthread_create() error\n");
                exit(1);
        }
        sleep(1);
        return 0;
}

Suggested fix:
The memory is actually lost in the thread-specific data, which is never freed. Fortunately there is a mechanism in POSIX threads to handle just such a case. When pthread_key_create() is invoked, it can include a clean-up function.

The following patch moves the cleanup information out of the my_thread_end() function and includes it in the thread-specific data clean-up function. It seems to be safe, although perhaps pthread_setthreadspecific() needs to be invoked in the cleanup function. Anyway, my_thread_end() is never called on Linux.

diff -Naur mysql-4.0.24/mysys/my_thr_init.c mysql-4.0.24-noleak/mysys/my_thr_init.c
--- mysql-4.0.24/mysys/my_thr_init.c    2005-03-05 01:38:16.000000000 +0100
+++ mysql-4.0.24-noleak/mysys/my_thr_init.c     2005-05-31 19:25:10.000000000 +0200
@@ -44,6 +44,8 @@
 pthread_mutexattr_t my_errchk_mutexattr;
 #endif

+void my_thread_var_free(struct st_my_thread_var *tmp);
+
 /*
   initialize thread environment

@@ -57,7 +59,7 @@

 my_bool my_thread_global_init(void)
 {
-  if (pthread_key_create(&THR_KEY_mysys,0))
+  if (pthread_key_create(&THR_KEY_mysys,my_thread_var_free))
   {
     fprintf(stderr,"Can't initialize threads: error %d\n",errno);
     return 1;
@@ -185,16 +187,8 @@
   return error;
 }

-
-void my_thread_end(void)
+void my_thread_var_free(struct st_my_thread_var *tmp)
 {
-  struct st_my_thread_var *tmp;
-  tmp= my_pthread_getspecific(struct st_my_thread_var*,THR_KEY_mysys);
-
-#ifdef EXTRA_DEBUG_THREADS
-  fprintf(stderr,"my_thread_end(): tmp=%p,thread_id=%ld\n",
-         tmp,pthread_self());
-#endif
   if (tmp && tmp->init)
   {
 #if !defined(DBUG_OFF)
@@ -215,6 +209,22 @@
     tmp->init= 0;
 #endif
   }
+}
+
+
+void my_thread_end(void)
+{
+  struct st_my_thread_var *tmp;
+  tmp= my_pthread_getspecific(struct st_my_thread_var*,THR_KEY_mysys);
+printf("%s:%d\n", __FILE__, __LINE__);
+
+#ifdef EXTRA_DEBUG_THREADS
+  fprintf(stderr,"my_thread_end(): tmp=%p,thread_id=%ld\n",
+         tmp,pthread_self());
+#endif
+
+  my_thread_var_free(tmp);
+
   /* The following free has to be done, even if my_thread_var() is 0 */
 #if (!defined(__WIN__) && !defined(OS2)) || defined(USE_TLS)
   pthread_setspecific(THR_KEY_mysys,0);

[1 Jun 2005 15:44] Aleksey Kishkin

Hi. According to documentation mysql_init must be called once (at start the whole program).
 In the beginning of each thread you need to call mysql_thread_init() , and to call mysql_thread_end().

Could you check if memory leak still present if you invoke functions in this order?

[1 Jun 2005 16:16] Shane Kerr

Well smack my ass and call me Sally.

Yes, adding mysql_thread_end() certainly fixes the problem. :)

Strangely, this is code that we've been running for 4 years on Solaris and never had any memory leak.

Anyway, I would suggest that the submitted patch should still be adopted, as it removes the need for explicit clean-up. It is a "safe" change, that works identically with or without my_thread_end(). Probably best to leave out the debugging printf() though.