Memory leak in PKIX init.

RESOLVED FIXED in 3.12

Status

NSS
Libraries
P1
normal
RESOLVED FIXED
10 years ago
10 years ago

People

(Reporter: Slavomir Katuscak, Assigned: Alexei Volkov)

Tracking

Firefox Tracking Flags

(Not tracked)

Details

(Whiteboard: PKIX)

Attachments

(1 attachment, 1 obsolete attachment)

4.89 KB, patch
Nelson Bolyard (seldom reads bugmail)
: review+
Details | Diff | Splinter Review
(Reporter)

Description

10 years ago
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.
(Reporter)

Comment 1

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

Comment 2

10 years ago
Created attachment 283566 [details] [diff] [review]
Patch v1

Destroy lock at shutdown. Also, to not return error if library is already initialized.
Code clean up.
Attachment #283566 - Flags: review?(nelson)
(Assignee)

Updated

10 years ago
Priority: -- → P1
Whiteboard: PKIX
Target Milestone: --- → 3.12
Version: 3.12 → trunk
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);
>+        }
Attachment #283566 - Flags: review?(nelson) → review-
(Assignee)

Comment 4

10 years ago
Created attachment 283729 [details] [diff] [review]
Patch v2. Changes in comment blocks.
Attachment #283566 - Attachment is obsolete: true
Attachment #283729 - Flags: review?(nelson)
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.
Attachment #283729 - Flags: review?(nelson) → review-
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+
Attachment #283729 - Flags: review- → review+
(Assignee)

Comment 7

10 years ago
patch has been integrated with fixed comment blocks.
Status: NEW → RESOLVED
Last Resolved: 10 years ago
Resolution: --- → FIXED
You need to log in before you can comment on or make changes to this bug.