On Thu, Apr 07, 2016 at 01:01:52PM +0200, Pavel Březina wrote:
On 04/07/2016 11:51 AM, Jakub Hrozek wrote:
>On Thu, Apr 07, 2016 at 09:16:21AM +0200, Lukas Slebodnik wrote:
>>On (06/04/16 18:38), Jakub Hrozek wrote:
>>>Hi,
>>>
>>>I'm sorry I didn't catch this when I developed the original patch,
but
>>>today I was trying to write the leak patches as Lukas suggested the
>>>other day..I haven't succeeded at that yet, but I found another leak.
>>>
>>Even if you will not able to write a test your effort has some outcome.
>>I glad my idea was not useless.
>
>I'm actually not sure how to write them properly. Normally for the leak
>checks, we would use global_talloc_context instead of NULL in the code
>from the start, but since the memberof plugin uses NULL internally and
>it's a plugin, so there's no _send-style entry point, this wouldn't
>work.
>
>I'll try to see if we can use talloc_enable_null_tracking(), though, but
>ldb also leaks some memory on the NULL context during normal operation
>(IIRC when loading modules), so maybe we'll have to do some trickery
>there..
You can use talloc_total_size(NULL) if you enable null tracking.
OK, that helped me find another leak..see patch #2.
But I'm not sure full NULL-tracking is practical in tests, because there
are so many leaks happening in ldb during normal operation that we
always find some. Mostly it's when casefolding DNs or exploding DNs
For example:
==31730== 666 bytes in 6 blocks are possibly lost in loss record 1,132 of 1,425
==31730== at 0x4C28C50: malloc (in
/usr/lib64/valgrind/vgpreload_memcheck-amd64-linux.so)
==31730== by 0x8BEB7E5: __talloc_with_prefix (talloc.c:668)
==31730== by 0x8BEB7E5: __talloc (talloc.c:708)
==31730== by 0x8BEB7E5: _talloc_named_const (talloc.c:865)
==31730== by 0x8BEB7E5: _talloc_array (talloc.c:2587)
==31730== by 0x8E0582B: ldb_val_dup (ldb_msg.c:106)
==31730== by 0x8E0D437: ldb_handler_copy (attrib_handlers.c:39)
==31730== by 0x8E082B2: ldb_dn_casefold_internal (ldb_dn.c:937)
==31730== by 0x8E091B7: ldb_dn_compare (ldb_dn.c:1136)
==31730== by 0xC1BAEA4: mbof_append_muop (memberof.c:246)
==31730== by 0xC1BF812: mbof_del_fill_muop (memberof.c:2484)
==31730== by 0xC1BD6D2: mbof_orig_del_callback (memberof.c:1539)
==31730== by 0xD60353F: ltdb_callback (ldb_tdb.c:1398)
==31730== by 0x89DDA0C: tevent_common_loop_timer_delay (tevent_timed.c:341)
==31730== by 0x89DEA39: epoll_event_loop_once (tevent_epoll.c:911)
or:
==31730== 714 bytes in 7 blocks are possibly lost in loss record 1,152 of 1,425
==31730== at 0x4C28C50: malloc (in
/usr/lib64/valgrind/vgpreload_memcheck-amd64-linux.so)
==31730== by 0x8BEA613: __talloc_with_prefix (talloc.c:668)
==31730== by 0x8BEA613: __talloc (talloc.c:708)
==31730== by 0x8BEA613: _talloc_named_const (talloc.c:865)
==31730== by 0x8BEA613: _talloc_memdup (talloc.c:2286)
==31730== by 0x8E07A3A: ldb_dn_explode.part.1 (ldb_dn.c:713)
==31730== by 0xC1BF787: mbof_del_fill_muop (memberof.c:2477)
==31730== by 0xC1BD6D2: mbof_orig_del_callback (memberof.c:1539)
==31730== by 0xD60353F: ltdb_callback (ldb_tdb.c:1398)
==31730== by 0x89DDA0C: tevent_common_loop_timer_delay (tevent_timed.c:341)
==31730== by 0x89DEA39: epoll_event_loop_once (tevent_epoll.c:911)
==31730== by 0x89DD136: std_event_loop_once (tevent_standard.c:114)
==31730== by 0x89D938C: _tevent_loop_once (tevent.c:533)
==31730== by 0x8E1647E: ldb_wait (ldb.c:631)
==31730== by 0x8E16F7B: ldb_autotransaction_request (ldb.c:573)
So I'm not sure if it's practical to enable NULL tracking. Feel free to
only use the third patch for verification of the previous leak fixes, but
I guess we should only commit the first two.. If we want to have tests,
then I think valgrind suppression would be more practical.