Memory leak if exception is thrown in connection constructor

[ociotec]

sqlpp11-connector-mysql/src/connection.cpp

Lines 139 to 151 in e85d2ae

	connection::connection(const std::shared_ptr<connection_config>& config)
	: _handle(new detail::connection_handle_t(config))
	{
	if (mysql_set_character_set(_handle->mysql.get(), _handle->config->charset.c_str()))
	{
	throw sqlpp::exception("MySQL error: can't set character set " + _handle->config->charset);
	}
	if (mysql_select_db(_handle->mysql.get(), _handle->config->database.c_str()))
	{
	throw sqlpp::exception("MySQL error: can't select database '" + _handle->config->database + "'");
	}
	}

_handle is init with a new object, but if an exception is thrown by the constructor this memory is not deleted due the fact C++ will not call the destructor due the invalid object creation.

Memory should delete in the constructor itself before throwing the exception.

[ociotec]

By the way, credits: Valgrind detected this issue.

[rbock]

Thanks! I believe I fixed this with the commit I just pushed to develop.

[ociotec]

@rbock sorry but now I have 2 possible memory leaks and 1 definite memory leak report from valgrind (before I only had the 2 possible memory leaks).

I'm running valgrind with these options:

valgrind --leak-check=full --error-limit=no --show-leak-kinds=definite,indirect,possible \
         --log-file=reports/valgrind.log \
         --xml=yes --xml-file=reports/valgrind.xml

I've atteched the XML report valgrind.xml.tar.gz (I don't know why but plain text report is empty...).

[ociotec]

I was able to generate the valgrid plain text log file (you can only generate plain text or XML, but not both at the same time...): valgrind.log

I hope this could help you... thanks! 👍

[rbock]

I am sure this will help. Will look into it asap. Thanks for the reports!!

[rbock]

Thanks! This helped indeed. A new version is on develop.

I fixed a leak and added an raii struct for mysql_library_init/mysql_library_end. valgrind seems happy in the few tests I ran by now.

[ociotec]

@rbock I will try to test it asap. Thanks!

[ociotec]

@rbock now we have the 2 original possible memory leaks detected by Valgrind (valgrind.log).

I think problem could fix it adding delete _handle before throwing the exceptions:

    connection::connection(const std::shared_ptr<connection_config>& config)
        : _handle(new detail::connection_handle_t(config))
    {
      if (mysql_set_character_set(_handle->mysql.get(), _handle->config->charset.c_str()))
      {
        delete _handle;
        throw sqlpp::exception("MySQL error: can't set character set " + _handle->config->charset);
      }

      if (mysql_select_db(_handle->mysql.get(), _handle->config->database.c_str()))
      {
        delete _handle;
        throw sqlpp::exception("MySQL error: can't select database '" + _handle->config->database + "'");
      }
}

Maybe it's good to take a look to mysql_set_character_set & mysql_select_db to check if they also throw any exception...

[rbock]

Are you sure you are using the latest version? If so, which program are you running with valgrind?

Deleting _handle does not really help since it is a unique_ptr and will clean itself up.

There was a problem in case new MYSQL failed, but I just pushed something that I guess will fix that, too.

For ./tests/Sqlpp11MySQLSampleTest valgrind tells me:

All heap blocks were freed -- no leaks are possible

Hope this is it now. In any case, thanks for pushing!

[ociotec]

Run Valgrind like this (include possible leak kind):

valgrind --leak-check=full --error-limit=no --show-leak-kinds=definite,indirect,possible \
         --log-file=valgrind.log <program-to-run>

[ociotec]

@rbock I think I correctly updated develop version due the fact the 1 definite leak error dissapeared with your latest version.

[rbock]

Sure, that's what I do. And it says no leaks are possible. This is why I would like to know which program you are running. Can you post the code of a minimal program that shows the leak?

Have you tried with the code I pushed today (like 30mins ago)?

[rbock]

Also, can you please make sure that you are using either the

const auto library_raii = sqlpp::mysql::scoped_library_initializer_t{};

or

::sqlpp::mysql::global_library_init();

(See tests)

These make sure to initialize the mysql library and free its resources when you are done.

Thanks!

[ociotec]

@rbock finally, with latest changes + calling sqlpp::mysql::global_library_init(); we achieved a clean Valgrind report (valgrind.log).

Thanks a lot!

[rbock]

Awesome!

Thanks for your help :-)