130954 - crash [@nsComponentManagerImpl::FreeServices] (this=0x0)

Status: RESOLVED FIXED

(Core :: XPCOM, defect)

Description

[timeless]

afaik nothing checks to verify that there is a gComponentManager.

[yes i know, at this point i'm already quite dead, but i don't need a core file telling me this]

console output:
###!!! ASSERTION: NS_InitXPCOM failed: 'NS_SUCCEEDED(rv)', file /home/timeless/mozilla/xpfe/bootstrap/nsAppRunner.cpp, line 1681
###!!! Break: at file /home/timeless/mozilla/xpfe/bootstrap/nsAppRunner.cpp, line 1681
###!!! ASSERTION: Setting main thread twice?: 'Error', file /home/timeless/mozilla/xpcom/threads/nsThread.cpp, line 401
###!!! Break: at file /home/timeless/mozilla/xpcom/threads/nsThread.cpp, line 401
###!!! ASSERTION: Setting main thread twice?: 'Error', file /home/timeless/mozilla/xpcom/threads/nsThread.cpp, line 401
###!!! Break: at file /home/timeless/mozilla/xpcom/threads/nsThread.cpp, line 401
###!!! ASSERTION: Setting main thread twice?: 'Error', file /home/timeless/mozilla/xpcom/threads/nsThread.cpp, line 401
###!!! Break: at file /home/timeless/mozilla/xpcom/threads/nsThread.cpp, line 401
###!!! ASSERTION: Setting main thread twice?: 'Error', file /home/timeless/mozilla/xpcom/threads/nsThread.cpp, line 401
###!!! Break: at file /home/timeless/mozilla/xpcom/threads/nsThread.cpp, line 401
###!!! ASSERTION: Setting main thread twice?: 'Error', file /home/timeless/mozilla/xpcom/threads/nsThread.cpp, line 401
###!!! Break: at file /home/timeless/mozilla/xpcom/threads/nsThread.cpp, line 401
###!!! ASSERTION: Setting main thread twice?: 'Error', file /home/timeless/mozilla/xpcom/threads/nsThread.cpp, line 401
###!!! Break: at file /home/timeless/mozilla/xpcom/threads/nsThread.cpp, line 401
###!!! ASSERTION: Setting main thread twice?: 'Error', file /home/timeless/mozilla/xpcom/threads/nsThread.cpp, line 401
###!!! Break: at file /home/timeless/mozilla/xpcom/threads/nsThread.cpp, line 401
###!!! ASSERTION: Setting main thread twice?: 'Error', file /home/timeless/mozilla/xpcom/threads/nsThread.cpp, line 401
###!!! Break: at file /home/timeless/mozilla/xpcom/threads/nsThread.cpp, line 401
###!!! ASSERTION: Setting main thread twice?: 'Error', file /home/timeless/mozilla/xpcom/threads/nsThread.cpp, line 401
###!!! Break: at file /home/timeless/mozilla/xpcom/threads/nsThread.cpp, line 401
Segmentation fault - core dumped
./run-mozilla.sh -g -d gdb51 ./mozilla-bin /root/coredumps/mozilla-bin.3087.core

#0  nsComponentManagerImpl::FreeServices (this=0x0) at /home/timeless/mozilla/xpcom/components/nsComponentManager.cpp:1918
#1  0x28268551 in NS_ShutdownXPCOM (servMgr=0x0) at /home/timeless/mozilla/xpcom/build/nsXPComInit.cpp:548
#2  0x0805cdaa in main (argc=1, argv=0xbfbff844) at /home/timeless/mozilla/xpfe/bootstrap/nsAppRunner.cpp:1706
#3  0x080569c5 in _start ()

#1  0x28268551 in NS_ShutdownXPCOM (servMgr=0x0) at /home/timeless/mozilla/xpcom/build/nsXPComInit.cpp:548
548         nsComponentManagerImpl::gComponentManager->FreeServices();
(gdb) l
543         // We may have AddRef'd for the caller of NS_InitXPCOM, so release it
544         // here again:
545         NS_IF_RELEASE(servMgr);
546
547         // Shutdown global servicemanager
548         nsComponentManagerImpl::gComponentManager->FreeServices();

Comment 1

[timeless]

Attachment: gracefully quit when there's no component manager

Comment 2

[David Baron :dbaron: (⌚️UTC-4, no longer working on Mozilla)]

