54189 - PR_Cleanup and destructors

Status: RESOLVED WONTFIX

(NSPR :: NSPR, defect, P3)

Description

[Jon Smirl]

I'm working on cleaning up memory leaks in XPCOM shutdown. A big source of 
leaks in XPCOM is that it isn't calling PR_Cleanup. The problem is thread local 
storage destructors. When XPCOM is shutting down it unloads dll code by calling 
PR_Unload. The code implmenting the thread local storage destructor may be in 
the dll that was just unloaded. To get around this XPCOMShutdown just didn't 
call PR_Cleanup.

One solution to fix this would be make _PR_DestroyThreadPrivate a public 
function. It would need to be modified to make it null out the destructor 
pointers after calling them since PR_Clean will call it again.

XPCOM shutdown would then look like this:
PR_DestroyThreadPrivate
do the XPCOM calls necessary to unload the dlls
PR_Cleanup

You can't unload the DLLs after calling PR_Cleanup because the XPCOM unload 
routinues need NSPR active.

Comment 1

[Wan-Teh Chang]

You can only call PR_Cleanup() (at the end of
the main() function) if none of the functions
invoked after the main() function returns (e.g.,
C++ static destructors, C atexit() handlers,
POSIX thread specific data destructors, Windows
DllMain() process detach handlers, etc.) will
call NSPR.

As NSPR gets used in more and more situations,
it is getting difficult to guarantee that NSPR
functions won't ever be called after PR_Cleanup()
is called.  So PR_Cleanup() has become a function
that can only be used in a controlled environment,
such as a small test case.  In a complicated
program like Mozilla, you can't use PR_Cleanup().

Given this, do you still want this bug to be fixed?

Comment 2

[Jon Smirl]

This was discovered in the standalone build of XPCOM. The only component having 
problems with this was XPConnect which has been fixed (bug 54264) to work 
around the problem. 

An architectually clean version of PR_Cleanup should be implemented in the 
future but we can work around it for now. 

I'm running stress tests on NSPR/XPCOM/JSEng in a web server environment and 
the combination is leaking about 2MB/minute. When these test exits they dump 
1000s of memory leaks. It is much each just to eliminate all the leaks 
(including static ones) than it is to sort through lists of a thousand leaks 
trying to decide which ones are important and which aren't after each run. The 
address of the leaks move depending on the test cases and the same leaks need 
to be checked over and over if they're not fixed.

You could fix this and we can test it XPCOM standalone without impacting the 
browser.

Comment 3

[Asa Dotzler [:asa]]

setting Jon Smirl's bugs to New

Comment 4

[Wan-Teh Chang]

Jon,

I haven't spent any time on this bug until today.  Sorry
about that.

I read the bug description a few times to recall what the
issue was.  I am going to mark it WONTFIX.  Reasons below.

You wrote:
> One solution to fix this would be make
> _PR_DestroyThreadPrivate a public  function. It would
> need to be modified to make it null out the destructor
> pointers after calling them since PR_Clean will call it
> again.

This is not necessary.  XPCOM shutdown just needs to
invoke the TPD destructors itself and the destructors
or XPCOM should null out the TPD pointers.
_PR_DestroyThreadPrivate won't invoke a TPD destructor
unless the TPD pointer is not null.

> You can't unload the DLLs after calling PR_Cleanup
> because the XPCOM unload routinues need NSPR active.

You can get around this restriction by using system
calls to load and unload the DLLs.

In any case, it is XPCOM's responsibility to destroy
any TPD that depend on the DLLs before unloading the
DLLs.  The key to prevent PR_Cleanup from invoking
the destructors again is for XPCOM or the destructors
to null out the TPD pointers.