Last Comment Bug 398401 - Memory leak in PKIX init.
: Memory leak in PKIX init.
Status: RESOLVED FIXED
PKIX
:
Product: NSS
Classification: Components
Component: Libraries (show other bugs)
: trunk
: All All
: P1 normal (vote)
: 3.12
Assigned To: Alexei Volkov
:
:
Mentors:
Depends on:
Blocks:
  Show dependency treegraph
 
Reported: 2007-10-03 06:49 PDT by Slavomir Katuscak
Modified: 2007-10-09 09:52 PDT (History)
2 users (show)
See Also:
Crash Signature:
(edit)
QA Whiteboard:
Iteration: ---
Points: ---


Attachments
Patch v1 (4.27 KB, patch)
2007-10-04 10:19 PDT, Alexei Volkov
nelson: review-
Details | Diff | Splinter Review
Patch v2. Changes in comment blocks. (4.89 KB, patch)
2007-10-05 11:13 PDT, Alexei Volkov
nelson: review+
Details | Diff | Splinter Review

Description Slavomir Katuscak 2007-10-03 06:49:02 PDT
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 2007-10-03 06:55:28 PDT
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 2007-10-04 10:19:15 PDT
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.
Comment 3 Nelson Bolyard (seldom reads bugmail) 2007-10-04 14:18:35 PDT
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 2007-10-05 11:13:22 PDT
Created attachment 283729 [details] [diff] [review]
Patch v2. Changes in comment blocks.
Comment 5 Nelson Bolyard (seldom reads bugmail) 2007-10-05 14:53:54 PDT
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) 2007-10-08 16:09:03 PDT
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 2007-10-09 09:52:27 PDT
patch has been integrated with fixed comment blocks.

Note You need to log in before you can comment on or make changes to this bug.