430544 - Solve the leak of PLArenas in NSS tools

Status: RESOLVED INACTIVE

(NSS :: Tools, defect, P5)

Description

[Wan-Teh Chang]

Since NSS uses PLArena's extensively, any NSS tool that calls PR_Cleanup should also call PL_ArenaFinish (right before the PR_Cleanup call) to destroy NSPR's internal PLArena freelist.

Comment 2

[Nelson Bolyard (seldom reads bugmail)]

Should we just make NSS_Shutdown call this function?

That would fix all the tools and customer programs, too, in one blow.
And if the customer programs are already calling it (fat chance!) it 
wouldn't hurt if it gets called twice, I think.

Comment 3

[Wan-Teh Chang]

NSS_Shutdown can't call PL_ArenaFinish or PR_Cleanup because
the application may continue to use PLArena or NSPR after
shutting down NSS.

We can add a SECU_Shutdown function that does
    NSS_Shutdown
    PL_ArenaFinish
    PR_Cleanup
or add a SECU_ShutdownNSPR function that does
    PL_ArenaFinish
    PR_Cleanup
to share the common shutdown sequence in the command-line
tools.

Comment 5

[Ben Boeckel]

If NSS can't clean them up, could/should NSPR clean up the arenas at the very least? I assume that if the arenas need NSPR, NSPR can clean them up safely. If not, it should probably be documented.

Comment 6

[Nelson Bolyard (seldom reads bugmail)]

I think you're suggesting that PR_Cleanup could call PL_ArenaFinish.
IIRC, the reason we don't do that is to avoid circular dependencies 
between NSPR's shared libraries. NSPR has 3 or 4 shared libs in all.
We let all the libs depend on libnspr4, but we don't let libnspr4 
depend on any of the others.  

Now, some ways to work around that might include:
a) having a way for the other libs to dynamically register shut down 
functions with libnspr, so that libnspr's PR_Cleanup can call those 
shutdown functions without being linked against those libs.
b) create Cleanup functions in the other libs, e.g. PL_Cleanup, that
call PR_Cleanup, and convert NSS's test programs to call PL_Cleanup
instead of PR_Cleanup.

Comment 7

[Hanno Boeck]

Attachment: certutil-arenafinish.diff (checked in)

Comment 8

[Wan-Teh Chang]

Comment on attachment 461053 [details] [diff] [review]
certutil-arenafinish.diff (checked in)

r=wtc.  I checked in this patch on the NSS trunk (NSS 3.13).

Checking in certutil.c;
/cvsroot/mozilla/security/nss/cmd/certutil/certutil.c,v  <--  certutil.c
new revision: 1.150; previous revision: 1.149
done