Memory leak in MySQL C++ Connector

Question

I am using MySQL C++ connector. Valgrind is showing it is leaking 192 bytes on every connection. It is leaking memory only in threaded environment without threading it is not leaking any memory.What I am doing wrong ? Do I need to call some other functions for cleanup ? Sample code:

#include <pthread.h>
#include <iostream>
#include <cppconn/driver.h>
#include <cppconn/exception.h>
#include <cppconn/resultset.h>
#include <cppconn/statement.h>
#include <cppconn/prepared_statement.h>

using namespace std;

void* test(void* arg) {

  try {
    sql::Driver *driver;
    sql::Connection *con;
    /* Create a connection */
    driver = get_driver_instance();
    con = driver->connect("tcp://127.0.0.1:3306", "root", "root");
    /* Connect to the MySQL test database */
    con->setSchema("test");
    delete con;
  } catch (sql::SQLException &e) {
    cout << "# ERR: SQLException in " << __FILE__;
    cout << "(" << __FUNCTION__ << ") on line " << __LINE__ << endl;
    cout << "# ERR: " << e.what();
    cout << " (MySQL error code: " << e.getErrorCode();
    cout << ", SQLState: " << e.getSQLState() << " )" << endl;
  }
  return NULL;
}

int main() {
  pthread_t thread1, thread2, thread3, thread4;
  pthread_create(&thread1, NULL, test, NULL);
  pthread_create(&thread2, NULL, test, NULL);
  pthread_join(thread1, NULL);
  pthread_join(thread2, NULL);
  return 0;
}

Valgrind output:

==10252== Memcheck, a memory error detector
==10252== Copyright (C) 2002-2011, and GNU GPL'd, by Julian Seward et al.
==10252== Using Valgrind-3.7.0 and LibVEX; rerun with -h for copyright info
==10252== Command: ./app/bin/app-test
==10252== Parent PID: 6312
==10252== 
==10252== 
==10252== HEAP SUMMARY:
==10252==     in use at exit: 384 bytes in 2 blocks
==10252==   total heap usage: 212 allocs, 210 frees, 208,400 bytes allocated
==10252== 
==10252== 192 bytes in 1 blocks are definitely lost in loss record 1 of 2
==10252==    at 0x4C29DB4: calloc (in /usr/lib/valgrind/vgpreload_memcheck-amd64-linux.so)
==10252==    by 0x5E1CB3E: my_thread_init (in /usr/lib/x86_64-linux-gnu/libmysqlclient.so.18.0.0)
==10252==    by 0x5E1CE3C: my_thread_global_init (in /usr/lib/x86_64-linux-gnu/libmysqlclient.so.18.0.0)
==10252==    by 0x5E1AA54: my_init (in /usr/lib/x86_64-linux-gnu/libmysqlclient.so.18.0.0)
==10252==    by 0x5DF86CA: mysql_server_init (in /usr/lib/x86_64-linux-gnu/libmysqlclient.so.18.0.0)
==10252==    by 0x4EA3C08: sql::mysql::NativeAPI::getCApiHandle(sql::SQLString const&) (in /usr/lib/libmysqlcppconn.so.5.1.1.0)
==10252==    by 0x4EA40EA: sql::mysql::NativeAPI::MySQL_NativeDriverWrapper::MySQL_NativeDriverWrapper(sql::SQLString const&) (in /usr/lib/libmysqlcppconn.so.5.1.1.0)
==10252==    by 0x4EA4138: sql::mysql::NativeAPI::createNativeDriverWrapper(sql::SQLString const&) (in /usr/lib/libmysqlcppconn.so.5.1.1.0)
==10252==    by 0x4E6F2F3: sql::mysql::MySQL_Driver::MySQL_Driver(sql::SQLString const&) (in /usr/lib/libmysqlcppconn.so.5.1.1.0)
==10252==    by 0x4E6F50B: sql::mysql::get_driver_instance_by_name(char const*) (in /usr/lib/libmysqlcppconn.so.5.1.1.0)
==10252==    by 0x40AB8C: test(void*) (main.cc:17)
==10252==    by 0x50D9E99: start_thread (pthread_create.c:308)
==10252== 
==10252== 192 bytes in 1 blocks are definitely lost in loss record 2 of 2
==10252==    at 0x4C29DB4: calloc (in /usr/lib/valgrind/vgpreload_memcheck-amd64-linux.so)
==10252==    by 0x5E1CB3E: my_thread_init (in /usr/lib/x86_64-linux-gnu/libmysqlclient.so.18.0.0)
==10252==    by 0x5DF86DC: mysql_server_init (in /usr/lib/x86_64-linux-gnu/libmysqlclient.so.18.0.0)
==10252==    by 0x5DFE85E: mysql_init (in /usr/lib/x86_64-linux-gnu/libmysqlclient.so.18.0.0)
==10252==    by 0x4EA4A82: sql::mysql::NativeAPI::MySQL_NativeConnectionWrapper::MySQL_NativeConnectionWrapper(boost::shared_ptr<sql::mysql::NativeAPI::IMySQLCAPI>) (in /usr/lib/libmysqlcppconn.so.5.1.1.0)
==10252==    by 0x4EA4014: sql::mysql::NativeAPI::MySQL_NativeDriverWrapper::conn_init() (in /usr/lib/libmysqlcppconn.so.5.1.1.0)
==10252==    by 0x4E6F0DC: sql::mysql::MySQL_Driver::connect(sql::SQLString const&, sql::SQLString const&, sql::SQLString const&) (in /usr/lib/libmysqlcppconn.so.5.1.1.0)
==10252==    by 0x40ABE8: test(void*) (main.cc:18)
==10252==    by 0x50D9E99: start_thread (pthread_create.c:308)
==10252==    by 0x5AFCCBC: clone (clone.S:112)
==10252== 
==10252== LEAK SUMMARY:
==10252==    definitely lost: 384 bytes in 2 blocks
==10252==    indirectly lost: 0 bytes in 0 blocks
==10252==      possibly lost: 0 bytes in 0 blocks
==10252==    still reachable: 0 bytes in 0 blocks
==10252==         suppressed: 0 bytes in 0 blocks
==10252== 
==10252== For counts of detected and suppressed errors, rerun with: -v
==10252== ERROR SUMMARY: 2 errors from 2 contexts (suppressed: 2 from 2)

