soci icon indicating copy to clipboard operation
soci copied to clipboard

Valgrind memory leaks in rowsets / values.h with mysql

Open b1c9g76 opened this issue 8 years ago • 6 comments

==18396== 60 (56 direct, 4 indirect) bytes in 1 blocks are definitely lost in loss record 1,923 of 3,250
==18396==    at 0x4C2A105: operator new(unsigned long) (in /usr/lib64/valgrind/vgpreload_memcheck-amd64-linux.so)
==18396==    by 0x5332A0: void soci::values::set<std::string>(std::string const&, soci::indicator) (values.h:217)
==18396==    by 0x4FB39D: soci::values& soci::values::operator<< <std::string>(std::string const&) (values.h:224)
==18396==    by 0x42E895: soci::type_conversion<MyVectorOfStrings, void>::to_base(MyVectorOfStrings const&, soci::values&, soci::indicator&) (database.hpp:33)
==18396==    by 0x5C3B92: soci::details::conversion_use_type<MyVectorOfStrings>::convert_to_base() (type-conversion.h:156)
==18396==    by 0x422928: soci::details::use_type<soci::values>::pre_use() (values-exchange.h:62)
==18396==    by 0x50673B2: soci::details::statement_impl::pre_use() (in /usr/lib64/libsoci_core.so.3.2.3)
==18396==    by 0x506A373: soci::details::statement_impl::execute(bool) (in /usr/lib64/libsoci_core.so.3.2.3)
==18396==    by 0x422371: soci::statement::execute(bool) (statement.h:210)
==18396==    by 0x5556E3: soci::details::rowset_impl<soci::row>::rowset_impl(soci::details::prepare_temp_type const&) (rowset.h:126)
==18396==    by 0x5370EE: soci::rowset<soci::row>::rowset(soci::details::prepare_temp_type const&) (rowset.h:185)

Seeing a bunch of memory leaks coming from pointers in values.h

Having a looking the code I see some todos around the pointers that are included in the leaks:

//TODO To make values generally usable outside of type_conversion's,
// these should be reference counted smart pointers
row * row_;
std::vector<details::standard_use_type *> uses_;
std::map<details::use_type_base *, indicator *> unused_;
std::vector<indicator *> indicators_;
std::map<std::string, std::size_t> index_;
std::vector<details::copy_base *> deepCopies_;

mutable std::size_t currentPos_;

Is this a known problem? And if so, are there any known workarounds for seeing rowset data released properly?

b1c9g76 avatar Nov 16 '16 09:11 b1c9g76

@b1c9g76 We have some issues with MySQL backend. But seems to me none of them are similar to problem you've encountered.

To better understand the issue could you please write minimal example or test to expose issue described?

And patches are always welcomed.

Thank you.

snikulov avatar Nov 16 '16 14:11 snikulov

It look at this stage that in values, use_types are being pushed onto the uses_ vector (4 in the case of a two variable query) (values.h:217) but when we get to clean up, the vector size is only 3 (statement.cpp:168 ish). One of them is getting clobbered along the way resulting in a leak.

Any ideas?

b1c9g76 avatar Nov 17 '16 06:11 b1c9g76

I'm wondering if void standard_use_type::bind(statement_impl & st, int & position) in use_type.cpp is right. It seems that 2 of the 4 passed values are bound and the other two are not. Are unbound pointers getting freed? Still trying to work out how all this works (and why there are 2 times the number of parameters in the first place)

b1c9g76 avatar Nov 17 '16 08:11 b1c9g76

On 17/11/2016 09:22, b1c9g76 wrote:

(and why there are 2 times the number of parameters in the first place)

Because you are trying to reuse the statement object or re-binding parameters between statement executions?

Regards,

Maciej Sobczak * www.msobczak.com * www.inspirel.com

msobczak avatar Nov 17 '16 15:11 msobczak

Thanks for your comment.

I'm not sure how. There's one call to prepare and that's it. Our use() call is a vector with two strings in it. So I'm not thinking it's that.

We're certainly reusing the session object. In the case of repeated prepare() calls for queries only, is there a way to explicitly delete the statement object? Or are we just using the whole thing incorrectly?

PS. We merged in the patch from the other mysql leak bug and found that it greatly reduced the amount of leaks.

b1c9g76 avatar Nov 17 '16 21:11 b1c9g76

I stand corrected. We are calling twice by virtue of using the vector binding type_conversion solution as described here: https://github.com/SOCI/soci/issues/354

Redbolt's final solution which subtly uses position binding rather than name binding one you detailed causes the leak as use() seems to be getting called twice. Still not sure as to why to_base() gets called twice, but that was the cause of the leak. Still might be a problem there with position binding as multiple calls, while daft, should be easy enough to clean up.

b1c9g76 avatar Nov 18 '16 01:11 b1c9g76