398090 - Problems with Stan's pointer tracker code

Status: NEW

(NSS :: Libraries, defect, P5)

Description

[Nelson Bolyard (seldom reads bugmail)]

+++ This bug was initially created as a clone of Bug #370536 +++

I set out to fix the leak reported in bug 370536, and I discovered several
problems with Stan's pointer tracker code.  They were complicated enough
that I thought it best to just document them, disable the pointer tracker
code completely, and see about fixing them later.

1. Not one pointer tracker, but many.

Project Stan was all about eliminating "singletons", and have explicit 
instances of all objects.  So, there isn't (supposed to be) just one global
pointer tracker object, but many.  

nssPointerTracker_initialize takes an argument, the address of a handle to 
an nssPointerTracker object to be initialized.  There are 10 calls to this function, and they pass the addresses of 5 different handles.

asn1/asn1.c:71:  rv = nssPointerTracker_initialize(&decoder_pointer_tracker);
asn1/asn1.c:128:  rv = nssPointerTracker_initialize(&decoder_pointer_tracker);
asn1/asn1.c:151:  rv = nssPointerTracker_initialize(&encoder_pointer_tracker);
asn1/asn1.c:208:  rv = nssPointerTracker_initialize(&encoder_pointer_tracker);
base/arena.c:138:  rv = nssPointerTracker_initialize(&arena_pointer_tracker);
base/arena.c:195:  rv = nssPointerTracker_initialize(&arena_pointer_tracker);
pki1/atav.c:585:  rv = nssPointerTracker_initialize(&atav_pointer_tracker);
pki1/atav.c:643:  rv = nssPointerTracker_initialize(&atav_pointer_tracker);
pki1/oid.c:348:  rv = nssPointerTracker_initialize(&oid_pointer_tracker);
pki1/oid.c:522:  rv = nssPointerTracker_initialize(&oid_pointer_tracker);

2. There are NO calls to nssPointerTracker_finalize.  To finalize all five
of known handles, it would need to be called 5 times, once for each handle.

3. But nssPointerTracker_initialize internally uses a callonce function, 
so that no matter how many times it is called, it only initializes ONE 
pointer tracker object (handle).  The first caller wins, and all the other 
callers lose.  The other 4 handles are apparently never actually initialized.
As it happens, in NSS now, the winner is always this line:

base/arena.c:138:  rv = nssPointerTracker_initialize(&arena_pointer_tracker);

4. Apparently the other 4 tracker objects are never initialized, and so
none of the tracker code for those 4 objects has ever actually been used.
If we fix the implementation, so that those 4 tracker handles actually 
start to work, who knows what other problems will emerge?  This is the 
main reason I elected not to try to fix this bug for NSS 3.12.  

5. nss/lib/base/tracker.c has its own private implementation of 
PR_CallOnceWithArg, by the name of call_once.  It uses PR_CallOnce.
Why it has its own, instead of using NSPR's implementation is unknown. 
Obviously, since it uses PR_CallOnce, the reason cannot be to avoid 
using NSPR's implementation.  I suspect that it was implemented before
NSPR's PR_CallOnceWithArg was implemented (PR_CallOnce was implemented
LONG before PR_CallOnceWithArg) and was simply never converted to use
NSPR's PR_CallOnceWithArg when that was finally implemented.  


Potential solutions:

We can either 
a) stay with the idea of 5 separate pointer tracker objects, or else
b) collapse all 5 pointer tracker objects into a single one.

If we stay with multiple separate pointer tracker objects, then it will
be necessary to change the nssPointerTracker_initialize function signature
to include a pointer to the unique PRCallOnceType struct for each pointer
tracker object, so that each object can be individually initialized.  

It will also be necessary to arrange for each one of those tracker 
objects to be finalized.  One way to do that would be to register a 
finalize routine for each object by calling NSS_RegisterShutdown.

If we collapse all pointer trackers into a single one, we might be able
to leave the _initialize function signature alone, and have it output
the same handle value to all callers.   This is potentially a lot less
work, but I don't yet know if there are any issues with collapsing them
into a single set.