The default bug view has changed. See this FAQ.

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.