Bug 709826 - Memory leak: when extra referrals configured
Summary: Memory leak: when extra referrals configured
Keywords:
Status: CLOSED CURRENTRELEASE
Alias: None
Product: 389
Classification: Retired
Component: Replication - General
Version: 1.2.8
Hardware: Unspecified
OS: Unspecified
high
unspecified
Target Milestone: ---
Assignee: Noriko Hosoi
QA Contact: Viktor Ashirov
URL:
Whiteboard:
Depends On:
Blocks: 434915 389_1.2.9 711533
TreeView+ depends on / blocked
 
Reported: 2011-06-01 18:23 UTC by Noriko Hosoi
Modified: 2015-12-07 16:44 UTC (History)
2 users (show)

Fixed In Version:
Doc Type: Bug Fix
Doc Text:
Clone Of:
: 711533 (view as bug list)
Environment:
Last Closed: 2015-12-07 16:44:29 UTC
Embargoed:


Attachments (Terms of Use)
git patch file (master) (1.17 KB, patch)
2011-06-03 01:15 UTC, Noriko Hosoi
nhosoi: review?
rmeggins: review+
Details | Diff

Description Noriko Hosoi 2011-06-01 18:23:07 UTC
Description of problem:
valgrind captured this leak.

condition:
Set up simple 2 MMR.
Configure extra referrals.

    ==14844== 1,322,989 bytes in 71,304 blocks are definitely lost in loss record 2,149 of 2,154
    ==14844==    at 0x4A05FDE: malloc (vg_replace_malloc.c:236)
    ==14844==    by 0x3989807F8B: ber_memalloc_x (in /lib64/liblber-2.4.so.2.5.6)
    ==14844==    by 0x3989803EEF: ber_get_stringbv (in /lib64/liblber-2.4.so.2.5.6)
    ==14844==    by 0x39898040BC: ??? (in /lib64/liblber-2.4.so.2.5.6)
    ==14844==    by 0x39898045E6: ber_scanf (in /lib64/liblber-2.4.so.2.5.6)
    ==14844==    by 0x93FCDC9: multimaster_extop_StartNSDS50ReplicationRequest (in /usr/lib64/dirsrv/plugins/libreplication-plugin.so)
    ==14844==    by 0x313DC89B3D: plugin_call_exop_plugins (in /usr/lib64/dirsrv/libslapd.so.0.0.0)
    ==14844==    by 0x4182D6: ??? (in /usr/sbin/ns-slapd)
    ==14844==    by 0x413563: ??? (in /usr/sbin/ns-slapd)
    ==14844==    by 0x3985829632: ??? (in /lib64/libnspr4.so)
    ==14844==    by 0x39774077E0: start_thread (in /lib64/libpthread-2.12.so)
    ==14844==    by 0x39770E68EC: clone (in /lib64/libc-2.12.so)
    ==14844==

