Bug 548537 - possible memory leaks in DNA plugin
Summary: possible memory leaks in DNA plugin
Keywords:
Status: CLOSED CURRENTRELEASE
Alias: None
Product: 389
Classification: Retired
Component: Server - DNA Plug-in
Version: 1.3.0
Hardware: All
OS: Linux
low
medium
Target Milestone: ---
Assignee: Nathan Kinder
QA Contact: Viktor Ashirov
URL:
Whiteboard:
Depends On:
Blocks: 434914 389_1.2.5
TreeView+ depends on / blocked
 
Reported: 2009-12-17 18:07 UTC by Noriko Hosoi
Modified: 2015-12-07 16:38 UTC (History)
2 users (show)

Fixed In Version:
Doc Type: Bug Fix
Doc Text:
Clone Of:
Environment:
Last Closed: 2015-12-07 16:38:48 UTC


Attachments (Terms of Use)
Patch (1.25 KB, patch)
2009-12-17 19:10 UTC, Nathan Kinder
no flags Details | Diff
Revised Patch (1.27 KB, patch)
2009-12-17 22:14 UTC, Nathan Kinder
nhosoi: review+
Details | Diff
valgrind logs (380.00 KB, application/x-tar)
2010-06-02 20:04 UTC, Jenny Severance
no flags Details
with debug (320.00 KB, application/x-tar)
2010-06-02 20:59 UTC, Jenny Severance
no flags Details

Description Noriko Hosoi 2009-12-17 18:07:52 UTC
Description of problem:
Found in TET DNA + valgrind:

dnarun.vg
dna_scen3.vg
dna_scen5.vg
dna_scen6.vg
dna_scen7.vg
dna_scen8.vg
==27588== 19,654 (2,557 direct, 17,097 indirect) bytes in 46 blocks are definitely lost in loss record 31 of 39
==27588==    at 0x4006F3D: malloc (vg_replace_malloc.c:207)
==27588==    by 0x4039A22: slapi_ch_malloc (ch_malloc.c:155)
==27588==    by 0x4049AFF: slapi_entry_attr_get_charptr (entry.c:2441)
==27588==    by 0x4DBC7C6: dna_pre_op (dna.c:2645)
==27588==    by 0x4DBCC51: dna_add_pre_op (dna.c:2823)
==27588==    by 0x407FBD2: plugin_call_func (plugin.c:1400)
==27588==    by 0x407FAAD: plugin_call_list (plugin.c:1362)
==27588==    by 0x407E246: plugin_call_plugins (plugin.c:355)
==27588==    by 0x4031E62: op_shared_add (add.c:606)
==27588==    by 0x40311BA: do_add (add.c:232)
==27588==    by 0x8057521: connection_dispatch_operation (connection.c:544)
==27588==    by 0x8058B9B: connection_threadmain (connection.c:2267)
==27588==    by 0xDA0990: (within /lib/libnspr4.so)
==27588==    by 0x5ED934: start_thread (in /lib/libpthread-2.10.1.so)
==27588==    by 0x52294D: clone (in /lib/libc-2.10.1.so)

dna_scen4.vg
==1033== 158 (32 direct, 126 indirect) bytes in 4 blocks are definitely lost in loss record 7 of 40
==1033==    at 0x4006F3D: malloc (vg_replace_malloc.c:207)
==1033==    by 0x4145D97: nslberi_malloc (in /usr/lib/libldap60.so)
==1033==    by 0x4145DC5: ber_flatten (in /usr/lib/libldap60.so)
==1033==    by 0x4DBAB52: dna_create_range_request (dna.c:1649)
==1033==    by 0x4DBA6CB: dna_request_range (dna.c:1529)
==1033==    by 0x4DB9E5C: dna_fix_maxval (dna.c:1264)
==1033==    by 0x4DBA14A: dna_notice_allocation (dna.c:1349)
==1033==    by 0x4DBB647: dna_get_next_value (dna.c:2021)
==1033==    by 0x4DBC9D9: dna_pre_op (dna.c:2737)
==1033==    by 0x4DBCC51: dna_add_pre_op (dna.c:2823)
==1033==    by 0x407FBD2: plugin_call_func (plugin.c:1400)
==1033==    by 0x407FAAD: plugin_call_list (plugin.c:1362)
==1033==    by 0x407E246: plugin_call_plugins (plugin.c:355)
==1033==    by 0x4031E62: op_shared_add (add.c:606)
==1033==    by 0x40311BA: do_add (add.c:232)
==1033==    by 0x8057521: connection_dispatch_operation (connection.c:544)
==1033==    by 0x8058B9B: connection_threadmain (connection.c:2267)
==1033==    by 0xDA0990: (within /lib/libnspr4.so)
==1033==    by 0x5ED934: start_thread (in /lib/libpthread-2.10.1.so)
==1033==    by 0x52294D: clone (in /lib/libc-2.10.1.so)

Comment 1 Nathan Kinder 2009-12-17 19:10:21 UTC
Created attachment 379075 [details]
Patch

This fixes two small memory leaks in the DNA plugin.  We were
leaking the extended operation request data for range requests as
well as an attribute value for ADD operations when checking for
the magic value.

Comment 2 Noriko Hosoi 2009-12-17 19:54:02 UTC
dnarun, dna_scen6,8 do not show the leak any more.

