Comment 29 for bug 867424

Revision history for this message
In , Oliver-henshaw (oliver-henshaw) wrote :

Actually, comment #14 is accurate.Calling SmcCloseConnection from ~nsNativeAppSupportUnix should always be safe, as all shutdown phases will have completed by this point (in the current code)

I managed to understand the shutdown sequence a little better by attaching gdb and sprinkling some breakpoints around then doing a normal quit. I couldn't get much direct insight into quit-on-logout as just attaching a debugger seemed to slow things down enough that firefox would always SIGHUP, but from my newly-aqcquired understanding, the code paths *are* the same.

Breakpoint 7, nsObserverService::NotifyObservers (this=0x7f029b4d0330, aSubject=0x0, aTopic=0x7f02b12745a1 "profile-before-change", aSomeData=0x7f02b1592000 <nsXREDirProvider::DoShutdown()::kShutdownPersist> u"shutdown-persist") at /usr/src/debug/firefox-37.0.1/mozilla-release/xpcom/ds/nsObserverService.cpp:319
319 {
[Current thread is 1 (Thread 0x7f02b3d5f780 (LWP 8791))]
#0 nsObserverService::NotifyObservers (this=0x7f029b4d0330, aSubject=0x0, aTopic=0x7f02b12745a1 "profile-before-change", aSomeData=0x7f02b1592000 <nsXREDirProvider::DoShutdown()::kShutdownPersist> u"shutdown-persist") at /usr/src/debug/firefox-37.0.1/mozilla-release/xpcom/ds/nsObserverService.cpp:319
#1 0x00007f02b0ab1fe0 in nsXREDirProvider::DoShutdown (this=0x7fffeed02f88) at /usr/src/debug/firefox-37.0.1/mozilla-release/toolkit/xre/nsXREDirProvider.cpp:905
#2 0x00007f02b0aaa102 in ScopedXPCOMStartup::~ScopedXPCOMStartup (this=0x7f02b29ac7c0, __in_chrg=<optimized out>) at /usr/src/debug/firefox-37.0.1/mozilla-release/toolkit/xre/nsAppRunner.cpp:1324
#3 0x00007f02b0aafc63 in XREMain::XRE_main (this=0x7fffeed02f48, argc=<optimized out>, argv=<optimized out>, aAppData=<optimized out>) at /usr/src/debug/firefox-37.0.1/mozilla-release/toolkit/xre/nsAppRunner.cpp:4312
#4 0x00007f02b0aafeac in XRE_main (argc=1, argv=0x7fffeed04468, aAppData=0x7fffeed03158, aFlags=<optimized out>) at /usr/src/debug/firefox-37.0.1/mozilla-release/toolkit/xre/nsAppRunner.cpp:4505
#5 0x00000000004040fd in do_main (argc=argc@entry=1, argv=argv@entry=0x7fffeed04468, xreDirectory=0x7f02b2949780) at /usr/src/debug/firefox-37.0.1/mozilla-release/browser/app/nsBrowserApp.cpp:292
#6 0x000000000040389f in main (argc=1, argv=0x7fffeed04468) at /usr/src/debug/firefox-37.0.1/mozilla-release/browser/app/nsBrowserApp.cpp:661

i.e. profile-before-change (and web-workers-shutdown which happens just after) happens when toolkit/xre/nsAppRunner.cpp:XREMain::XRE_main destroys the ScopedXPCOMStartup object, just after exiting the event loop.

Breakpoint 3, nsNativeAppSupportUnix::~nsNativeAppSupportUnix (this=0x7f02b29e5460, __in_chrg=<optimized out>) at /usr/src/debug/firefox-37.0.1/mozilla-release/toolkit/xre/nsNativeAppSupportUnix.cpp:126
126 class nsNativeAppSupportUnix : public nsNativeAppSupportBase
[Current thread is 1 (Thread 0x7f02b3d5f780 (LWP 8791))]
#0 nsNativeAppSupportUnix::~nsNativeAppSupportUnix (this=0x7f02b29e5460, __in_chrg=<optimized out>) at /usr/src/debug/firefox-37.0.1/mozilla-release/toolkit/xre/nsNativeAppSupportUnix.cpp:126
#1 0x00007f02b0ab0e3e in nsNativeAppSupportBase::Release (this=0x7f02b29e5460) at /usr/src/debug/firefox-37.0.1/mozilla-release/toolkit/xre/nsNativeAppSupportBase.cpp:16
#2 0x00007f02b0aafebd in XRE_main (argc=1, argv=0x7fffeed04468, aAppData=0x7fffeed03158, aFlags=<optimized out>) at /usr/src/debug/firefox-37.0.1/mozilla-release/toolkit/xre/nsAppRunner.cpp:4507
#3 0x00000000004040fd in do_main (argc=argc@entry=1, argv=argv@entry=0x7fffeed04468, xreDirectory=0x7f02b2949780) at /usr/src/debug/firefox-37.0.1/mozilla-release/browser/app/nsBrowserApp.cpp:292
#4 0x000000000040389f in main (argc=1, argv=0x7fffeed04468) at /usr/src/debug/firefox-37.0.1/mozilla-release/browser/app/nsBrowserApp.cpp:661

...whereas ~nsNativeAppSupportUnix is called when it goes out of scope when XRE_main returns,so is necessarily later.

As for the appService->Quit() called from die_cb (DieCB in the patch), it does indeed deliberately queue the work of exiting for after it returns:

// Quit the application. We will asynchronously call the appshell's
// Exit() method via nsAppExitEvent to allow one last pass
// through any events in the queue. This guarantees a tidy cleanup.

So attachment 608321 (with minor tweaks) should indeed fix bug #557601. I'm not sure whether it can be fixed in the current code.