[CONC-117] Memory leak while reconnecting to server Created: 2014-12-20  Updated: 2016-02-29  Resolved: 2014-12-21

Status: Closed
Project: MariaDB Connector/C
Component/s: None
Affects Version/s: 2.0.0
Fix Version/s: 2.1

Type: Bug Priority: Major
Reporter: Tianhong Assignee: Georg Richter
Resolution: Fixed Votes: 0
Labels: None
Environment:

Linux 32/64 bit

Attachments: File test-leak.cpp    

 Description   

Hi,

memory-leak is detected when the server is off and the client keeps reconnecting to it. Here is test code with steps how you can repeat the bug:

//--------------------------TEST CODE--------------------------------------
 
#include <stdio.h>
 
#include <stdlib.h>
 
#include <time.h>
 
#include <iostream>
 
 
 
#include "mysql.h"
 
 
 
using namespace std;
 
 
 
int main(int argc, char** argv)
 
{
 
    int nRC = 0;
 
    MYSQL*   pMySQL = NULL;
 
    my_bool  bReconnect = 1;
 
    MYSQL_RES *pRes;
 
 
 
    pMySQL = mysql_init(NULL);
 
    mysql_options(pMySQL, MYSQL_OPT_RECONNECT, &bReconnect);
 
 
 
    if( mysql_real_connect( pMySQL, "127.0.0.1", "root", "",
 
                            NULL, 3306, NULL, 0) != NULL    )
 
        while(true) {
 
            if( (nRC = mysql_query(pMySQL, "SELECT 'hello world'")) == 0) {
 
                pRes = mysql_store_result(pMySQL);
 
                mysql_free_result(pRes);
 
                cout << "Query ok...." << endl;
 
            }
 
            else
 
                cout << "Query error, code: " << nRC
 
                     << ", msg: " << mysql_error(pMySQL) << endl;
 
            usleep(4000);
 
        }
 
    else
 
        cout << "Unable to connect to mysqld" << endl;
 
 
 
    mysql_close(pMySQL);
 
 
 
    return 0;
 
}
 
//----------------------------------------------------------------------

Step 1. Build the test code
Step 2. Start MySQL/Mariadb server
Step 3. Execute the test binary (in terminal or valgrind), and it would keep printing "Query OK"..
Step 4. Shut down MySQL/Mariadb server. And the client would keep printing "Query error..."
Step 5. Watch the memory usage of the test binary, and it would keep raising.

After debugging for a while, I thought the bug locates in function "mysql_reconnect" in file "libmariadb.c" around line 1992.

The original logic is to let a tmp_client to rebuild a new connection which has to allocate some structures such as hash_table for keeping the mysql_options. But when it fails to reconnect, the destructor "mysql_close" should be called in order to free the allocated memory. And it seems someone has fogot this point.

So after line 1992: my_set_error(mysql,..... )
I added this line as a test patch:
mysql_close(&tmp_mysql);

And it seems to work well without memory leak any more.
But I'm not 100% sure of it and looking forward for your response.

Thanks and best wish
Tianhong



 Comments   
Comment by Georg Richter [ 2014-12-20 ]

Thanks for your bug report. Verified - the leak also exists in servers (GPL licensed) client library.

Comment by Tianhong [ 2014-12-21 ]

Hi Georg,

Thanks for your reply.

As you said, I've tried the same test code with the c client library from MySQL-5.6.20 and didn't find any leak. The memory usage looks quite normal.

Comment by Georg Richter [ 2014-12-21 ]

Fixed in rev. 185

Comment by Tianhong [ 2014-12-21 ]

Hi Georg,

thanks for your work and support.

Today I reviewed the code and thought that there are also a few bugs in function "mysql_reconnect"
The problems are:
1. original options would get lost if reconnecting fails, because of the bzero() before mysql_real_connect()
2. The free_me flag in original mysql object is set to 0, which would lead to memory leak. You should store the old value and set it back after the cloning from tmp_mysql object.