dna_scen1,2,3,5,7 still do:
==2787== 29,884 (4,234 direct, 25,650 indirect) bytes in 44 blocks are definitely lost in loss record 61 of 73
==2787==    at 0x4A0763E: malloc (vg_replace_malloc.c:207)
==2787==    by 0x4C50449: slapi_ch_malloc (ch_malloc.c:155)
==2787==    by 0x4C610A6: slapi_entry_attr_get_charptr (entry.c:2441)
==2787==    by 0xADEB5FF: dna_pre_op (dna.c:2648)
==2787==    by 0xADEBB68: dna_add_pre_op (dna.c:2828)
==2787==    by 0x4C9B03E: plugin_call_func (plugin.c:1400)
==2787==    by 0x4C9AF03: plugin_call_list (plugin.c:1362)
==2787==    by 0x4C99460: plugin_call_plugins (plugin.c:355)
==2787==    by 0x4C47F02: op_shared_add (add.c:606)
==2787==    by 0x4C4728B: do_add (add.c:232)
==2787==    by 0x413221: connection_dispatch_operation (connection.c:544)
==2787==    by 0x4149A1: connection_threadmain (connection.c:2267)
==2787==    by 0x32B7429262: (within /lib64/libnspr4.so)
==2787==    by 0x37A6406869: start_thread (in /lib64/libpthread-2.10.1.so)
==2787==    by 0x37A58DE3BC: clone (in /lib64/libc-2.10.1.so)

dna_scen4 still shows this leak:
==6865== 70 (32 direct, 38 indirect) bytes in 2 blocks are definitely lost in loss record 31 of 75
==6865==    at 0x4A0763E: malloc (vg_replace_malloc.c:207)
==6865==    by 0x537109C: ber_get_stringal (decode.c:321)
==6865==    by 0x53719A5: ber_scanf (decode.c:534)
==6865==    by 0x53534BA: ldap_parse_extended_result (extendop.c:254)
==6865==    by 0x53538D9: ldap_extended_operation_s (extendop.c:190)
==6865==    by 0xADE9466: dna_request_range (dna.c:1568)
==6865==    by 0xADE8981: dna_fix_maxval (dna.c:1264)
==6865==    by 0xADE8C22: dna_notice_allocation (dna.c:1349)
==6865==    by 0xADEA238: dna_get_next_value (dna.c:2024)
==6865==    by 0xADEB887: dna_pre_op (dna.c:2742)
==6865==    by 0xADEBB68: dna_add_pre_op (dna.c:2828)
==6865==    by 0x4C9B03E: plugin_call_func (plugin.c:1400)
==6865==    by 0x4C9AF03: plugin_call_list (plugin.c:1362)
==6865==    by 0x4C99460: plugin_call_plugins (plugin.c:355)
==6865==    by 0x4C47F02: op_shared_add (add.c:606)
==6865==    by 0x4C4728B: do_add (add.c:232)
==6865==    by 0x413221: connection_dispatch_operation (connection.c:544)
==6865==    by 0x4149A1: connection_threadmain (connection.c:2267)
==6865==    by 0x32B7429262: (within /lib64/libnspr4.so)
==6865==    by 0x37A6406869: start_thread (in /lib64/libpthread-2.10.1.so)
==6865==    by 0x37A58DE3BC: clone (in /lib64/libc-2.10.1.so)

Comment 3 Noriko Hosoi 2009-12-17 21:06:53 UTC
Please ignore my previous test result.

The first type of leak from slapi_entry_attr_get_charptr (entry.c:2441) completely disappears once the patch is applied correctly.

Comment 4 Nathan Kinder 2009-12-17 22:14:24 UTC
Created attachment 379106 [details]
Revised Patch

This revised patch should address the additional leak of the range transfer extended operation response data.

Comment 5 Noriko Hosoi 2009-12-17 22:53:33 UTC
Comment on attachment 379106 [details]
Revised Patch

Excellent!

No leaks are reported from the server that the patch is applied to.

Comment 6 Nathan Kinder 2009-12-17 23:11:13 UTC
Thanks for the review!  Pushed to master.

Counting objects: 13, done.
Delta compression using 2 threads.
Compressing objects: 100% (7/7), done.
Writing objects: 100% (7/7), 805 bytes, done.
Total 7 (delta 5), reused 0 (delta 0)
To ssh://git.fedorahosted.org/git/389/ds.git
   c4c1ea0..176c542  master -> master

Comment 8 Jenny Severance 2010-06-02 20:04:19 UTC
Created attachment 419147 [details]
valgrind logs

Comment 9 Jenny Severance 2010-06-02 20:05:10 UTC
Please review the valgrind logs attached for the memory leak fix.  Thanks!

Comment 11 Jenny Severance 2010-06-02 20:59:36 UTC
Created attachment 419164 [details]
with debug

Comment 13 Rich Megginson 2010-06-02 21:11:08 UTC
None of these leaks are the original leak reported by this bug.  I think we can declare this bug verified.

Comment 14 Nathan Kinder 2010-06-02 22:58:50 UTC
There is a leak of the shared config DN when dna config is parsed in these reports, but it is not the same as the leaks from the original report (as Rich said).

This bug can be marked as verified, but please open a new bug for the new leak and provide the debug leak reports.

Comment 15 Jenny Severance 2010-06-03 12:42:41 UTC
verified - RHEL 4

version:
redhat-ds-base-8.2.0-2010060204.el4dsrv

Thanks Nathan!


Note You need to log in before you can comment on or make changes to this bug.