Should we really be calling NS_ShutdownXPCOM if NS_InitXPCOM failed?

Comment 3

[timeless]

dbradley suggests that NS_InitXPCOM2 should clean up after itself and that we shouldn't call NS_ShutdownXPCOM

Comment 4

[Conrad Carlen (not reading bugmail)]

> dbradley suggests that NS_InitXPCOM2 should clean up after itself and that we
shouldn't call NS_ShutdownXPCOM

I would agree with that as a matter of principal. 

Comment 5

[Doug Turner (:dougt)]

what conrad said.  

Comment 6

[Benjamin Smedberg]

Ping->NEW  Is this still an issue?  This patch has not been checked in... is it
still valid?

Comment 7

[Doug Turner (:dougt)]

Comment on attachment 74112 [details] [diff] [review]
gracefully quit when there's no component manager

r=dougt.  this doesn't hurt anything.

Comment 8

[Doug Turner (:dougt)]

Attachment: Fixes the problem at the source

Comment 9

[timeless]

Comment on attachment 98075 [details] [diff] [review]
Fixes the problem at the source

>-  rv = NS_ShutdownXPCOM( NULL );
>+  if (NS_SUCCEEDED(rv)) 
>+    rv = NS_ShutdownXPCOM( NULL );
>   NS_ASSERTION(NS_SUCCEEDED(rv), "NS_ShutdownXPCOM failed");
this assertion is associated w/ the shutdown call. move it into a block {} for
r=timeless.

Comment 10

[Doug Turner (:dougt)]

Attachment: Fixes the problem at the source v.2

Comment 11

[Brendan Eich [:brendan]]

Comment on attachment 98515 [details] [diff] [review]
Fixes the problem at the source v.2

>@@ -1860,7 +1860,8 @@
>   // always exit in that case.
>   remoterv = HandleRemoteArguments(argc, argv, &argused);
>   if (argused) {
>-    NS_ShutdownXPCOM(NULL);
>+    if (NS_SUCCEEDED(rv)) // only call NS_ShutdownXPCOM if Init succeeded.
>+      NS_ShutdownXPCOM(NULL);
>     return remoterv;
>   }
> #endif
>@@ -1873,8 +1874,10 @@
>     NS_ASSERTION(NS_SUCCEEDED(rv), "DoOnShutdown failed");
>   }
> 
>-  rv = NS_ShutdownXPCOM( NULL );
>-  NS_ASSERTION(NS_SUCCEEDED(rv), "NS_ShutdownXPCOM failed");
>+  if (NS_SUCCEEDED(rv)) { // only call NS_ShutdownXPCOM if Init succeeded.
>+      rv = NS_ShutdownXPCOM( NULL );

How about fixing the bogo-spaces inside the parens?  The rest of the patch's
calls, if not all calls in the rest of this source file, don't use this style.

/be

>+      NS_ASSERTION(NS_SUCCEEDED(rv), "NS_ShutdownXPCOM failed");
>+  }
> 
>   return TranslateReturnValue(mainResult);
> }

Comment 12

[Doug Turner (:dougt)]

Attachment: Fixes the problem at the source v.2

Fixes brendan's nits.
"For the love of briano" NULL -> nsnull

Comment 13

[Brendan Eich [:brendan]]

Comment on attachment 98519 [details] [diff] [review]
 Fixes the problem at the source v.2

sr=[e-mail], thanks for cleaning this up.  Who should r=?

/be

Comment 14

[Doug Turner (:dougt)]

The file blames to just about everyone.  I randomly picked alec to review this.
 Could you also look at timeless patch (74112)

Comment 15

[timeless]

adding alecf to the cc list so that he can see the review request...

Comment 16

[Alec Flett]

Comment on attachment 98519 [details] [diff] [review]
 Fixes the problem at the source v.2

sr=alecf

Comment 17

[Doug Turner (:dougt)]

Checking in nsAppRunner.cpp;
/cvsroot/mozilla/xpfe/bootstrap/nsAppRunner.cpp,v  <--  nsAppRunner.cpp
new revision: 1.372; previous revision: 1.371
done


Also checked in null check:

Checking in nsXPComInit.cpp;
/cvsroot/mozilla/xpcom/build/nsXPComInit.cpp,v  <--  nsXPComInit.cpp
new revision: 1.156; previous revision: 1.155
done