ber_scanf is called from decode_startrepl_extop, which is called by multimaster_extop_StartNSDS50ReplicationRequest.  There is one ber_scanf call with 'v' (sequence of strings).

     320 static int
     321 decode_startrepl_extop(Slapi_PBlock *pb, char **protocol_oid, char **repl_r     oot,
     322                        RUV **supplier_ruv, char ***extra_referrals, char **     csnstr,
     323                        char **data_guid, struct berval **data, int *is90)
     324 {
                    [...]
     391     /* Get the optional set of referral URLs */
     392     if (ber_peek_tag(tmp_bere, &len) == LBER_SET)
     393     {
     394         if (ber_scanf(tmp_bere, "[v]", extra_referrals) == LBER_ERROR)
     395         {
     396             rc = -1;
     397             goto free_and_return;
     398         }
     399     }

The caller multimaster_extop_StartNSDS50ReplicationRequest is responsible to release extra_referrals/referrals, which is calling slapi_ch_free.  Actually, shouldn't it be charray_free or ber_bvecfree?

Rich Megginson wrote:
> Yes - slapi_ch_array_free

     603 int
     604 multimaster_extop_StartNSDS50ReplicationRequest(Slapi_PBlock *pb)
     605 {
                    [...]
     641     /* Decode the extended operation */
     642     if (decode_startrepl_extop(pb, &protocol_oid, &repl_root, &supplier_ruv     ,
     643             &referrals, &replicacsnstr, &data_guid, &data, &is90) == -1)
                    [...]
    1086     /* referrals */
    1087     slapi_ch_free((void **)&referrals);
    1088

Comment 1 Noriko Hosoi 2011-06-03 01:15:17 UTC
Created attachment 502700 [details]
git patch file (master)

File: ldap/servers/plugins/replication/repl_extop.c

Description: "referrals" allocated in ber_scanf called with "[v]"
option is an array of chars.  It should have been freed by
slapi_ch_array_free, but slapi_ch_free was actually called, which
does not releases the real strings, but just the array of pointers.

This patch calls slapi_ch_array_free.

Comment 2 Noriko Hosoi 2011-06-03 01:28:02 UTC
Steps to reproduce:
0. MMR topology
  M1 <--> M2
    \    /
     v  v
      Hub
       |
       v
     Consumer

The leak is observed on Consumer.  To make the leak happens, the supplier has to have a state "referral on update" and referrals in the mapping tree.  It's realized in the hub.

Sample mapping tree entry of Hub:
dn: cn=dc\3Dexample\2Cdc\3Dcom,cn=mapping tree,cn=config
objectClass: top
objectClass: extensibleObject
objectClass: nsMappingTree
cn: dc=example,dc=com
cn: "dc=example,dc=com"
nsslapd-state: referral on update
nsslapd-backend: userRoot
nsslapd-referral: ldap://M1:PORT1/dc%3Dexample%2Cdc%3Dcom
nsslapd-referral: ldap://M2:PORT2/dc%3Dexample%2Cdc%3Dcom

1. Since each leak is not big, it'd be easier to verify with valgrind.
   Install ds-replication-debuginfo package.
   Start Consumer with valgrind

2. Run consumer initialization on M1 then Hub

3. Do some modifications on M1 or M2.

4. Stop Consumer.

5. Check the output from valgrind.
   If it does not contain this stacktrace, the bug is verified.
==18922== 259 bytes in 7 blocks are definitely lost in loss record 1,416 of 1,846
==18922==    at 0x4A0515D: malloc (vg_replace_malloc.c:195)
==18922==    by 0x3253C2B62E: ber_get_stringa (decode.c:284)
==18922==    by 0x3253C2C033: ber_scanf (decode.c:584)
==18922==    by 0x8F8599B: decode_startrepl_extop (repl_extop.c:394)
==18922==    by 0x8F85FA2: multimaster_extop_StartNSDS50ReplicationRequest (repl_extop.c:642)
==18922==    by 0x4CA24F6: plugin_call_exop_plugins (plugin.c:452)
==18922==    by 0x41C031: do_extended (extendop.c:377)
==18922==    by 0x413C53: connection_dispatch_operation (connection.c:641)
==18922==    by 0x4152D3: connection_threadmain (connection.c:2328)
==18922==    by 0x3262429632: _pt_root (ptthread.c:187)
==18922==    by 0x3252807760: start_thread (pthread_create.c:301)
==18922==    by 0x32520E098C: clone (clone.S:115)

Comment 4 Noriko Hosoi 2011-06-03 17:08:06 UTC
Reviewed by Rich (Thank you!!)

Pushed to master.
$ git merge 709826
Updating b4e6d8b..5a6518b
Fast-forward
 ldap/servers/plugins/replication/repl_extop.c |    4 ++--
 1 files changed, 2 insertions(+), 2 deletions(-)

$ git push
Counting objects: 13, done.
Delta compression using up to 4 threads.
Compressing objects: 100% (7/7), done.
Writing objects: 100% (7/7), 868 bytes, done.
Total 7 (delta 5), reused 0 (delta 0)
To ssh://git.fedorahosted.org/git/389/ds.git
   b4e6d8b..5a6518b  master -> master

RHEL-6 branch: cherry-picked the commit and pushed to RHEL-6.
commit 5a6518b567f4be6ee6ec2171461256842a8ae3e6
Author: Noriko Hosoi <nhosoi>
Date:   Thu Jun 2 18:00:46 2011 -0700

$ git cherry-pick 5a6518b567f4be6ee6ec2171461256842a8ae3e6
[RHEL-6 208b0bb] Bug 709826 - Memory leak: when extra referrals configured
 1 files changed, 2 insertions(+), 2 deletions(-)

$ git cherry-pick 5a6518b567f4be6ee6ec2171461256842a8ae3e6
[RHEL-6 208b0bb] Bug 709826 - Memory leak: when extra referrals configured
 1 files changed, 2 insertions(+), 2 deletions(-)
[nhosoi@kiki ldapserver 1496]$ git push redhat RHEL-6
Counting objects: 13, done.
Delta compression using up to 4 threads.
Compressing objects: 100% (7/7), done.
Writing objects: 100% (7/7), 865 bytes, done.
Total 7 (delta 5), reused 0 (delta 0)
To ssh://git.engineering.redhat.com/srv/git/users/rmeggins/ds.git
   e964e1a..208b0bb  RHEL-6 -> RHEL-6


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