Memory Leak in SQL backend

[andrewkcorcoran]

On shutdown Clang LeakSanitizer is warning about a single object leak in libmysqlclient.so and stderr has Error in my_thread_global_end(): 1 threads didn't exit. Adding the below calls to the mysql thread init and end methods fixes the issue. https://github.com/SOCI/soci/blob/master/src/backends/mysql/session.cpp#L54 probably needs to be updated to call these methods for every thread (a good place to use thread_local perhaps?).

https://dev.mysql.com/doc/c-api/8.0/en/mysql-thread-init.html
https://dev.mysql.com/doc/c-api/8.0/en/mysql-thread-end.html

[vadz]

The documentation you linked states that calling these functions is unnecessary if mysql_library_{init,end}() are called, so I don't see how could it help.

I'm also not sure when exactly do you get the error message: is this for your own application? If so, please try reproducing it in SOCI test suite to make sure it's really due to a bug in SOCI.

TIA!

[andrewkcorcoran]

My reading of the documentation is that (for debug builds) the thread end call is required. The error occurs in a debug build of our application. When I modify the soci code to use these two functions the error goes away but I'll look to see if I can create a minimal example.

[vadz]

As it's open source, it should be possible to confirm this by looking at its sources, but my understanding is that mysql_library_xxx() functions already call mysql_thread_xxx() ones. If they don't, the documentation of the init function is really misleading.

[andrewkcorcoran]

Minimal example below which triggers an error Error in my_thread_global_end(): 1 threads didn't exit to print to stderr and a LSAN error when compiled in Debug with SOCI v4.0.3

void Test(const std::string & connection_string)
{
    auto t = std::thread{[&]() {
        auto session = soci::session{};
        session.open(soci::mysql, connection_string);
        // mysql_thread_end(); // Uncomment to resolve error
    }};
    t.join();
}

Error in my_thread_global_end(): 1 threads didn't exit

=================================================================
==3612==ERROR: LeakSanitizer: detected memory leaks

Direct leak of 200 byte(s) in 1 object(s) allocated from:
    #0 0x127aa49 in calloc (/local/1/home/snip/source_code/snip2/build/clang/source/bin/test_sql+0x127aa49) (BuildId: 4ab046215d7fed3cc76d70c44a19ebfaa68128e8)
    #1 0x7f53a6da506e in my_thread_init /home/buildbot/buildbot/padding_for_CPACK_RPM_BUILD_SOURCE_DIRS_PREFIX/mariadb-10.1.36/mysys/my_thr_init.c:295
    #2 0x7f53a6da5216 in my_thread_global_init /home/buildbot/buildbot/padding_for_CPACK_RPM_BUILD_SOURCE_DIRS_PREFIX/mariadb-10.1.36/mysys/my_thr_init.c:190
    #3 0x7f53a6da2ebc in my_init /home/buildbot/buildbot/padding_for_CPACK_RPM_BUILD_SOURCE_DIRS_PREFIX/mariadb-10.1.36/mysys/my_init.c:105
    #4 0x7f53a6d60afb in mysql_server_init /home/buildbot/buildbot/padding_for_CPACK_RPM_BUILD_SOURCE_DIRS_PREFIX/mariadb-10.1.36/libmysql/libmysql.c:134
    #5 0x205e29d in (anonymous namespace)::mysql_library::mysql_library() /local/1/home/snip/source_code/snip2/build/clang/_deps/soci-src/src/backends/mysql/session.cpp:40:13
    #6 0x205948f in (anonymous namespace)::mysql_library::ensure_initialized() /local/1/home/snip/source_code/snip2/build/clang/_deps/soci-src/src/backends/mysql/session.cpp:56:30
    #7 0x2056885 in soci::mysql_session_backend::mysql_session_backend(soci::connection_parameters const&) /local/1/home/snip/source_code/snip2/build/clang/_deps/soci-src/src/backends/mysql/session.cpp:363:5
    #8 0x205573c in soci::mysql_backend_factory::make_session(soci::connection_parameters const&) const /local/1/home/snip/source_code/snip2/build/clang/_deps/soci-src/src/backends/mysql/factory.cpp:26:17
    #9 0x215aa53 in soci::session::open(soci::connection_parameters const&) /local/1/home/snip/source_code/snip2/build/clang/_deps/soci-src/src/core/session.cpp:172:29
    #10 0x215c31c in soci::session::open(soci::backend_factory const&, std::string const&) /local/1/home/snip/source_code/snip2/build/clang/_deps/soci-src/src/core/session.cpp:180:5

[andrewkcorcoran]

I don't see any reason to look at the source of mysql to figure this out. The documentation from https://dev.mysql.com/doc/c-api/8.0/en/mysql-thread-end.html is very clear. For debug builds mysql_thread_end must be called for every corresponding mysql_thread_init. The documentation at https://dev.mysql.com/doc/c-api/8.0/en/mysql-thread-init.html is also very clear, mysql_thread_init must be called for every thread. The library may call it on your behalf in some circumstances, but the function has still been called regardless if it was done implicitly or explicitly.

[vadz]

This is really weird because looking at the code you can see that it behaves in the way I thought it would, i.e. mysql_server_end() does call mysql_thread_end():

https://github.com/mysql/mysql-server/blob/ea7087d885006918ad54458e7aad215b1650312c/libmysql/libmysql.cc#L202-L206

(my_end() calls my_thread_end() inside it too). Could this have been changed/fixed in a later version?

Anyhow, if a simple test like this fails, then let's call my_thread_end() ourselves. I am not sure if it should be called before or after mysql_library_end(), but if doing it before (as in the test above) works, let's do it like this. Would you please make a PR with the proposed change?

TIA!