398401 - Memory leak in PKIX init.

Status: RESOLVED FIXED

(NSS :: Libraries, defect, P1)

Description

[Slavomir Katuscak]

I tested memory leaks with PKIX init + shutdown process (see bug 391774 where shutdown was not called). Also with shutdown there is still one leak:

Block in use (biu):
Found block of size 88 bytes at address 0x8180a00 (2.65% of total)
At time of allocation, the call stack was:
    [1] calloc() at 0xb4d308a0
    [2] PR_Calloc() at line 475 in "prmem.c"
    [3] PR_NewLock() at line 174 in "ptsynch.c"
    [4] PKIX_PL_Initialize() at line 114 in "pkix_pl_lifecycle.c"
    [5] PKIX_Initialize() at line 124 in "pkix_lifecycle.c"
    [6] nss_Init() at line 523 in "nssinit.c"
    [7] NSS_Initialize() at line 614 in "nssinit.c"
    [8] main() at line 1915 in "selfserv.c"

Found in selfserv, strsclnt, ocspclnt.

Comment 1

[Slavomir Katuscak]

Stacks (there are 2, differs a bit on platforms) removed from bug 391774 and assigned to this bug in ignore list.

Comment 2

[Alexei Volkov]

Attachment: Patch v1

Destroy lock at shutdown. Also, to not return error if library is already initialized.
Code clean up.

Comment 3

[Nelson Bolyard (seldom reads bugmail)]

Comment on attachment 283566 [details] [diff] [review]
Patch v1

The changes to the executable code are OK, but the changes make the block
comments that precede them be completely wrong.  The comments need to be fixed, too.

>@@ -109,21 +109,22 @@ PKIX_Initialize(
>         /*
>          * If we are called a second time other than in the situation handled
>          * above, we return a statically allocated error. Our technique works
>          * most of the time, but may not work if multiple threads call this
>          * function simultaneously. However, the function's documentation
>          * makes it clear that this is prohibited, so it's not our
>          * responsibility.
>          */
> 
>         if (pkixIsInitialized){
>-                return (PKIX_ALLOC_ERROR());
>+                /* Already initialized */
>+                PKIX_RETURN(LIFECYCLE);
>         }


>@@ -102,24 +102,28 @@ PKIX_PL_Initialize(
> 
>         /*
>          * This function can only be called once. If it has already been
>          * called, we return a statically allocated error. Our technique works
>          * most of the time, but may not work if multiple threads call this
>          * function simultaneously. However, the function's documentation
>          * makes it clear that this is prohibited, so it's not our
>          * responsibility.
>          */
> 
>-        if (pkix_pl_initialized) return (PKIX_ALLOC_ERROR());
>+        if (pkix_pl_initialized) {
>+            PKIX_RETURN(OBJECT);
>+        }

Comment 4

[Alexei Volkov]

Attachment: Patch v2. Changes in comment blocks.

Comment 5

[Nelson Bolyard (seldom reads bugmail)]

Comment on attachment 283729 [details] [diff] [review]
Patch v2. Changes in comment blocks.

Strike all the following text from both places.

>                                                  Our technique works
>          * most of the time, but may not work if multiple threads call this
>          * function simultaneously. However, the function's documentation
>          * makes it clear that this is prohibited, so it's not our
>          * responsibility.

Comment 6

[Nelson Bolyard (seldom reads bugmail)]

Comment on attachment 283729 [details] [diff] [review]
Patch v2. Changes in comment blocks.

Alexei, When you make the changes described in my previous comment, then r+

Comment 7

[Alexei Volkov]

patch has been integrated with fixed comment blocks.