Answer 1

I also get a Valgrind warning. You cannot change MySQL++, but it's possible to limit the problem to 0 or 1 leak. For me, I have ONE class with 1 thread that handles all the SQL-communication for sync/async messages using a thread-safe queue on top.

For every message, I have a class with semaphore, callback function, type, message-id. The id is used in logging.

Also, all re-entrant problems may be solved, since not all mysql implementations are thread-safe.

If people are interested, I'll put this code on sourceforge. It's proven to work, because it's used in many dutch restaurants in Linux POS software with MySQL5.7. Also I checked it with valgrind.

Answer 2

First problem is that your code is wrong. In case of any exception in try block, program will never reach delete con; statement and you will be left with open connection to MySQL server and memory leak. You have to declare and delete con outside the try block or use C++11 smart pointer:

Wrong:

  try {
    sql::Connection *con = driver->connect("URL", "user", "password");

    // code

    delete con;
  }
  catch (const sql::SQLException &e) {
    // code
  }

Correct, before C++11:

  sql::Connection *con = NULL;     
  try {
    con = driver->connect("URL", "user", "password");

    // code

  }
  catch (const sql::SQLException &e) {
    // code
  }
  delete con; // delete on NULL is harmless.
  con = NULL;

Correct, C++11, recommended way:

  try {
    // unique_ptr will automatically call delete when it goes out of scope.
    // That will also happen in case of exception.
    std::unique_ptr<sql::Connection> con (driver->connect("URL", "user", "password"));     

    // code

  }
  catch (const sql::SQLException &e) {
    // code
  }

Also, driver = get_driver_instance(); should be called only once in your program, not in every thread. So you should make sql::Driver *driver; a global variable and call driver = get_driver_instance(); in main() before you create any threads.

Answer 3

as WhozCraig suggested you can add delete Driver; to your test function but i would suggest using auto_ptr or the C++11 unique_ptr or shared_ptr for everything MYSQL and you will never have to worry about memory leaks

take this for example

with C++11

std::unique_ptr< sql::Connection > con( driver->connect("tcp://127.0.0.1:3306", "root", "root"));

or the C++

std::auto_ptr< sql::Connection > con( driver->connect("tcp://127.0.0.1:3306", "root", "root"));

EDIT

you can't just delete Driver , i will look into it more when i have the time

UPDATE

I looked through the driver.h source code, it is indeed protected: virtual ~Driver() {} so you can't just use a delete, how ever in public: there are two

virtual void threadInit() = 0;

    virtual void threadEnd() = 0;

which may be what you need

there is also this example which may be very useful and does this a bit differently then you