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.
Actually, comment #14 is accurate.Calling SmcCloseConnection from ~nsNativeAppSup portUnix 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, nsObserverServi ce::NotifyObser vers (this=0x7f029b4 d0330, aSubject=0x0, aTopic= 0x7f02b12745a1 "profile- before- change" , aSomeData= 0x7f02b1592000 <nsXREDirProvid er::DoShutdown( )::kShutdownPer sist> u"shutdown- persist" ) at /usr/src/ debug/firefox- 37.0.1/ mozilla- release/ xpcom/ds/ nsObserverServi ce.cpp: 319 ce::NotifyObser vers (this=0x7f029b4 d0330, aSubject=0x0, aTopic= 0x7f02b12745a1 "profile- before- change" , aSomeData= 0x7f02b1592000 <nsXREDirProvid er::DoShutdown( )::kShutdownPer sist> u"shutdown- persist" ) at /usr/src/ debug/firefox- 37.0.1/ mozilla- release/ xpcom/ds/ nsObserverServi ce.cpp: 319 r::DoShutdown (this=0x7fffeed 02f88) at /usr/src/ debug/firefox- 37.0.1/ mozilla- release/ toolkit/ xre/nsXREDirPro vider.cpp: 905 tup::~ScopedXPC OMStartup (this=0x7f02b29 ac7c0, __in_chrg= <optimized out>) at /usr/src/ debug/firefox- 37.0.1/ mozilla- release/ toolkit/ xre/nsAppRunner .cpp:1324 02f48, argc=<optimized out>, argv=<optimized out>, aAppData=<optimized out>) at /usr/src/ debug/firefox- 37.0.1/ mozilla- release/ toolkit/ xre/nsAppRunner .cpp:4312 4468, aAppData= 0x7fffeed03158, aFlags=<optimized out>) at /usr/src/ debug/firefox- 37.0.1/ mozilla- release/ toolkit/ xre/nsAppRunner .cpp:4505 entry=0x7fffeed 04468, xreDirectory= 0x7f02b2949780) at /usr/src/ debug/firefox- 37.0.1/ mozilla- release/ browser/ app/nsBrowserAp p.cpp:292 4468) at /usr/src/ debug/firefox- 37.0.1/ mozilla- release/ browser/ app/nsBrowserAp p.cpp:661
319 {
[Current thread is 1 (Thread 0x7f02b3d5f780 (LWP 8791))]
#0 nsObserverServi
#1 0x00007f02b0ab1fe0 in nsXREDirProvide
#2 0x00007f02b0aaa102 in ScopedXPCOMStar
#3 0x00007f02b0aafc63 in XREMain::XRE_main (this=0x7fffeed
#4 0x00007f02b0aafeac in XRE_main (argc=1, argv=0x7fffeed0
#5 0x00000000004040fd in do_main (argc=argc@entry=1, argv=argv@
#6 0x000000000040389f in main (argc=1, argv=0x7fffeed0
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, nsNativeAppSupp ortUnix: :~nsNativeAppSu pportUnix (this=0x7f02b29 e5460, __in_chrg= <optimized out>) at /usr/src/ debug/firefox- 37.0.1/ mozilla- release/ toolkit/ xre/nsNativeApp SupportUnix. cpp:126 ortUnix : public nsNativeAppSupp ortBase ortUnix: :~nsNativeAppSu pportUnix (this=0x7f02b29 e5460, __in_chrg= <optimized out>) at /usr/src/ debug/firefox- 37.0.1/ mozilla- release/ toolkit/ xre/nsNativeApp SupportUnix. cpp:126 ortBase: :Release (this=0x7f02b29 e5460) at /usr/src/ debug/firefox- 37.0.1/ mozilla- release/ toolkit/ xre/nsNativeApp SupportBase. cpp:16 4468, aAppData= 0x7fffeed03158, aFlags=<optimized out>) at /usr/src/ debug/firefox- 37.0.1/ mozilla- release/ toolkit/ xre/nsAppRunner .cpp:4507 entry=0x7fffeed 04468, xreDirectory= 0x7f02b2949780) at /usr/src/ debug/firefox- 37.0.1/ mozilla- release/ browser/ app/nsBrowserAp p.cpp:292 4468) at /usr/src/ debug/firefox- 37.0.1/ mozilla- release/ browser/ app/nsBrowserAp p.cpp:661
126 class nsNativeAppSupp
[Current thread is 1 (Thread 0x7f02b3d5f780 (LWP 8791))]
#0 nsNativeAppSupp
#1 0x00007f02b0ab0e3e in nsNativeAppSupp
#2 0x00007f02b0aafebd in XRE_main (argc=1, argv=0x7fffeed0
#3 0x00000000004040fd in do_main (argc=argc@entry=1, argv=argv@
#4 0x000000000040389f in main (argc=1, argv=0x7fffeed0
...whereas ~nsNativeAppSup portUnix 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.