120651 - Leak in getting cert list

Status: RESOLVED FIXED

(NSS :: Libraries, defect, P1)

Description

[Rangan Sen]

Getting a list of CA certs by calling 'PK11_ListCerts' and 
subsequently invoking CERT_DestroyCertList is leaking a big chunk of 
memmory. In fact just executing a block like:

for(int j=0;j<20;j++){
   caList = PK11_ListCerts(PK11CertListCA, 0);
   CERT_DestroyCertList(caList);
}

increases around 300K of VM in each pass [and the total VM consumed 
steadily rises - never goes down]. This is probably not a cache related issue,
because repeating the same call increases VM consumption by almost the same
amount everytime.

Comment 1

[Wan-Teh Chang]

Assigned the bug to Bob.

Comment 2

[Ian McGreer]

At one time I had run this through purify and it was clean.  I'll try it again.

Comment 3

[Robert Relyea]

On the latest source, Purify shows not leaks on

Comment 4

[Robert Relyea]

The above comment should read "on the latest source, Purify shows no leaks in
the sample code fragment"

Comment 5

[Rangan Sen]

True, but running the code fragment in a debugger, and tracking the Virtual Mem
 usage in NT, it still shows an increment of VM usage by around 120K in each
pass. I am not sure why purify does not show it, but since the VM usage grows
pretty consistently and never seems to reach a steady value [even after 50
passes], there's definitely a leak somewhere. What's interesting is that the
increment of VM usage per pass has now come down from around 300K to 120K,
indicating some of the involved leaks have already got plugged, probably as a
side effect of other fixes.

Comment 6

[Robert Relyea]

Can you specify the EXACT steps you are going through to reproduce this problem.
I don't see that in my simple test program which just includes your code fragment.

Thanks.

Comment 7

[Rangan Sen]

I just set a breakpoint here:

for(int j=0;j<50;j++){
--->   caList = PK11_ListCerts(PK11CertListCA, 0);
   CERT_DestroyCertList(caList);
}

and open the NT Task Manager->Processes Tab, add VM Size in the display by
selecting it from view->select columns. Everytime it executes the loop, the VM
size grows by around 120K.

Comment 8

[Rangan Sen]

Bob, here's the test program - 

#include <security/nss.h>
#include <security/ssl.h>
#include <security/pk11func.h>
#include <security/cert.h>
#include <security/certt.h>
#include <security/certdb.h>
#include <security/nssb64.h>
#include <security/secmod.h>
#include <prlink.h>

main()
{
CERTCertList * caList;
int k;
char c;
caList = NULL;
NSS_InitReadWrite(".");


for(k=0;k<50;k++){
  caList = PK11_ListCerts(PK11CertListCA, 0);
  CERT_DestroyCertList(caList);
  printf("Loop %d Done",k);
  scanf("%c",&c);
}

}

Comment 9

[Robert Relyea]

OK this bug is at least related to the leaks found in 123081.

bob

Comment 10

[Robert Relyea]

The following patches reduce the memory leaks by 1/3. You need to apply all
three patches. I'm also going to check in changes which renames the .c files in
the built-in directory so they dont' conflict with the files in the ckfw
directory. The patches do not reflect those changes.

Comment 11

[Robert Relyea]

Attachment: Patch 1 (in mozilla/security/nss/lib/ckfw

Comment 12

[Robert Relyea]

Attachment: path2 (in mozilla/security/nss/lib/ckfw/builtins )

Note this patch also requires a regen of certdata.h

Comment 13

[Robert Relyea]

Attachment: Patch 3 (also in mozilla/security/nss/lib/ckfw/builtins)

These patches are in files that will also be renamed.

Comment 14

[Robert Relyea]

Attachment: Final patch for memory leaks.

1) Fix leaks in error paths (bfind.c -- found by inspection).
2) Don't allocate hash table data out of the arena. PL_Hash grows and shrinks
the hash buckets as necessary. In arenas they will just grow.
3) Don't allocate temparary locks out of the global instance arena pool.

This looks like we get down to '0' leaks (we are at least small enough not to
show measurable growth in the VM table after 200 runs.

Comment 15

[Robert Relyea]

See patch