45068 - NS_ShutdownXPCOM followed by NS_InitXPCOM leaks and crashes

Status: RESOLVED WONTFIX

(Core :: XPCOM, enhancement, P4)

Description

[Jon Smirl]

From Bugzilla Helper:
User-Agent: Mozilla/4.0 (compatible; MSIE 5.01; Windows 95)
BuildID:    CVS

NS_ShutdownXPCOM followed by NS_InitXPCOM. Right now I can shutdown ok but when 
I reinit XPCOM I GPF on  NS_ADDREF(thread) in GetIThread(). It looks like self-
>privateData in PR_GetThreadPrivate is pointing to memory that was freed in 
XPCOM shutdown.

Reproducible: Always
Steps to Reproduce:
NS_ShutdownXPCOM followed by NS_InitXPCOM

Comment 1

[Jon Smirl]

Here's the fix, a dangling point is left in the thread local storage after the 
thread object has been deleted.

Change the shutdown function in xpcom/threads/nsThread.cpp...

void 
nsThread::Shutdown()
{
    if (gMainThread) {
        // XXX nspr doesn't seem to be calling the main thread's destructor
        // callback, so let's help it out:
        nsThread::Exit(NS_STATIC_CAST(nsThread*, gMainThread));

				// this will cause gMainThread to be released
				PR_SetThreadPrivate
(nsThread::kIThreadSelfIndex, NULL);
				// don't keep a dangling reference
				gMainThread = NULL;

//      nsrefcnt cnt;
//      NS_RELEASE2(gMainThread, cnt);
//      NS_WARN_IF_FALSE(cnt == 0, "Main thread being held past XPCOM 
shutdown.");
    }
}

fix the destructor in xpcom/ds/nsDirectoryService.cpp...

nsDirectoryService::~nsDirectoryService()
{
  leave everything as is, add:
  mService = NULL;
}

Comment 2

[Asa Dotzler [:asa]]

setting bug status to New and adding patch keyword

Comment 3

[Adam Lock]

Adding a dependency between this bug and 38671.

Comment 4

[locka]

Added embed keyword

Comment 5

[locka]

Added some people to the CC for this bug

Comment 6

[Ray Whitmer]

Can we, for now, just eliminate the actual shutdown and second startup code 
except in  debug environments or environments where people are looking for 
leaks?

Comment 7

[Doug Turner (:dougt)]

ray, this is an embedded use of xpcom.  So that we do not leak all over the 
place, we need to allow for a shutdown - startup.  

Lets address the problems and not bandaide the problem.

Comment 8

[Ray Whitmer]

OK.  My answer is to remove the function NS_ShutdownXPCOM, and have a single 
function that can be called anywhere to track outstanding leaks.  Actual 
shutdown of XPCOM will not happen soon.  I will fix this in M18 if you agree 
with this solution.  Otherwise, it becomes future.

Comment 9

[Adam Lock]

This is not acceptable for componentization because object based on XPCOM will 
leak a huge chunk of memory into their host's address space with no way of 
cleaning it up. Basically, as far as the host process is concerned, it creates 
some component (GTK, ActiveX, whatever) and when it decides to delete it, all 
the memory that the component used should go too. If we delete NS_TermXPCOM then 
that's obviously impossible.

If you're concerned about services and marshalled pointers still being alive and 
refcounted when NS_TermXPCOM is called, then we need to establish some rules of 
behaviour for these objects. Perhaps services should be singletons for instance 
that ignore calls to AddRef and *always* go away when the Term happens?  Anyone 
who tries to call them after is in error and a crash is their just desserts. Or 
maybe services/marshalled pointers should return NS_ERROR_XPCOM_NOT_INITIALISED 
if they are called when XPCOM is not initialised?

Comment 10

[Ray Whitmer]

Perhaps we could schedule meetings this week, starting today, identifying the 
real requirements you are trying to solve and all the XPCOM changes that are 
required for this enhancement.

Comment 11

[Warren Harris]

Assigning rest of xpcom stuff to myself (from Ray).

Comment 12

[Asa Dotzler [:asa]]

reasigning warren bugs to default component owners.

Comment 13

[Doug Turner (:dougt)]

reassign all kandrot xpcom bug.

Comment 14

[Doug Turner (:dougt)]

moving out based on my workload.  Yell, if you have a problem.

Comment 15

[Judson Valeski]

per discussion w/ dougt, topembed minusing this bug.

Comment 16

[Jesse Ruderman]

bsmedberg, can you tell whether this bug is (still) valid?

Comment 17

[Benjamin Smedberg]

Yes, I'd like this to remain open.

Comment 18

[Benjamin Smedberg]

I'm now going to reverse my 2009-self and close this, it's not something we are actually doing or planning on doing.