XPConnect need to clean up it's TLS before it is unloaded from memory. This only applies to primordial thread. A simple fix is attached. In the current code if DLL unloading is enabled XPconnect can be unloaded while NSPR still has a destructor callback. Clearing TLS will prevent the destructor callback from being called.

I'm not byuibg this. Your patch has problems. - If called from CleanupAllThreads it will repeatedly set the TLS for the current thread. - It has no check for gTLSIndex != BAD_TLS_INDEX. Luckily nspr will catch that. - mMost importantly, it will cause a double delete on the object in question when Cleanup is called from ~xpcPerThreadData. See: http://lxr.mozilla.org/seamonkey/source/nsprpub/pr/src/threads/prtpd.c#150 xpc_ThreadDataDtorCB is necessary for cases where lots of threads are created and destroyed. Otherwise we leak this per-thread data. I'm amazed that you are trying to support dll unloading. We have no code that is prepared for that. xpconnect was never intended to support being unloaded. If this were to be supported why would it only apply to the primordial thread? Because that is the only place you see it crash in your application? This problem could just as easily happen if anpother thread were to finish after unloading xpconnect. No? How can you possibly safely unload xpconnect? Are you also killing off all of xpcom and everything associated with it? But leaving nspr running in the process?

At the point where I call XPCOM shutdown my app has ensured all of the other thread have exited. When the other threads exit NSPR automatically catches their exit and calls their TLS destructors. Only the main thread has a shutdown problem. Isn't it an error to be calling XPCOMShutdown if other threads are active? I'm only enabling DLL unloading as part of the shutdown process. A bunch of leaks are associated with the DLL wrappers. When I exit my app in a simple test case I have over 500 leaks from XPCOM. It is very time consuming teying to sort out which ones I'm causing vs ones built into XPCOM. The simplest solution is just to fix them all. Check out bug 53933 for the changes to XPCOM shutdown

The patch adds PR_SetThreadPrivate(gTLSIndex, nsnull) to the end of xpcPerThreadData::CleanupAllThreads() not Cleanup(). CleanupAllThreads is only called from ~nsXPConnect which is only destroyed during XPCOM shutdown on the primary thread. This patch has no effect on anything but the primary thread. Without this patch the xpcPerThreadData for the primary thread is one of the leaks I'm trying to clean up. xpc_ThreadDataDtorCB is not called for the primary thread (it is called for all other threads) because XPCOM shutdown didn't call PR_Cleanup. When I added PR_Cleanup to XPCOM shutdown and enabled DLL unloading, I got a GPF on xpc_ThreadDataDtorCB. The easiest solution to this was to clear the TLS from CleanupAllThreads thus preventing xpc_ThreadDataDtorCB from being called form PR_Cleanup. Without this patch PR_Cleanup needs to be broken into a two phase process see bug 54189 I didn't think it was likely that NSPR would get changed so I fixed XPConnect instead.

Duh, Sorry I misread the patch and jumped to conclusions. I agree with your reasoning. I'm glad to see you did this outside the locked block. One little change... I'll do the gTLSIndex != BAD_TLS_INDEX check first to maintian local consistency. Thanks for setting me straight. I'll attach my slightly different patch and get this checked into the trunk soonish.

Looks good to me.

Fix checked in to trunk. Thanks!

Marking Verified -