391774 - PKIX_Shutdown is not called by nssinit.c

Status: RESOLVED FIXED

(NSS :: Libraries, defect, P1)

Description

[David Baron :dbaron: (⌚️UTC-4, no longer working on Mozilla)]

This bug is filed based on the NSS_3_12_ALPHA1B tag, which is what is currently used by the trunk of Firefox, etc.

security/nss/lib/nss/nssinit.c currently calls PKIX_Initialize but not PKIX_Shutdown.  It seems that someplace in NSS_Shutdown should have the following code:

+    if (plContext) {
+        PKIX_Shutdown(plContext);
+        plContext = NULL;
+    }

This alone doesn't fix any memory leaks, but with improvements to PKIX_Shutdown I think it would fix some memory leaks.

Comment 1

[Nelson Bolyard (seldom reads bugmail)]

The fix for this bug should not be committed while it remains blocked by 
bug 391815, and bug 391815 remains unresolved.

Comment 2

[Alexei Volkov]

nss shutdown function now calls PKIX_Shutdown. The fix was committed with the patch for bug 395427. But still, PKIX_Shutdown is ifdef-ed awaiting a fix for bug  391815. So, will keep this bug open until ifdef for PKIX_Shutdown call is removed.   

Comment 3

[Alexei Volkov]

Attachment: remove ifdef for PKIX_Shutdown

Comment 4

[Nelson Bolyard (seldom reads bugmail)]

Comment on attachment 281745 [details] [diff] [review]
remove ifdef for PKIX_Shutdown

This patch is obviously correct, as far as it goes, but it also needs to remove from the file of ignored leak stacks any stacks that will be fixed by the removal of the ifdef.  It may or may not be possible to do that now, depending on whether all the leak stacks caused by PKIX initialization have been put into the ignored file already, or not.  They can't be removed until they are put in, of course.  
The next patch should have two parts:
1) the part for nssinit.c that removes the ifdefs (the present patch), and 
2) the patch to the "ignored" file.

Comment 5

[Slavomir Katuscak]

This leak is already detected by our tools, dependency on bug 391815 can be
removed.

Comment 6

[Nelson Bolyard (seldom reads bugmail)]

Comment on attachment 281745 [details] [diff] [review]
remove ifdef for PKIX_Shutdown

Please attach a patch that will remove the leak stacks that will be fixed by this patch.  I will give this patch and that one r+ at the same time.

Comment 7

[Alexei Volkov]

Attachment: ignore file patch

PKIX_Shutdown will be called from NSS_Shutdown with patch in attachment 281745 [details] [diff] [review] applied to the trunk.

This patch removes all one time initialization leaks from ignore file in the assumption that PKIX_Shutdown will free these memories blocks. But memory checking tinderbox should go orange, since PKIX_Shitdown currently does not take care of at least one real leak: 

  -*/main/NSS_Initialize/*/PKIX_Initialize/PKIX_PL_Initialize/PR_NewLock/**

Comment 8

[Nelson Bolyard (seldom reads bugmail)]

Comment on attachment 283055 [details] [diff] [review]
ignore file patch

Please do not remove any stacks that you know are not actually fixed by the first patch for this bug.

Comment 9

[Slavomir Katuscak]

Attachment: Patch v1.

I tested memory leaks with PKIX shutdown uncommented to find leaks which stays there. There was only one PKIX leak found (Alexei already mentioned it in comment 7), so I opened new bug for this (bug 398401) and reassigned stacks related to this bug to it in ignore list. 

Now we can uncomment PKIX shutdown + remove all stacks assigned to it from ignore list and close this bug. I prepared patch for this (replacing previous patches).

Comment 10

[Nelson Bolyard (seldom reads bugmail)]

Comment on attachment 283353 [details] [diff] [review]
Patch v1.

r=nelson
Alexei or Slavo, please commit this patch ASAP.

Comment 11

[Alexei Volkov]

/cvsroot/mozilla/security/nss/tests/memleak/ignored,v  <--  ignored
new revision: 1.27; previous revision: 1.26
/cvsroot/mozilla/security/nss/lib/nss/nssinit.c,v  <--  nssinit.c
new revision: 1.84; previous revision: 1.83