Closed Bug 19183 Opened 20 years ago Closed 20 years ago

Crash on shutdown... Service Manager was deleted too soon.

Categories

(Core :: XPCOM, defect, P2, major)

x86
Windows NT
defect

Tracking

()

RESOLVED FIXED

People

(Reporter: rpotts, Assigned: dp)

References

Details

(Keywords: crash)

Attachments

(1 file)

I just hot this crash when exiting mozilla...

It looks like a DLL is unloading a service in its shutdown routine :-(

It looks like nsTextTransformer::Shutdown() requires the ServiceManager.
Unfortunately, it has already been released when Shutdown() is called :-(

Should the ServiceManager be Released *before* ComponentManager is shut down?

Below is the stack trace:
==========================

nsServiceManager::ReleaseService(const nsID & {...}, nsISupports * 0x011d15f0,
nsIShutdownListener * 0x00000000) line 492 + 26 bytes
nsTextTransformer::Shutdown() line 103 + 19 bytes
nsLayoutModule::Shutdown() line 262
nsLayoutModule::~nsLayoutModule() line 172
nsLayoutModule::`scalar deleting destructor'(unsigned int 0x00000001) + 15 bytes
nsLayoutModule::Release(nsLayoutModule * const 0x011bcfe0) line 174 + 134 bytes
nsDll::Shutdown() line 394 + 18 bytes
nsFreeLibrary(nsDll * 0x00c13560, nsIServiceManager * 0x00000000, int
0x00000003) line 353
nsFreeLibraryEnum(nsHashKey * 0x00c13450, void * 0x00c13560, void * 0x0012fdf4)
line 401 + 64 bytes
_hashEnumerate(PLHashEntry * 0x00c13410, int 0x00000005, void * 0x0012fddc) line
89 + 26 bytes
PL_HashTableEnumerateEntries(PLHashTable * 0x00c056a0, int (PLHashEntry *, int,
void *)* 0x10019760 _hashEnumerate(PLHashEntry *, int, void *), void *
0x0012fddc) line 368 + 15 bytes
nsHashtable::Enumerate(int (nsHashKey *, void *, void *)* 0x10046cb0
nsFreeLibraryEnum(nsHashKey *, void *, void *), void * 0x0012fdf4) line 218 + 20
bytes
nsNativeComponentLoader::UnloadAll(nsNativeComponentLoader * const 0x00c05c70,
int 0x00000003) line 883
nsComponentManagerImpl::UnloadLibraries(nsIServiceManager * 0x00000000, int
0x00000003) line 1949 + 22 bytes
nsComponentManagerImpl::Shutdown() line 272
NS_ShutdownXPCOM(nsIServiceManager * 0x00000000) line 539 + 11 bytes
main(int 0x00000001, char * * 0x00c04900) line 699 + 8 bytes
mainCRTStartup() line 338 + 17 bytes
KERNEL32! 77f
Severity: normal → major
Priority: P3 → P2
Target Milestone: M12
Status: NEW → ASSIGNED
mmh! That is ok for a dll to access the servicemanager on shutdown. Got to
see...
I dont get the crash at all. ReleaseService() is obsolete; all it does is
NS_RELEASE anyway. So I dont know why this would ever be a problem. Are you
still seeing this ?
No I haven't been seeing this crash, but I forget what I did to cause the
nsTextTransformer to get loaded...

I still think that the code is rather dubious...  The call to NS_RELEASE2() will
only null out the mGlobalServiceManager if the refcount is zero.  If anyone else
is holding a reference, mGlobalServiceManager will not be nulled.

However, if those other references are released during the shutdown process,
then mGlobalServiceManager will be a bogus pointer and calls through
nsServiceManager::xxx() will crash...
Status: ASSIGNED → RESOLVED
Closed: 20 years ago
Resolution: --- → WORKSFORME
Rick, it looks ok. The fact that we are hitting the release of the service
manager means nonone is holding on to it.
Exactly. No one is holding onto it, but someone could still call though the
*static* nsServiceManager:xxx() functions which would *try* to use
mGlobalServiceManager which is now bogus!
Status: RESOLVED → REOPENED
I think now I understand where you are getting at. Let me try summarizing.

NS_RELEASE2(mGlobalServiceManager, cnt) will set mGlobalServiceManager to NULL
only after the completion of the release. So during the release
mGlobalServiceManager holds its pointer value. And the question you ask is what
if someone calls back into the nsServiceManager:: during the servicemanager
release. Would we be ok as this would access mGlobalServiceManager ?

One point though: mglobalservicemanager wont be garbage at any point. It will be
either NULL or the right object that hasn't been completly deleted.

I think we would be ok. But it is better to avoid this case. Here is what I
think I will do. before calling release on the mGlobalServiceManager, we null
the mGlobalServiceManager out.

tmpServMgr = mGlobalServiceManager;
mGlobalServiceManager = NULL;
NS_RELEASE2(tmpServMgr, cnt);
Status: REOPENED → ASSIGNED
Resolution: WORKSFORME → ---
Clearing WORKSFORME resolution due to reopen.
*** Bug 20596 has been marked as a duplicate of this bug. ***
Can one of your seeing the problem try the patch out.

Index: nsXPComInit.cpp
===================================================================
RCS file: /cvsroot/mozilla/xpcom/build/nsXPComInit.cpp,v
retrieving revision 1.49
diff -c -r1.49 nsXPComInit.cpp
*** nsXPComInit.cpp	1999/11/30 04:50:26	1.49
--- nsXPComInit.cpp	1999/12/03 19:03:31
***************
*** 528,534 ****
      // here again:

      NS_IF_RELEASE(servMgr);
!     NS_RELEASE2(nsServiceManager::mGlobalServiceManager, cnt);
      NS_ASSERTION(cnt == 0, "Service Manager being held past XPCOM shutdown.");

      // Release the global case converter
--- 528,541 ----
      // here again:

      NS_IF_RELEASE(servMgr);
!
!     // Dont use COMPtr here. We want to delete the global service manager
!     // and before that make the global serivce manager NULL so that while
!     // deletion is in progress, no one will be able to use the global
!     // servicemanager.
!     nsIServiceManager *tmpServiceManager =
nsServiceManager::mGlobalServiceManager;
!     nsServiceManager::mGlobalServiceManager = NULL;
!     NS_RELEASE2(tmpServiceManager, cnt);
      NS_ASSERTION(cnt == 0, "Service Manager being held past XPCOM shutdown.");

      // Release the global case converter
The crash is fixed now (I found a reproducible case). But I get the following
assertions:

"Service Manager being held past XPCOM shutdown."

NTDLL! 77f7629c()
nsDebug::Assertion(const char * 0x10090cf8, const char * 0x10090cec, const char
* 0x10090cbc, int 538) line 284 + 13 bytes
NS_ShutdownXPCOM(nsIServiceManager * 0x00000000) line 538 + 65 bytes
main(int 1, char * * 0x01004e00) line 713 + 8 bytes
mainCRTStartup() line 338 + 17 bytes
KERNEL32! 77f1ba06()

NS_ERROR("Setting main thread twice?");

NTDLL! 77f7629c()
nsDebug::Error(const char * 0x1009d8a8, const char * 0x1009d878, int 309) line
309 + 13 bytes
nsIThread::SetMainThread() line 309 + 20 bytes
NS_InitXPCOM(nsIServiceManager * * 0x00000000, nsFileSpec * 0x00000000,
nsFileSpec * 0x00000000) line 182 + 5 bytes
nsServiceManager::GetGlobalServiceManager(nsIServiceManager * * 0x0012fb7c) line
474 + 11 bytes
nsServiceManager::GetService(const nsID & {...}, const nsID & {...}, nsISupports
* * 0x0012fba4, nsIShutdownListener * 0x00000000) line 498 + 9 bytes
nsCharsetMenu::Done() line 232 + 22 bytes
nsCharsetMenu::~nsCharsetMenu() line 186
nsCharsetMenu::`scalar deleting destructor'(unsigned int 1) + 15 bytes
nsCharsetMenu::Release(nsCharsetMenu * const 0x02df3c60) line 165 + 134 bytes
DeleteEntry(nsHashKey * 0x02df4ae0, void * 0x02df32c0, void * 0x00000000) line
213 + 18 bytes
_hashEnumerateRemove(PLHashEntry * 0x02df4aa0, int 32, void * 0x0012fc54) line
227 + 26 bytes
PL_HashTableEnumerateEntries(PLHashTable * 0x01004980, int (PLHashEntry *, int,
void *)* 0x1001bab0 _hashEnumerateRemove(PLHashEntry *, int, void *), void *
0x0012fc54) line 368 + 15 bytes
nsHashtable::Reset(int (nsHashKey *, void *, void *)* 0x100469b0
DeleteEntry(nsHashKey *, void *, void *), void * 0x00000000) line 243 + 20 bytes
nsObjectHashtable::Reset() line 344
nsObjectHashtable::~nsObjectHashtable() line 310
nsObjectHashtable::`scalar deleting destructor'(unsigned int 1) + 15 bytes
nsServiceManagerImpl::~nsServiceManagerImpl() line 240 + 31 bytes
nsServiceManagerImpl::`scalar deleting destructor'(unsigned int 1) + 15 bytes
nsServiceManagerImpl::Release(nsServiceManagerImpl * const 0x01004a20) line 249
+ 132 bytes
nsVCardFactory::~nsVCardFactory() line 83 + 27 bytes
nsVCardFactory::`scalar deleting destructor'(unsigned int 1) + 15 bytes
nsVCardFactory::Release(nsVCardFactory * const 0x03fea200) line 111 + 131 bytes
nsCOMPtr<nsIFactory>::assign_assuming_AddRef(nsIFactory * 0x00000000) line 416
nsCOMPtr<nsIFactory>::assign_with_AddRef(nsISupports * 0x00000000) line 761
nsCOMPtr<nsIFactory>::operator=(nsIFactory * 0x00000000) line 516
nsFactoryEntry::~nsFactoryEntry() line 160
nsFactoryEntry::`scalar deleting destructor'(unsigned int 1) + 15 bytes
nsFactoryEntry_Destroy(nsHashKey * 0x010204e0, void * 0x0103b2d0, void *
0x00000000) line 180 + 28 bytes
_hashEnumerateRemove(PLHashEntry * 0x010204a0, int 144, void * 0x0012fdc8) line
227 + 26 bytes
PL_HashTableEnumerateEntries(PLHashTable * 0x01004590, int (PLHashEntry *, int,
void *)* 0x1001bab0 _hashEnumerateRemove(PLHashEntry *, int, void *), void *
0x0012fdc8) line 368 + 15 bytes
nsHashtable::Reset(int (nsHashKey *, void *, void *)* 0x10040cf0
nsFactoryEntry_Destroy(nsHashKey *, void *, void *), void * 0x00000000) line 243
+ 20 bytes
nsObjectHashtable::Reset() line 344
nsObjectHashtable::~nsObjectHashtable() line 310
nsObjectHashtable::`scalar deleting destructor'(unsigned int 1) + 15 bytes
nsComponentManagerImpl::Shutdown() line 272 + 31 bytes
NS_ShutdownXPCOM(nsIServiceManager * 0x00000000) line 545 + 11 bytes
main(int 1, char * * 0x01004e00) line 713 + 8 bytes
mainCRTStartup() line 338 + 17 bytes
KE

nsDebug::Error(const char * 0x1009d8a8, const char * 0x1009d878, int 309) line
309 + 13 bytes
nsIThread::SetMainThread() line 309 + 20 bytes
NS_InitXPCOM(nsIServiceManager * * 0x00000000, nsFileSpec * 0x00000000,
nsFileSpec * 0x00000000) line 182 + 5 bytes
nsServiceManager::GetGlobalServiceManager(nsIServiceManager * * 0x0012fb18) line
474 + 11 bytes
nsServiceManager::GetService(const nsID & {...}, const nsID & {...}, nsISupports
* * 0x0012fb58, nsIShutdownListener * 0x00000000) line 498 + 9 bytes
nsGetServiceByCID::operator()(const nsID & {...}, void * * 0x0012fb58) line 39 +
22 bytes
nsCOMPtr<nsIAppShellService>::assign_from_helper(const nsCOMPtr_helper & {...},
const nsID & {...}) line 768 + 18 bytes
nsCOMPtr<nsIAppShellService>::nsCOMPtr<nsIAppShellService>(const nsCOMPtr_helper
& {...}) line 497
nsWebShellWindow::Close(nsWebShellWindow * const 0x023c67d0) line 421 + 30 bytes
nsAppShellService::~nsAppShellService() line 163
nsAppShellService::`scalar deleting destructor'(unsigned int 1) + 15 bytes
nsAppShellService::Release(nsAppShellService * const 0x01023560) line 176 + 134
bytes
DeleteEntry(nsHashKey * 0x01023460, void * 0x01023520, void * 0x00000000) line
213 + 18 bytes
_hashEnumerateRemove(PLHashEntry * 0x01023420, int 56, void * 0x0012fc54) line
227 + 26 bytes
PL_HashTableEnumerateEntries(PLHashTable * 0x01004980, int (PLHashEntry *, int,
void *)* 0x1001bab0 _hashEnumerateRemove(PLHashEntry *, int, void *), void *
0x0012fc54) line 368 + 15 bytes
nsHashtable::Reset(int (nsHashKey *, void *, void *)* 0x100469b0
DeleteEntry(nsHashKey *, void *, void *), void * 0x00000000) line 243 + 20 bytes
nsObjectHashtable::Reset() line 344
nsObjectHashtable::~nsObjectHashtable() line 310
nsObjectHashtable::`scalar deleting destructor'(unsigned int 1) + 15 bytes
nsServiceManagerImpl::~nsServiceManagerImpl() line 240 + 31 bytes
nsServiceManagerImpl::`scalar deleting destructor'(unsigned int 1) + 15 bytes
nsServiceManagerImpl::Release(nsServiceManagerImpl * const 0x01004a20) line 249
+ 132 bytes
nsVCardFactory::~nsVCardFactory() line 83 + 27 bytes
nsVCardFactory::`scalar deleting destructor'(unsigned int 1) + 15 bytes
nsVCardFactory::Release(nsVCardFactory * const 0x03fea200) line 111 + 131 bytes
nsCOMPtr<nsIFactory>::assign_assuming_AddRef(nsIFactory * 0x00000000) line 416
nsCOMPtr<nsIFactory>::assign_with_AddRef(nsISupports * 0x00000000) line 761
nsCOMPtr<nsIFactory>::operator=(nsIFactory * 0x00000000) line 516
nsFactoryEntry::~nsFactoryEntry() line 160
nsFactoryEntry::`scalar deleting destructor'(unsigned int 1) + 15 bytes
nsFactoryEntry_Destroy(nsHashKey * 0x010204e0, void * 0x0103b2d0, void *
0x00000000) line 180 + 28 bytes
_hashEnumerateRemove(PLHashEntry * 0x010204a0, int 144, void * 0x0012fdc8) line
227 + 26 bytes
PL_HashTableEnumerateEntries(PLHashTable * 0x01004590, int (PLHashEntry *, int,
void *)* 0x1001bab0 _hashEnumerateRemove(PLHashEntry *, int, void *), void *
0x0012fdc8) line 368 + 15 bytes
nsHashtable::Reset(int (nsHashKey *, void *, void *)* 0x10040cf0
nsFactoryEntry_Destroy(nsHashKey *, void *, void *), void * 0x00000000) line 243
+ 20 bytes
nsObjectHashtable::Reset() line 344
nsObjectHashtable::~nsObjectHashtable() line 310
nsObjectHashtable::`scalar deleting destructor'(unsigned int 1) + 15 bytes
nsComponentManagerImpl::Shutdown() line 272 + 31 bytes
NS_ShutdownXPCOM(nsIServiceManager * 0x00000000) line 545 + 11 bytes
main(int 1, char * * 0x01004e00) line 713 + 8 bytes
mainCRTStartup() line 338 + 17 bytes
KERNEL3

nsDebug::Error(const char * 0x1009d8a8, const char * 0x1009d878, int 309) line
309 + 13 bytes
nsIThread::SetMainThread() line 309 + 20 bytes
NS_InitXPCOM(nsIServiceManager * * 0x00000000, nsFileSpec * 0x00000000,
nsFileSpec * 0x00000000) line 182 + 5 bytes
nsServiceManager::GetGlobalServiceManager(nsIServiceManager * * 0x0012fa78) line
474 + 11 bytes
nsServiceManager::GetService(const nsID & {...}, const nsID & {...}, nsISupports
* * 0x0012fab8, nsIShutdownListener * 0x00000000) line 498 + 9 bytes
nsGetServiceByCID::operator()(const nsID & {...}, void * * 0x0012fab8) line 39 +
22 bytes
nsCOMPtr<nsIXPConnect>::assign_from_helper(const nsCOMPtr_helper & {...}, const
nsID & {...}) line 768 + 18 bytes
nsCOMPtr<nsIXPConnect>::nsCOMPtr<nsIXPConnect>(const nsCOMPtr_helper & {...})
line 497
nsJSContext::~nsJSContext() line 179 + 31 bytes
nsJSContext::`scalar deleting destructor'(unsigned int 1) + 15 bytes
nsJSContext::Release(nsJSContext * const 0x026ff960) line 186 + 134 bytes
nsWebShell::~nsWebShell() line 687 + 18 bytes
nsWebShell::`scalar deleting destructor'(unsigned int 1) + 15 bytes
nsWebShell::Release(nsWebShell * const 0x023c64b0) line 764 + 137 bytes
nsWebShellWindow::Close(nsWebShellWindow * const 0x023c67d0) line 435 + 18 bytes
nsAppShellService::~nsAppShellService() line 163
nsAppShellService::`scalar deleting destructor'(unsigned int 1) + 15 bytes
nsAppShellService::Release(nsAppShellService * const 0x01023560) line 176 + 134
bytes
DeleteEntry(nsHashKey * 0x01023460, void * 0x01023520, void * 0x00000000) line
213 + 18 bytes
_hashEnumerateRemove(PLHashEntry * 0x01023420, int 56, void * 0x0012fc54) line
227 + 26 bytes
PL_HashTableEnumerateEntries(PLHashTable * 0x01004980, int (PLHashEntry *, int,
void *)* 0x1001bab0 _hashEnumerateRemove(PLHashEntry *, int, void *), void *
0x0012fc54) line 368 + 15 bytes
nsHashtable::Reset(int (nsHashKey *, void *, void *)* 0x100469b0
DeleteEntry(nsHashKey *, void *, void *), void * 0x00000000) line 243 + 20 bytes
nsObjectHashtable::Reset() line 344
nsObjectHashtable::~nsObjectHashtable() line 310
nsObjectHashtable::`scalar deleting destructor'(unsigned int 1) + 15 bytes
nsServiceManagerImpl::~nsServiceManagerImpl() line 240 + 31 bytes
nsServiceManagerImpl::`scalar deleting destructor'(unsigned int 1) + 15 bytes
nsServiceManagerImpl::Release(nsServiceManagerImpl * const 0x01004a20) line 249
+ 132 bytes
nsVCardFactory::~nsVCardFactory() line 83 + 27 bytes
nsVCardFactory::`scalar deleting destructor'(unsigned int 1) + 15 bytes
nsVCardFactory::Release(nsVCardFactory * const 0x03fea200) line 111 + 131 bytes
nsCOMPtr<nsIFactory>::assign_assuming_AddRef(nsIFactory * 0x00000000) line 416
nsCOMPtr<nsIFactory>::assign_with_AddRef(nsISupports * 0x00000000) line 761
nsCOMPtr<nsIFactory>::operator=(nsIFactory * 0x00000000) line 516
nsFactoryEntry::~nsFactoryEntry() line 160
nsFactoryEntry::`scalar deleting destructor'(unsigned int 1) + 15 bytes
nsFactoryEntry_Destroy(nsHashKey * 0x010204e0, void * 0x0103b2d0, void *
0x00000000) line 180 + 28 bytes
_hashEnumerateRemove(PLHashEntry * 0x010204a0, int 144, void * 0x0012fdc8) line
227 + 26 bytes
PL_HashTableEnumerateEntries(PLHashTable * 0x01004590, int (PLHashEntry *, int,
void *)* 0x1001bab0 _hashEnumerateRemove(PLHashEntry *, int, void *), void *
0x0012fdc8) line 368 + 15 bytes
nsHashtable::Reset(int (nsHashKey *, void *, void *)* 0x10040cf0
nsFactoryEntry_Destroy(nsHashKey *, void *, void *), void * 0x00000000) line 243
+ 20 bytes
nsObjectHashtable::Reset() line 344
nsObjectHashtable::~nsObjectHashtable() line 310
nsObjectHashtable::`scalar deleting destructor'(unsigned int 1) + 15 bytes
nsComponentManagerImpl::Shutdown() line 272 + 31 bytes
NS_ShutdownXPCOM(nsIServiceManager * 0x00000000) line 545 + 11 bytes
main(int 1, char * * 0x01004e00) line 713 + 8 bytes
mainCRTStartup() line 338 + 17 bytes
KERNEL32! 77f1ba06()
mGlobalServiceManager *will* be garbage...  It is only cleared in
nsServiceManager::ShutdownServiceManager() which is *never* called!

So, when the service manager finally goes away, this *global* pointer is still
non-null.
Rick, I was counting on NS_RELEASE2() to set it 0.

#define NS_RELEASE2(_ptr,_rv)                                                \
  PR_BEGIN_MACRO                                                             \
    _rv = NS_LOG_RELEASE_CALL((_ptr), (_ptr)->Release(),__FILE__,__LINE__);  \
    if (0 == (_rv)) (_ptr) = 0;                                              \
  PR_END_MACRO

If the release returned an error, then there is a case where this pointer would
have been garbage. Is that what you were refering to.
Looks like ShutdownGlobalServiceManager is doing about the same thing as
NS_RELEASE2, but for some reason mGlobalServiceManager is public (!) and that's
why it's accessible from NS_ShutdownXPCOM. Ideally,
ShutdownGlobalServiceManager would be used instead.

The real problem in the stack traces from Putterman is that the destruction of
some of the services are indirectly causing the service manager to get
recreated. It looks like the code in
nsServiceManagerImpl::~nsServiceManagerImpl should be pulled out into a
Shutdown/Cleanup/Close method and called explicitly before the service
manager goes away. Or maybe the nsCharsetMenu::Done,d nsCharsetMenu::Done and
nsJSContext::~nsJSContext that want the service manager should be changed
(somehow) to not require it.

...I'm sure I remember fixing this exact problem once for rdf -- maybe the
change got lost.
Me too. I remember fixes for similar reinit happening many times. I think we
fixed a lot of the access to these via the nsIServiceManager object as opposed
to the global. Now the module conversion caused more of the static object to get
accessed.

I was thinking of adding a state variable and handling this. {NOT_INIT, INIT,
SHUTDOWN_HAPPENING, SHUTDOWN_DONE} and GetGlobalServiceManager() will obey this.

And the caller might have to be fixed too.
Putternam, Can someone explain how I can reproduce this.
this message caused the problem for me every time.  I just had to display it and
then when I shut down I would crash.  You could just add this to your mailbox.
I just realized it would probably be easier for me to send you the message, so I
will do that.
Is there a way to remove an attachment?  I just realized what I attached.
hey dp,

If you look closely at the NS_RELEASE2(...) macro it *only* zeros the pointer if
the refcount goes to 0.  The macro is a bit hard to read, but basically it says:
  rv = ptr->Release();
  if (rv == 0) ptr = nsnull;

Remember Release() does *not* return an error code, but the new refcount of the
component...

-- rick
Status: ASSIGNED → RESOLVED
Closed: 20 years ago20 years ago
Resolution: --- → FIXED
I fixed vcard to not hold servicemanager. Also, changed XPCOMShutdown to use

nsServiceManager::ShutdownGlobalServiceManager()
Blocks: 21564
Adding crash keyword
Keywords: crash
No longer blocks: 21564
You need to log in before you can comment on or make changes to this bug.