I wrote the patch with some comment.
Please do a code review if you have time.

static my_bool mysql_reconnect(MYSQL *mysql)
{
MYSQL tmp_mysql;
//--Origial free_me flag in mysql object
my_bool free_me;
LIST *li_stmt= mysql->stmts;
DBUG_ENTER("mysql_reconnect");

if (!mysql->reconnect ||
(mysql->server_status & SERVER_STATUS_IN_TRANS) || !mysql->host_info)

{ /* Allov reconnect next time */ mysql->server_status&= ~SERVER_STATUS_IN_TRANS; my_set_error(mysql, CR_SERVER_GONE_ERROR, SQLSTATE_UNKNOWN, 0); DBUG_RETURN(1); }

mysql_init(&tmp_mysql);
tmp_mysql.options=mysql->options;

/* don't reread options from configuration files */
tmp_mysql.options.my_cnf_group= tmp_mysql.options.my_cnf_file= NULL;

/* make sure that we reconnect with the same character set */
if (!tmp_mysql.options.charset_name ||
strcmp(tmp_mysql.options.charset_name, mysql->charset->csname))

{ my_free(tmp_mysql.options.charset_name, MYF(MY_ALLOW_ZERO_PTR)); tmp_mysql.options.charset_name= my_strdup(mysql->charset->csname, MYF(MY_WME)); }

tmp_mysql.reconnect= mysql->reconnect;
//--------BUG----------------------------
//-We have to keep the original options--
//-So don't set the options to zero here-
//bzero((char*) &mysql->options,sizeof(mysql->options));
if (!mysql_real_connect(&tmp_mysql,mysql->host,mysql->user,mysql->passwd,
mysql->db, mysql->port, mysql->unix_socket,
mysql->client_flag | CLIENT_REMEMBER_OPTIONS))

{ my_set_error(mysql, tmp_mysql.net.last_errno, tmp_mysql.net.sqlstate, tmp_mysql.net.last_error); //--If it fails, just destroy the tmp_mysql. //--But don't delete the options in tmp_mysql //--, because the options are shared between mysql and tmp_mysql //--The options need to be reused next time. //--Set the options in tmp_mysql to NULL, in order to guard //--the original pointers of options in mysql object. bzero((char*) &tmp_mysql.options,sizeof(tmp_mysql.options)); mysql_close(&tmp_mysql); DBUG_RETURN(1); }

/* reset the connection in all active statements
todo: check stmt->mysql in mysql_stmt* functions ! */
for (;li_stmt;li_stmt= li_stmt->next)
{
MYSQL_STMT *stmt= (MYSQL_STMT *)li_stmt->data;

if (stmt->state != MYSQL_STMT_INITTED)

{ stmt->mysql= NULL; stmt->state= MYSQL_STMT_INITTED; SET_CLIENT_STMT_ERROR(stmt, CR_SERVER_LOST, SQLSTATE_UNKNOWN, 0); }

else
tmp_mysql.stmts= list_add(tmp_mysql.stmts, &stmt->list);
}

//-------------BUG----------------
//We also have to keep the origial free_me flag
//The flag needs to be set back after cloning.
free_me = mysql->free_me;
mysql->free_me=0;
mysql->stmts= NULL;
//----BUG-------------------------
//--The options should be set to NULL before mysql_close()
//--Otherwise, the options would be deleted in mysql_close()
memset(&mysql->options, 0, sizeof(mysql->options));
mysql_close(mysql);
//memset(&mysql->options, 0, sizeof(mysql->options));
*mysql=tmp_mysql;
//--Now, set the original free_me flag back.
mysql->free_me = free_me;
mysql->reconnect= 1;
net_clear(&mysql->net);
mysql->affected_rows= ~(my_ulonglong) 0;
DBUG_RETURN(0);
}

Generated at Thu Feb 08 03:03:00 UTC 2024 using Jira 8.20.16#820016-sha1:9d11dbea5f4be3d4cc21f03a88dd11d8c8687422.