973637 - Crash due to null sXPConnect when running make package

Status: RESOLVED FIXED

(Core :: XPConnect, defect)

Description

[:gerard-majax]

Attachment: sxpconnect.gdb.txt

While hacking the Mulet to make it build as of bug 961745, I was fixing bug 973538. This exposes some kind of bad race condition that ends up in a segfault during the make package step.

The make package targets does some cache precompilation through toolkit/mozapps/installer/precompile_cache.js, which just does a Cu.import() on files. Importing DownloadsAPI.jsm triggers a chain that ends up in ./toolkit/components/jsdownloads/src/DownloadIntegration.jsm and more precisely in registering some event observers: http://dxr.mozilla.org/mozilla-central/source/toolkit/components/jsdownloads/src/DownloadIntegration.jsm#837

When xpcshell shutdowns, it triggers a "xpcom-shutdown" event which initiates a "network:offline-about-to-go-offline". This one is caught by the observer previously registered. When executing http://dxr.mozilla.org/mozilla-central/source/toolkit/components/jsdownloads/src/DownloadIntegration.jsm#978, this triggers come lazy module getter in XPCOMUtils.jsm, which finishes by calling nsContentUtils::GetCurrentJSContext(). Still, at this moment, sXPConnect is NULL. Hence, it segfaults.

Attached is the output of the gdb session.

Comment 1

[:gerard-majax]

Attachment: 0017-Bug-973637-Avoid-crash-at-shutdown-during-make-packa.patch

Attached is a patch that fixes the issue. I'll wait for some Try feedback before asking review on this.

Comment 2

[:gerard-majax]

Try pending at: https://tbpl.mozilla.org/?tree=Try&rev=eb284b6b1b22

Comment 3

[:gerard-majax]

Comment on attachment 8377226 [details] [diff] [review]
0017-Bug-973637-Avoid-crash-at-shutdown-during-make-packa.patch

Try is green: https://tbpl.mozilla.org/?tree=Try&rev=eb284b6b1b22

Paolo, since you did most of the work on this file, I'm asking you the review.

Comment 4

[:Paolo Amadini]

Comment on attachment 8377226 [details] [diff] [review]
0017-Bug-973637-Avoid-crash-at-shutdown-during-make-packa.patch

So, you're saying that "network:offline-about-to-go-offline" can be triggered after "xpcom-shutdown", and this is a problem because the observers are still registered?

In this case, probably the best solution is to explicitly unregister the observers on profile shutdown (by reading the list of topics from an array).

Comment 5

[:gerard-majax]

Yes, I think this is what happens. According to my previous GDB backtrace, we can see that:
 - nsIOService::Observe gets a notification of "xpcom-shutdown"
 - it calls nsIOService::SetOffline
 - nsIOService::SetOffline sends a notifyObservers on "network:offline-about-to-go-offline"

However, I'm not sure to understand your suggestion.

Comment 6

[:gerard-majax]

(In reply to :Paolo Amadini from comment #4)
> Comment on attachment 8377226 [details] [diff] [review]
> 0017-Bug-973637-Avoid-crash-at-shutdown-during-make-packa.patch
> 
> So, you're saying that "network:offline-about-to-go-offline" can be
> triggered after "xpcom-shutdown", and this is a problem because the
> observers are still registered?
> 
> In this case, probably the best solution is to explicitly unregister the
> observers on profile shutdown (by reading the list of topics from an array).

After retesting it, just adding an observer on "xpcom-shutdown" event (not even dealing with it properly) is enough to avoid the crash.

Comment 7

[:gerard-majax]

Attachment: 0001-Bug-973637-Avoid-crash-at-shutdown-during-make-packa.patch

Updated patch: unregistering observers on xpcom-shutdown event.

Comment 8

[:Paolo Amadini]

Comment on attachment 8380667 [details] [diff] [review]
0001-Bug-973637-Avoid-crash-at-shutdown-during-make-packa.patch

Looks good, but can you please define the list of topics in a shared array and iterate over it (i.e. "for (let topic of this.kObserverTopics)") both when registering and unregistering?

Also, I'm not sure but maybe we can unregister the observers a little earlier in the shutdown process, on "profile-before-change" rather than "xpcom-shutdown".

Comment 9

[:gerard-majax]

Attachment: 0001-Bug-973637-Avoid-crash-at-shutdown-during-make-packa.patch

Addressing previous remarks: creating a kObservedTopics array, adding and removing observers by iterating on this.

Comment 10

[:gerard-majax]

Pending try for the updated patch: https://tbpl.mozilla.org/?tree=Try&rev=300571d2e337

Comment 11

[:gerard-majax]

Attachment: 0001-Bug-973637-Avoid-crash-at-shutdown-during-make-packa.patch

Previous patch was orange on xpcshell tests on Try. This one should address the issue: xpcshell test passes locally. Try pending at: https://tbpl.mozilla.org/?tree=Try&rev=599edaa3467e

Comment 12

[:gerard-majax]

Attachment: 0001-Bug-973637-Avoid-crash-at-shutdown-during-make-packa.patch

Adding an observer on xpcom-shutdown is really needed to avoid the crash.
Try for this patch: https://tbpl.mozilla.org/?tree=Try&rev=952d67b1789a
Try for the mulet with this patch: https://tbpl.mozilla.org/?tree=Try&rev=80eba280cf4f

Comment 13

[:gerard-majax]

Comment on attachment 8381448 [details] [diff] [review]
0001-Bug-973637-Avoid-crash-at-shutdown-during-make-packa.patch

Forgot to reask for review :)

Comment 14

[:Paolo Amadini]

Comment on attachment 8381448 [details] [diff] [review]
0001-Bug-973637-Avoid-crash-at-shutdown-during-make-packa.patch

Review of attachment 8381448 [details] [diff] [review]:
-----------------------------------------------------------------

::: toolkit/components/jsdownloads/src/DownloadIntegration.jsm
@@ +137,5 @@
> +  "network:offline-about-to-go-offline",
> +  "network:offline-status-changed",
> +  "profile-before-change",
> +  "xpcom-shutdown"
> +];

nit: comma even in last item (it's a convention we use here)

@@ +843,5 @@
>      DownloadObserver.registerView(aList, aIsPrivate);
>      if (!DownloadObserver.observersAdded) {
>        DownloadObserver.observersAdded = true;
> +      for (let i in kObserverTopics) {
> +        let topic = kObserverTopics[i];

I missed this in my previous review, sorry, but this should have been a "for..of" loop: for (let topic of kObserverTopics)

@@ +1052,5 @@
> +      case "profile-before-change":
> +      case "xpcom-shutdown":
> +        for (let i in kObserverTopics) {
> +          let topic = kObserverTopics[i];
> +          Services.obs.removeObserver(this, topic, true);

Hm, I'm not convinced that we need to observe "xpcom-shutdown".

Assuming we remove "xpcom-shutdown" from the observed topics array, does the crash still occur? If so, is it for the same reasons outlined in comment 0?

Comment 15

[:gerard-majax]

As documented in comment #12, if I remove listener for "xpcom-shutdown" we still get to crash. I haven't checked the backtrace.

Comment 16

[:Paolo Amadini]

Comment on attachment 8381448 [details] [diff] [review]
0001-Bug-973637-Avoid-crash-at-shutdown-during-make-packa.patch

Review of attachment 8381448 [details] [diff] [review]:
-----------------------------------------------------------------

::: toolkit/components/jsdownloads/src/DownloadIntegration.jsm
@@ +844,5 @@
>      if (!DownloadObserver.observersAdded) {
>        DownloadObserver.observersAdded = true;
> +      for (let i in kObserverTopics) {
> +        let topic = kObserverTopics[i];
> +        Services.obs.addObserver(DownloadObserver, topic, true);

Ah, you may also try to pass false as the last parameter and remove the Ci.nsISupportsWeakReference below.

@@ +1052,5 @@
> +      case "profile-before-change":
> +      case "xpcom-shutdown":
> +        for (let i in kObserverTopics) {
> +          let topic = kObserverTopics[i];
> +          Services.obs.removeObserver(this, topic, true);

...and there is no third parameter to removeObserver.

Comment 17

[:Paolo Amadini]

(In reply to Alexandre LISSY :gerard-majax from comment #15)
> As documented in comment #12, if I remove listener for "xpcom-shutdown" we
> still get to crash. I haven't checked the backtrace.

I'd expect it to be different, by looking at it we might understand the underlying issue, which I think is worth doing to avoid similar cases in the future.

Comment 18

[:gerard-majax]

Here is the stack with profile-before-change observer but no xpcom-shutdown.

Program received signal SIGSEGV, Segmentation fault.
0x00007ffff4dee369 in nsContentUtils::GetCurrentJSContext () at /home/alex/codaz/Mozilla/b2g/gecko/content/base/src/nsContentUtils.cpp:5143
5143	  return sXPConnect->GetCurrentJSContext();
(gdb) bt
#0  0x00007ffff4dee369 in nsContentUtils::GetCurrentJSContext () at /home/alex/codaz/Mozilla/b2g/gecko/content/base/src/nsContentUtils.cpp:5143
#1  0x00007ffff4c0c014 in JSCLContextHelper::~JSCLContextHelper (this=0x7fffffff8aa0, __in_chrg=<optimized out>) at /home/alex/codaz/Mozilla/b2g/gecko/jProgram received signal SIGSEGV, Segmentation fault.
0x00007ffff4dee369 in nsContentUtils::GetCurrentJSContext () at /home/alex/codaz/Mozilla/b2g/gecko/content/base/src/nsContentUtils.cpp:5143
5143	  return sXPConnect->GetCurrentJSContext();
(gdb) bt
#0  0x00007ffff4dee369 in nsContentUtils::GetCurrentJSContext () at /home/alex/codaz/Mozilla/b2g/gecko/content/base/src/nsContentUtils.cpp:5143
#1  0x00007ffff4c0c014 in JSCLContextHelper::~JSCLContextHelper (this=0x7fffffff8aa0, __in_chrg=<optimized out>) at /home/alex/codaz/Mozilla/b2g/gecko/js/xpconnect/loader/mozJSComponentLoader.cpp:1437
#2  0x00007ffff4c0e3ca in mozJSComponentLoader::ImportInto (this=this@entry=0x7fffea239da0, aLocation=..., targetObj=targetObj@entry=..., callercx=0x7fffffff8a50, callercx@entry=0x7fffe95e1ba0, vp=...)
    at /home/alex/codaz/Mozilla/b2g/gecko/js/xpconnect/loader/mozJSComponentLoader.cpp:1336
#3  0x00007ffff4c0e7e3 in mozJSComponentLoader::Import (this=0x7fffea239da0, registryLocation=..., targetValArg=..., cx=0x7fffe95e1ba0, optionalArgc=<optimized out>, retval=...)
    at /home/alex/codaz/Mozilla/b2g/gecko/js/xpconnect/loader/mozJSComponentLoader.cpp:1127
#4  0x00007ffff4bbcac6 in nsXPCComponents_Utils::Import (this=<optimized out>, registryLocation=..., targetObj=..., cx=<optimized out>, optionalArgc=<optimized out>, retval=...)
    at /home/alex/codaz/Mozilla/b2g/gecko/js/xpconnect/src/XPCComponents.cpp:2738
#5  0x00007ffff433408e in NS_InvokeByIndex (that=<optimized out>, methodIndex=<optimized out>, paramCount=<optimized out>, params=<optimized out>)
    at /home/alex/codaz/Mozilla/b2g/gecko/xpcom/reflect/xptcall/src/md/unix/xptcinvoke_x86_64_unix.cpp:164
#6  0x00007ffff4bf8a79 in Invoke (this=0x7fffffff8e48) at /home/alex/codaz/Mozilla/b2g/gecko/js/xpconnect/src/XPCWrappedNative.cpp:2389
#7  Call (this=0x7fffffff8e48) at /home/alex/codaz/Mozilla/b2g/gecko/js/xpconnect/src/XPCWrappedNative.cpp:1734
#8  XPCWrappedNative::CallMethod (ccx=..., mode=mode@entry=XPCWrappedNative::CALL_METHOD) at /home/alex/codaz/Mozilla/b2g/gecko/js/xpconnect/src/XPCWrappedNative.cpp:1701
#9  0x00007ffff4bfa189 in XPC_WN_CallMethod (cx=0x7fffe95e1ba0, argc=2, vp=0x7fffe78a1218) at /home/alex/codaz/Mozilla/b2g/gecko/js/xpconnect/src/XPCWrappedNativeJSOps.cpp:1282
#10 0x00007ffff5be379b in CallJSNative (args=..., native=0x7ffff4bf9ffb <XPC_WN_CallMethod(JSContext*, unsigned int, JS::Value*)>, cx=0x7fffe95e1ba0)
    at /home/alex/codaz/Mozilla/b2g/gecko/js/src/../../js/src/jscntxtinlines.h:220
#11 js::Invoke (cx=0x7fffe95e1ba0, args=..., construct=<optimized out>) at /home/alex/codaz/Mozilla/b2g/gecko/js/src/../../js/src/vm/Interpreter.cpp:476
#12 0x00007ffff5bdf765 in Interpret (cx=0x7fffe95e1ba0, state=...) at /home/alex/codaz/Mozilla/b2g/gecko/js/src/../../js/src/vm/Interpreter.cpp:2608
#13 0x00007ffff5be33e3 in js::RunScript (cx=cx@entry=0x7fffe95e1ba0, state=...) at /home/alex/codaz/Mozilla/b2g/gecko/js/src/../../js/src/vm/Interpreter.cpp:423
#14 0x00007ffff5be368a in RunScript (state=..., cx=0x7fffe95e1ba0) at /home/alex/codaz/Mozilla/b2g/gecko/js/src/../../js/src/vm/Interpreter.cpp:390
#15 js::Invoke (cx=cx@entry=0x7fffe95e1ba0, args=..., construct=construct@entry=js::NO_CONSTRUCT) at /home/alex/codaz/Mozilla/b2g/gecko/js/src/../../js/src/vm/Interpreter.cpp:495
#16 0x00007ffff5af0edf in js_fun_call (cx=cx@entry=0x7fffe95e1ba0, argc=<optimized out>, vp=vp@entry=0x7fffe78a1178) at /home/alex/codaz/Mozilla/b2g/gecko/js/src/../../js/src/jsfun.cpp:910
#17 0x00007ffff5af12b5 in js_fun_apply (cx=0x7fffe95e1ba0, argc=<optimized out>, vp=0x7fffe78a1178) at /home/alex/codaz/Mozilla/b2g/gecko/js/src/../../js/src/jsfun.cpp:949
#18 0x00007ffff5be379b in CallJSNative (args=..., native=0x7ffff5af1090 <js_fun_apply(JSContext*, unsigned int, JS::Value*)>, cx=0x7fffe95e1ba0)
    at /home/alex/codaz/Mozilla/b2g/gecko/js/src/../../js/src/jscntxtinlines.h:220
#19 js::Invoke (cx=0x7fffe95e1ba0, args=..., construct=<optimized out>) at /home/alex/codaz/Mozilla/b2g/gecko/js/src/../../js/src/vm/Interpreter.cpp:476
#20 0x00007ffff5bdf765 in Interpret (cx=0x7fffe95e1ba0, state=...) at /home/alex/codaz/Mozilla/b2g/gecko/js/src/../../js/src/vm/Interpreter.cpp:2608
#21 0x00007ffff5be33e3 in js::RunScript (cx=cx@entry=0x7fffe95e1ba0, state=...) at /home/alex/codaz/Mozilla/b2g/gecko/js/src/../../js/src/vm/Interpreter.cpp:423
#22 0x00007ffff5be368a in RunScript (state=..., cx=0x7fffe95e1ba0) at /home/alex/codaz/Mozilla/b2g/gecko/js/src/../../js/src/vm/Interpreter.cpp:390
#23 js::Invoke (cx=cx@entry=0x7fffe95e1ba0, args=..., construct=construct@entry=js::NO_CONSTRUCT) at /home/alex/codaz/Mozilla/b2g/gecko/js/src/../../js/src/vm/Interpreter.cpp:495
#24 0x00007ffff5be40ab in js::Invoke (cx=cx@entry=0x7fffe95e1ba0, thisv=..., fval=..., argc=0, argv=<optimized out>, rval=...) at /home/alex/codaz/Mozilla/b2g/gecko/js/src/../../js/src/vm/Interpreter.cpp:532
#25 0x00007ffff5b0a586 in js::DirectProxyHandler::call (this=this@entry=0x7ffff7afda60 <js::CrossCompartmentWrapper::singleton>, cx=cx@entry=0x7fffe95e1ba0, proxy=..., proxy@entry=..., args=...)
    at /home/alex/codaz/Mozilla/b2g/gecko/js/src/../../js/src/jsproxy.cpp:465
#26 0x00007ffff5bc7d83 in js::CrossCompartmentWrapper::call (this=0x7ffff7afda60 <js::CrossCompartmentWrapper::singleton>, cx=0x7fffe95e1ba0, wrapper=..., args=...)
    at /home/alex/codaz/Mozilla/b2g/gecko/js/src/../../js/src/jswrapper.cpp:468
#27 0x00007ffff5b54abd in js::Proxy::call (cx=cx@entry=0x7fffe95e1ba0, proxy=proxy@entry=..., args=...) at /home/alex/codaz/Mozilla/b2g/gecko/js/src/../../js/src/jsproxy.cpp:2636
#28 0x00007ffff5b54bca in js::proxy_Call (cx=cx@entry=0x7fffe95e1ba0, argc=<optimized out>, vp=<optimized out>) at /home/alex/codaz/Mozilla/b2g/gecko/js/src/../../js/src/jsproxy.cpp:3079
#29 0x00007ffff5be38d1 in CallJSNative (args=..., native=0x7ffff5b54b80 <js::proxy_Call(JSContext*, unsigned int, JS::Value*)>, cx=0x7fffe95e1ba0)
    at /home/alex/codaz/Mozilla/b2g/gecko/js/src/../../js/src/jscntxtinlines.h:220
#30 js::Invoke (cx=cx@entry=0x7fffe95e1ba0, args=..., construct=construct@entry=js::NO_CONSTRUCT) at /home/alex/codaz/Mozilla/b2g/gecko/js/src/../../js/src/vm/Interpreter.cpp:469
#31 0x00007ffff5be40ab in js::Invoke (cx=cx@entry=0x7fffe95e1ba0, thisv=..., fval=..., argc=argc@entry=0, argv=argv@entry=0x0, rval=..., rval@entry=...)
    at /home/alex/codaz/Mozilla/b2g/gecko/js/src/../../js/src/vm/Interpreter.cpp:532
#32 0x00007ffff5be4220 in js::InvokeGetterOrSetter (cx=cx@entry=0x7fffe95e1ba0, obj=0x7fffe48acd80, fval=..., argc=argc@entry=0, argv=argv@entry=0x0, rval=...)
    at /home/alex/codaz/Mozilla/b2g/gecko/js/src/../../js/src/vm/Interpreter.cpp:604
#33 0x00007ffff5b960ea in js::Shape::get (this=<optimized out>, cx=cx@entry=0x7fffe95e1ba0, receiver=receiver@entry=..., obj=<optimized out>, pobj=<optimized out>, vp=..., vp@entry=...)
    at /home/alex/codaz/Mozilla/b2g/gecko/js/src/../../js/src/vm/Shape-inl.h:68
#34 0x00007ffff5b254f8 in NativeGetInline<(js::AllowGC)1> (vp=..., shape=..., pobj=..., receiver=..., obj=..., cx=0x7fffe95e1ba0) at /home/alex/codaz/Mozilla/b2g/gecko/js/src/../../js/src/jsobj.cpp:4319
#35 js::NativeGet (cx=cx@entry=0x7fffe95e1ba0, obj=obj@entry=..., pobj=..., shape=..., vp=...) at /home/alex/codaz/Mozilla/b2g/gecko/js/src/../../js/src/jsobj.cpp:4339
#36 0x00007ffff5a35e75 in js::FetchName<false> (cx=0x7fffe95e1ba0, obj=..., obj2=..., name=..., shape=..., vp=...) at /home/alex/codaz/Mozilla/b2g/gecko/js/src/../../js/src/vm/Interpreter-inl.h:192
#37 0x00007ffff5bd964e in NameOperation (vp=..., pc=<optimized out>, fp=<optimized out>, cx=<optimized out>) at /home/alex/codaz/Mozilla/b2g/gecko/js/src/../../js/src/vm/Interpreter.cpp:318
---Type <return> to continue, or q <return> to quit---
#38 Interpret (cx=0x7fffe95e1ba0, state=...) at /home/alex/codaz/Mozilla/b2g/gecko/js/src/../../js/src/vm/Interpreter.cpp:2724
#39 0x00007ffff5be33e3 in js::RunScript (cx=cx@entry=0x7fffe95e1ba0, state=...) at /home/alex/codaz/Mozilla/b2g/gecko/js/src/../../js/src/vm/Interpreter.cpp:423
#40 0x00007ffff5be368a in RunScript (state=..., cx=0x7fffe95e1ba0) at /home/alex/codaz/Mozilla/b2g/gecko/js/src/../../js/src/vm/Interpreter.cpp:390
#41 js::Invoke (cx=cx@entry=0x7fffe95e1ba0, args=..., construct=construct@entry=js::NO_CONSTRUCT) at /home/alex/codaz/Mozilla/b2g/gecko/js/src/../../js/src/vm/Interpreter.cpp:495
#42 0x00007ffff5be40ab in js::Invoke (cx=cx@entry=0x7fffe95e1ba0, thisv=..., fval=..., argc=3, argv=<optimized out>, rval=...) at /home/alex/codaz/Mozilla/b2g/gecko/js/src/../../js/src/vm/Interpreter.cpp:532
#43 0x00007ffff5af7b84 in JS_CallFunctionValue (cx=cx@entry=0x7fffe95e1ba0, objArg=<optimized out>, fval=..., argc=argc@entry=3, argv=argv@entry=0x7fffffffd760, rval=rval@entry=0x7fffffffd598)
    at /home/alex/codaz/Mozilla/b2g/gecko/js/src/../../js/src/jsapi.cpp:5014
#44 0x00007ffff4bfb1f8 in nsXPCWrappedJSClass::CallMethod (this=0x7fffe5f17c40, wrapper=<optimized out>, methodIndex=3, info_=0x7fffe961feb0, nativeParams=0x7fffffffd820)
    at /home/alex/codaz/Mozilla/b2g/gecko/js/xpconnect/src/XPCWrappedJSClass.cpp:1290
#45 0x00007ffff4334c65 in PrepareAndDispatch (self=0x7fffe33fbb20, methodIndex=<optimized out>, args=<optimized out>, gpregs=0x7fffffffd8e0, fpregs=0x7fffffffd910)
    at /home/alex/codaz/Mozilla/b2g/gecko/xpcom/reflect/xptcall/src/md/unix/xptcstubs_x86_64_linux.cpp:122
#46 0x00007ffff4334163 in SharedStub () from /home/alex/codaz/Mozilla/b2g/gecko/obj-x86_64-unknown-linux-gnu/dist/bin/libxul.so
#47 0x00007ffff4314714 in nsObserverList::NotifyObservers (this=<optimized out>, aSubject=aSubject@entry=0x7fffea230f00, aTopic=aTopic@entry=0x7ffff5e0a98f "network:offline-about-to-go-offline", 
    someData=someData@entry=0x7ffff5f79ef6 u"offline") at /home/alex/codaz/Mozilla/b2g/gecko/xpcom/ds/nsObserverList.cpp:96
#48 0x00007ffff431479e in NotifyObservers (someData=0x7ffff5f79ef6 u"offline", aTopic=0x7ffff5e0a98f "network:offline-about-to-go-offline", aSubject=0x7fffea230f00, this=0x7fffea227a00)
    at /home/alex/codaz/Mozilla/b2g/gecko/xpcom/ds/nsObserverService.cpp:302
#49 nsObserverService::NotifyObservers (this=0x7fffea227a00, aSubject=0x7fffea230f00, aTopic=0x7ffff5e0a98f "network:offline-about-to-go-offline", someData=0x7ffff5f79ef6 u"offline")
    at /home/alex/codaz/Mozilla/b2g/gecko/xpcom/ds/nsObserverService.cpp:290
#50 0x00007ffff43771f2 in SetOffline (offline=<optimized out>, this=0x7fffea230f00) at /home/alex/codaz/Mozilla/b2g/gecko/netwerk/base/src/nsIOService.cpp:703
#51 nsIOService::SetOffline (this=0x7fffea230f00, offline=<optimized out>) at /home/alex/codaz/Mozilla/b2g/gecko/netwerk/base/src/nsIOService.cpp:659
#52 0x00007ffff437f903 in nsIOService::Observe (this=0x7fffea230f00, subject=0x7fffea24d368, topic=0x7ffff5df6f68 "xpcom-shutdown", data=0x0)
    at /home/alex/codaz/Mozilla/b2g/gecko/netwerk/base/src/nsIOService.cpp:949
#53 0x00007ffff4314714 in nsObserverList::NotifyObservers (this=<optimized out>, aSubject=aSubject@entry=0x7fffea24d368, aTopic=aTopic@entry=0x7ffff5df6f68 "xpcom-shutdown", someData=someData@entry=0x0)
    at /home/alex/codaz/Mozilla/b2g/gecko/xpcom/ds/nsObserverList.cpp:96
#54 0x00007ffff431479e in NotifyObservers (someData=0x0, aTopic=0x7ffff5df6f68 "xpcom-shutdown", aSubject=0x7fffea24d368, this=0x7fffea227a00)
    at /home/alex/codaz/Mozilla/b2g/gecko/xpcom/ds/nsObserverService.cpp:302
#55 nsObserverService::NotifyObservers (this=0x7fffea227a00, aSubject=0x7fffea24d368, aTopic=aTopic@entry=0x7ffff5df6f68 "xpcom-shutdown", someData=someData@entry=0x0)
    at /home/alex/codaz/Mozilla/b2g/gecko/xpcom/ds/nsObserverService.cpp:290
#56 0x00007ffff42f230f in mozilla::ShutdownXPCOM (servMgr=0x0) at /home/alex/codaz/Mozilla/b2g/gecko/xpcom/build/nsXPComInit.cpp:699
#57 0x00007ffff4bf4e82 in XRE_XPCShellMain (argc=4, argv=0x7fffffffdfe0, envp=0x400) at /home/alex/codaz/Mozilla/b2g/gecko/js/xpconnect/src/XPCShellImpl.cpp:1602
#58 0x00007ffff3016ea5 in __libc_start_main (main=0x402e5c <main(int, char**, char**)>, argc=9, ubp_av=0x7fffffffdfb8, init=<optimized out>, fini=<optimized out>, rtld_fini=<optimized out>, 
    stack_end=0x7fffffffdfa8) at libc-start.c:260
#59 0x0000000000402d99 in _start ()
s/xpconnect/loader/mozJSComponentLoader.cpp:1437
#2  0x00007ffff4c0e3ca in mozJSComponentLoader::ImportInto (this=this@entry=0x7fffea239da0, aLocation=..., targetObj=targetObj@entry=..., callercx=0x7fffffff8a50, callercx@entry=0x7fffe95e1ba0, vp=...)
    at /home/alex/codaz/Mozilla/b2g/gecko/js/xpconnect/loader/mozJSComponentLoader.cpp:1336
#3  0x00007ffff4c0e7e3 in mozJSComponentLoader::Import (this=0x7fffea239da0, registryLocation=..., targetValArg=..., cx=0x7fffe95e1ba0, optionalArgc=<optimized out>, retval=...)
    at /home/alex/codaz/Mozilla/b2g/gecko/js/xpconnect/loader/mozJSComponentLoader.cpp:1127
#4  0x00007ffff4bbcac6 in nsXPCComponents_Utils::Import (this=<optimized out>, registryLocation=..., targetObj=..., cx=<optimized out>, optionalArgc=<optimized out>, retval=...)
    at /home/alex/codaz/Mozilla/b2g/gecko/js/xpconnect/src/XPCComponents.cpp:2738
#5  0x00007ffff433408e in NS_InvokeByIndex (that=<optimized out>, methodIndex=<optimized out>, paramCount=<optimized out>, params=<optimized out>)
    at /home/alex/codaz/Mozilla/b2g/gecko/xpcom/reflect/xptcall/src/md/unix/xptcinvoke_x86_64_unix.cpp:164
#6  0x00007ffff4bf8a79 in Invoke (this=0x7fffffff8e48) at /home/alex/codaz/Mozilla/b2g/gecko/js/xpconnect/src/XPCWrappedNative.cpp:2389
#7  Call (this=0x7fffffff8e48) at /home/alex/codaz/Mozilla/b2g/gecko/js/xpconnect/src/XPCWrappedNative.cpp:1734
#8  XPCWrappedNative::CallMethod (ccx=..., mode=mode@entry=XPCWrappedNative::CALL_METHOD) at /home/alex/codaz/Mozilla/b2g/gecko/js/xpconnect/src/XPCWrappedNative.cpp:1701
#9  0x00007ffff4bfa189 in XPC_WN_CallMethod (cx=0x7fffe95e1ba0, argc=2, vp=0x7fffe78a1218) at /home/alex/codaz/Mozilla/b2g/gecko/js/xpconnect/src/XPCWrappedNativeJSOps.cpp:1282
#10 0x00007ffff5be379b in CallJSNative (args=..., native=0x7ffff4bf9ffb <XPC_WN_CallMethod(JSContext*, unsigned int, JS::Value*)>, cx=0x7fffe95e1ba0)
    at /home/alex/codaz/Mozilla/b2g/gecko/js/src/../../js/src/jscntxtinlines.h:220
#11 js::Invoke (cx=0x7fffe95e1ba0, args=..., construct=<optimized out>) at /home/alex/codaz/Mozilla/b2g/gecko/js/src/../../js/src/vm/Interpreter.cpp:476
#12 0x00007ffff5bdf765 in Interpret (cx=0x7fffe95e1ba0, state=...) at /home/alex/codaz/Mozilla/b2g/gecko/js/src/../../js/src/vm/Interpreter.cpp:2608
#13 0x00007ffff5be33e3 in js::RunScript (cx=cx@entry=0x7fffe95e1ba0, state=...) at /home/alex/codaz/Mozilla/b2g/gecko/js/src/../../js/src/vm/Interpreter.cpp:423
#14 0x00007ffff5be368a in RunScript (state=..., cx=0x7fffe95e1ba0) at /home/alex/codaz/Mozilla/b2g/gecko/js/src/../../js/src/vm/Interpreter.cpp:390
#15 js::Invoke (cx=cx@entry=0x7fffe95e1ba0, args=..., construct=construct@entry=js::NO_CONSTRUCT) at /home/alex/codaz/Mozilla/b2g/gecko/js/src/../../js/src/vm/Interpreter.cpp:495
#16 0x00007ffff5af0edf in js_fun_call (cx=cx@entry=0x7fffe95e1ba0, argc=<optimized out>, vp=vp@entry=0x7fffe78a1178) at /home/alex/codaz/Mozilla/b2g/gecko/js/src/../../js/src/jsfun.cpp:910
#17 0x00007ffff5af12b5 in js_fun_apply (cx=0x7fffe95e1ba0, argc=<optimized out>, vp=0x7fffe78a1178) at /home/alex/codaz/Mozilla/b2g/gecko/js/src/../../js/src/jsfun.cpp:949
#18 0x00007ffff5be379b in CallJSNative (args=..., native=0x7ffff5af1090 <js_fun_apply(JSContext*, unsigned int, JS::Value*)>, cx=0x7fffe95e1ba0)
    at /home/alex/codaz/Mozilla/b2g/gecko/js/src/../../js/src/jscntxtinlines.h:220
#19 js::Invoke (cx=0x7fffe95e1ba0, args=..., construct=<optimized out>) at /home/alex/codaz/Mozilla/b2g/gecko/js/src/../../js/src/vm/Interpreter.cpp:476
#20 0x00007ffff5bdf765 in Interpret (cx=0x7fffe95e1ba0, state=...) at /home/alex/codaz/Mozilla/b2g/gecko/js/src/../../js/src/vm/Interpreter.cpp:2608
#21 0x00007ffff5be33e3 in js::RunScript (cx=cx@entry=0x7fffe95e1ba0, state=...) at /home/alex/codaz/Mozilla/b2g/gecko/js/src/../../js/src/vm/Interpreter.cpp:423
#22 0x00007ffff5be368a in RunScript (state=..., cx=0x7fffe95e1ba0) at /home/alex/codaz/Mozilla/b2g/gecko/js/src/../../js/src/vm/Interpreter.cpp:390
#23 js::Invoke (cx=cx@entry=0x7fffe95e1ba0, args=..., construct=construct@entry=js::NO_CONSTRUCT) at /home/alex/codaz/Mozilla/b2g/gecko/js/src/../../js/src/vm/Interpreter.cpp:495
#24 0x00007ffff5be40ab in js::Invoke (cx=cx@entry=0x7fffe95e1ba0, thisv=..., fval=..., argc=0, argv=<optimized out>, rval=...) at /home/alex/codaz/Mozilla/b2g/gecko/js/src/../../js/src/vm/Interpreter.cpp:532
#25 0x00007ffff5b0a586 in js::DirectProxyHandler::call (this=this@entry=0x7ffff7afda60 <js::CrossCompartmentWrapper::singleton>, cx=cx@entry=0x7fffe95e1ba0, proxy=..., proxy@entry=..., args=...)
    at /home/alex/codaz/Mozilla/b2g/gecko/js/src/../../js/src/jsproxy.cpp:465
#26 0x00007ffff5bc7d83 in js::CrossCompartmentWrapper::call (this=0x7ffff7afda60 <js::CrossCompartmentWrapper::singleton>, cx=0x7fffe95e1ba0, wrapper=..., args=...)
    at /home/alex/codaz/Mozilla/b2g/gecko/js/src/../../js/src/jswrapper.cpp:468
#27 0x00007ffff5b54abd in js::Proxy::call (cx=cx@entry=0x7fffe95e1ba0, proxy=proxy@entry=..., args=...) at /home/alex/codaz/Mozilla/b2g/gecko/js/src/../../js/src/jsproxy.cpp:2636
#28 0x00007ffff5b54bca in js::proxy_Call (cx=cx@entry=0x7fffe95e1ba0, argc=<optimized out>, vp=<optimized out>) at /home/alex/codaz/Mozilla/b2g/gecko/js/src/../../js/src/jsproxy.cpp:3079
#29 0x00007ffff5be38d1 in CallJSNative (args=..., native=0x7ffff5b54b80 <js::proxy_Call(JSContext*, unsigned int, JS::Value*)>, cx=0x7fffe95e1ba0)
    at /home/alex/codaz/Mozilla/b2g/gecko/js/src/../../js/src/jscntxtinlines.h:220
#30 js::Invoke (cx=cx@entry=0x7fffe95e1ba0, args=..., construct=construct@entry=js::NO_CONSTRUCT) at /home/alex/codaz/Mozilla/b2g/gecko/js/src/../../js/src/vm/Interpreter.cpp:469
#31 0x00007ffff5be40ab in js::Invoke (cx=cx@entry=0x7fffe95e1ba0, thisv=..., fval=..., argc=argc@entry=0, argv=argv@entry=0x0, rval=..., rval@entry=...)
    at /home/alex/codaz/Mozilla/b2g/gecko/js/src/../../js/src/vm/Interpreter.cpp:532
#32 0x00007ffff5be4220 in js::InvokeGetterOrSetter (cx=cx@entry=0x7fffe95e1ba0, obj=0x7fffe48acd80, fval=..., argc=argc@entry=0, argv=argv@entry=0x0, rval=...)
    at /home/alex/codaz/Mozilla/b2g/gecko/js/src/../../js/src/vm/Interpreter.cpp:604
#33 0x00007ffff5b960ea in js::Shape::get (this=<optimized out>, cx=cx@entry=0x7fffe95e1ba0, receiver=receiver@entry=..., obj=<optimized out>, pobj=<optimized out>, vp=..., vp@entry=...)
    at /home/alex/codaz/Mozilla/b2g/gecko/js/src/../../js/src/vm/Shape-inl.h:68
#34 0x00007ffff5b254f8 in NativeGetInline<(js::AllowGC)1> (vp=..., shape=..., pobj=..., receiver=..., obj=..., cx=0x7fffe95e1ba0) at /home/alex/codaz/Mozilla/b2g/gecko/js/src/../../js/src/jsobj.cpp:4319
#35 js::NativeGet (cx=cx@entry=0x7fffe95e1ba0, obj=obj@entry=..., pobj=..., shape=..., vp=...) at /home/alex/codaz/Mozilla/b2g/gecko/js/src/../../js/src/jsobj.cpp:4339
#36 0x00007ffff5a35e75 in js::FetchName<false> (cx=0x7fffe95e1ba0, obj=..., obj2=..., name=..., shape=..., vp=...) at /home/alex/codaz/Mozilla/b2g/gecko/js/src/../../js/src/vm/Interpreter-inl.h:192
#37 0x00007ffff5bd964e in NameOperation (vp=..., pc=<optimized out>, fp=<optimized out>, cx=<optimized out>) at /home/alex/codaz/Mozilla/b2g/gecko/js/src/../../js/src/vm/Interpreter.cpp:318
---Type <return> to continue, or q <return> to quit---
#38 Interpret (cx=0x7fffe95e1ba0, state=...) at /home/alex/codaz/Mozilla/b2g/gecko/js/src/../../js/src/vm/Interpreter.cpp:2724
#39 0x00007ffff5be33e3 in js::RunScript (cx=cx@entry=0x7fffe95e1ba0, state=...) at /home/alex/codaz/Mozilla/b2g/gecko/js/src/../../js/src/vm/Interpreter.cpp:423
#40 0x00007ffff5be368a in RunScript (state=..., cx=0x7fffe95e1ba0) at /home/alex/codaz/Mozilla/b2g/gecko/js/src/../../js/src/vm/Interpreter.cpp:390
#41 js::Invoke (cx=cx@entry=0x7fffe95e1ba0, args=..., construct=construct@entry=js::NO_CONSTRUCT) at /home/alex/codaz/Mozilla/b2g/gecko/js/src/../../js/src/vm/Interpreter.cpp:495
#42 0x00007ffff5be40ab in js::Invoke (cx=cx@entry=0x7fffe95e1ba0, thisv=..., fval=..., argc=3, argv=<optimized out>, rval=...) at /home/alex/codaz/Mozilla/b2g/gecko/js/src/../../js/src/vm/Interpreter.cpp:532
#43 0x00007ffff5af7b84 in JS_CallFunctionValue (cx=cx@entry=0x7fffe95e1ba0, objArg=<optimized out>, fval=..., argc=argc@entry=3, argv=argv@entry=0x7fffffffd760, rval=rval@entry=0x7fffffffd598)
    at /home/alex/codaz/Mozilla/b2g/gecko/js/src/../../js/src/jsapi.cpp:5014
#44 0x00007ffff4bfb1f8 in nsXPCWrappedJSClass::CallMethod (this=0x7fffe5f17c40, wrapper=<optimized out>, methodIndex=3, info_=0x7fffe961feb0, nativeParams=0x7fffffffd820)
    at /home/alex/codaz/Mozilla/b2g/gecko/js/xpconnect/src/XPCWrappedJSClass.cpp:1290
#45 0x00007ffff4334c65 in PrepareAndDispatch (self=0x7fffe33fbb20, methodIndex=<optimized out>, args=<optimized out>, gpregs=0x7fffffffd8e0, fpregs=0x7fffffffd910)
    at /home/alex/codaz/Mozilla/b2g/gecko/xpcom/reflect/xptcall/src/md/unix/xptcstubs_x86_64_linux.cpp:122
#46 0x00007ffff4334163 in SharedStub () from /home/alex/codaz/Mozilla/b2g/gecko/obj-x86_64-unknown-linux-gnu/dist/bin/libxul.so
#47 0x00007ffff4314714 in nsObserverList::NotifyObservers (this=<optimized out>, aSubject=aSubject@entry=0x7fffea230f00, aTopic=aTopic@entry=0x7ffff5e0a98f "network:offline-about-to-go-offline", 
    someData=someData@entry=0x7ffff5f79ef6 u"offline") at /home/alex/codaz/Mozilla/b2g/gecko/xpcom/ds/nsObserverList.cpp:96
#48 0x00007ffff431479e in NotifyObservers (someData=0x7ffff5f79ef6 u"offline", aTopic=0x7ffff5e0a98f "network:offline-about-to-go-offline", aSubject=0x7fffea230f00, this=0x7fffea227a00)
    at /home/alex/codaz/Mozilla/b2g/gecko/xpcom/ds/nsObserverService.cpp:302
#49 nsObserverService::NotifyObservers (this=0x7fffea227a00, aSubject=0x7fffea230f00, aTopic=0x7ffff5e0a98f "network:offline-about-to-go-offline", someData=0x7ffff5f79ef6 u"offline")
    at /home/alex/codaz/Mozilla/b2g/gecko/xpcom/ds/nsObserverService.cpp:290
#50 0x00007ffff43771f2 in SetOffline (offline=<optimized out>, this=0x7fffea230f00) at /home/alex/codaz/Mozilla/b2g/gecko/netwerk/base/src/nsIOService.cpp:703
#51 nsIOService::SetOffline (this=0x7fffea230f00, offline=<optimized out>) at /home/alex/codaz/Mozilla/b2g/gecko/netwerk/base/src/nsIOService.cpp:659
#52 0x00007ffff437f903 in nsIOService::Observe (this=0x7fffea230f00, subject=0x7fffea24d368, topic=0x7ffff5df6f68 "xpcom-shutdown", data=0x0)
    at /home/alex/codaz/Mozilla/b2g/gecko/netwerk/base/src/nsIOService.cpp:949
#53 0x00007ffff4314714 in nsObserverList::NotifyObservers (this=<optimized out>, aSubject=aSubject@entry=0x7fffea24d368, aTopic=aTopic@entry=0x7ffff5df6f68 "xpcom-shutdown", someData=someData@entry=0x0)
    at /home/alex/codaz/Mozilla/b2g/gecko/xpcom/ds/nsObserverList.cpp:96
#54 0x00007ffff431479e in NotifyObservers (someData=0x0, aTopic=0x7ffff5df6f68 "xpcom-shutdown", aSubject=0x7fffea24d368, this=0x7fffea227a00)
    at /home/alex/codaz/Mozilla/b2g/gecko/xpcom/ds/nsObserverService.cpp:302
#55 nsObserverService::NotifyObservers (this=0x7fffea227a00, aSubject=0x7fffea24d368, aTopic=aTopic@entry=0x7ffff5df6f68 "xpcom-shutdown", someData=someData@entry=0x0)
    at /home/alex/codaz/Mozilla/b2g/gecko/xpcom/ds/nsObserverService.cpp:290
#56 0x00007ffff42f230f in mozilla::ShutdownXPCOM (servMgr=0x0) at /home/alex/codaz/Mozilla/b2g/gecko/xpcom/build/nsXPComInit.cpp:699
#57 0x00007ffff4bf4e82 in XRE_XPCShellMain (argc=4, argv=0x7fffffffdfe0, envp=0x400) at /home/alex/codaz/Mozilla/b2g/gecko/js/xpconnect/src/XPCShellImpl.cpp:1602
#58 0x00007ffff3016ea5 in __libc_start_main (main=0x402e5c <main(int, char**, char**)>, argc=9, ubp_av=0x7fffffffdfb8, init=<optimized out>, fini=<optimized out>, rtld_fini=<optimized out>, 
    stack_end=0x7fffffffdfa8) at libc-start.c:260
#59 0x0000000000402d99 in _start ()

I don't see such a difference.

Comment 19

[:gerard-majax]

(In reply to :Paolo Amadini from comment #16)
> Comment on attachment 8381448 [details] [diff] [review]
> 0001-Bug-973637-Avoid-crash-at-shutdown-during-make-packa.patch
> 
> Review of attachment 8381448 [details] [diff] [review]:
> -----------------------------------------------------------------
> 
> ::: toolkit/components/jsdownloads/src/DownloadIntegration.jsm
> @@ +844,5 @@
> >      if (!DownloadObserver.observersAdded) {
> >        DownloadObserver.observersAdded = true;
> > +      for (let i in kObserverTopics) {
> > +        let topic = kObserverTopics[i];
> > +        Services.obs.addObserver(DownloadObserver, topic, true);
> 
> Ah, you may also try to pass false as the last parameter and remove the
> Ci.nsISupportsWeakReference below.
> 
> @@ +1052,5 @@
> > +      case "profile-before-change":
> > +      case "xpcom-shutdown":
> > +        for (let i in kObserverTopics) {
> > +          let topic = kObserverTopics[i];
> > +          Services.obs.removeObserver(this, topic, true);
> 
> ...and there is no third parameter to removeObserver.

Okay those changes are okay locally, thanks for catching those. Removing the nsISupportsWeakReference usage has no impact regarding the crash. As you can see in the previous comment, I don't see a big change in the backtrace we get even after making use of profile-before-change. I will ensure that we have no race in deregistering observers. Meanwhile, if you have any idea of things to check, feel free :).

Comment 20

[:gerard-majax]

Confere previous comment :)

Comment 21

[:Paolo Amadini]

(In reply to Alexandre LISSY :gerard-majax from comment #19)
> Okay those changes are okay locally, thanks for catching those. Removing the
> nsISupportsWeakReference usage has no impact regarding the crash. As you can
> see in the previous comment, I don't see a big change in the backtrace we
> get even after making use of profile-before-change.

Thanks for trying these out. I admit the backtrace is surprising, considering that the observers are unregistered. I assume you verified that the code being executed this time is inside Downloads.jsm, despite the C++ backtrace doesn't make it clear.

> I will ensure that we
> have no race in deregistering observers. Meanwhile, if you have any idea of
> things to check, feel free :).

I'd add a "dump" for each registration and deregistration - I can only think there might be an issue with the existing Downloads.jsm code, for which it doesn't do what it says, maybe only under your special building and testing conditions.

Another thing to try might be to deregister observers earlier in the shutdown process, there are several possible notifications to use:

https://developer.mozilla.org/Observer_Notifications#Application_shutdown

The earliest we could use is "quit-application".

Comment 22

[:gerard-majax]

(In reply to :Paolo Amadini from comment #21)
> (In reply to Alexandre LISSY :gerard-majax from comment #19)
> > Okay those changes are okay locally, thanks for catching those. Removing the
> > nsISupportsWeakReference usage has no impact regarding the crash. As you can
> > see in the previous comment, I don't see a big change in the backtrace we
> > get even after making use of profile-before-change.
> 
> Thanks for trying these out. I admit the backtrace is surprising,
> considering that the observers are unregistered. I assume you verified that
> the code being executed this time is inside Downloads.jsm, despite the C++
> backtrace doesn't make it clear.
> 
> > I will ensure that we
> > have no race in deregistering observers. Meanwhile, if you have any idea of
> > things to check, feel free :).
> 
> I'd add a "dump" for each registration and deregistration - I can only think
> there might be an issue with the existing Downloads.jsm code, for which it
> doesn't do what it says, maybe only under your special building and testing
> conditions.

Yes, that was my next move :)

> 
> Another thing to try might be to deregister observers earlier in the
> shutdown process, there are several possible notifications to use:
> 
> https://developer.mozilla.org/Observer_Notifications#Application_shutdown
> 
> The earliest we could use is "quit-application".

Instead of profile-before-change ?

Comment 23

[:Paolo Amadini]

(In reply to Alexandre LISSY :gerard-majax from comment #22)
> > The earliest we could use is "quit-application".
> 
> Instead of profile-before-change ?

Yeah, by the way right now I was working on a completely different thing (xpcshell test documentation), and I noticed a comment suggesting that the "profile-before-change" notification might not be raised in an xpcshell environment, unless the do_get_profile helper is called first, which we do in Downloads.jsm automated tests.

This could be the reason why the observers are not unregistered here, maybe your execution environment doesn't raise the profile notifications.

Comment 24

[:gerard-majax]

Okay, looks like profile-before-change did not got a chance to get called: no removeObserver() call seems to be reached.

Comment 25

[:gerard-majax]

(In reply to :Paolo Amadini from comment #23)
> (In reply to Alexandre LISSY :gerard-majax from comment #22)
> > > The earliest we could use is "quit-application".
> > 
> > Instead of profile-before-change ?
> 
> Yeah, by the way right now I was working on a completely different thing
> (xpcshell test documentation), and I noticed a comment suggesting that the
> "profile-before-change" notification might not be raised in an xpcshell
> environment, unless the do_get_profile helper is called first, which we do
> in Downloads.jsm automated tests.
> 
> This could be the reason why the observers are not unregistered here, maybe
> your execution environment doesn't raise the profile notifications.

Since it's during the cache precompilation step, that may be the rational behind this behavior.

Comment 26

[:gerard-majax]

None of profile-before-change, quit-application-requested or quit-application seems to be called in time. Adding an observer on xpcom-will-shutdown, however, gets called. What's your feeling on this ?

Comment 27

[:Paolo Amadini]

(In reply to Alexandre LISSY :gerard-majax from comment #26)
> None of profile-before-change, quit-application-requested or
> quit-application seems to be called in time. Adding an observer on
> xpcom-will-shutdown, however, gets called. What's your feeling on this ?

Looks good to me, with a comment in the source code about why we're doing this, mentioning the cache precompilation step.

Note that, since we don't have automated tests for this in our infrastructure, the comment alone won't protect us from possible regressions in the future, but any regression might be trivial to fix given that we know the cause now.

Comment 28

[:gerard-majax]

Attachment: 0001-Bug-973637-Avoid-crash-at-shutdown-during-make-packa.patch

Please find attached the updated version of the patch, with all your latest remarks addressed.

A Try build is pending at: https://tbpl.mozilla.org/?tree=Try&rev=9d744280abe7

Comment 29

[:Paolo Amadini]

Comment on attachment 8385394 [details] [diff] [review]
0001-Bug-973637-Avoid-crash-at-shutdown-during-make-packa.patch

Review of attachment 8385394 [details] [diff] [review]:
-----------------------------------------------------------------

r+ with the notes below. Thanks!

::: toolkit/components/jsdownloads/src/DownloadIntegration.jsm
@@ +135,5 @@
> +  "wake_notification",
> +  "resume_process_notification",
> +  "network:offline-about-to-go-offline",
> +  "network:offline-status-changed",
> +  "xpcom-will-shutdown" // Bug 973637: Listen on this to avoid segfault on Mulet during |make package|

No need to repeat comment here, and comma after last item in list is recommended

@@ +1047,4 @@
>            this._resumeOfflineDownloads();
>          }
>          break;
> +      // Bug 973637: Listen on this to avoid segfault on Mulet during |make package|

I'd spend a few more words here so that people don't need to go to the bug (feel free to elaborate as needed):

We need to unregister observers explicitly before we reach the "xpcom-shutdown" phase, otherwise observers may be notified when some required services are not available anymore. We can't unregister observers on "quit-application", because this module is also loaded during "make package" automation, and the quit notification is not sent in that execution environment (see bug 973637).

Comment 30

[:gerard-majax]

Your comment is nice, let me steal it.

Comment 31

[:gerard-majax]

Attachment: 0001-Bug-973637-Avoid-crash-at-shutdown-during-make-packa.patch

Better comment, stole from Paolo :)

Comment 32

[:gerard-majax]

Oops, let's wait Try before setting this.

Comment 33

[:gerard-majax]

Paolo, Try seems to be green, to you think we should run further test on this ?

Comment 34

[:Paolo Amadini]

(In reply to Alexandre LISSY :gerard-majax from comment #33)
> Paolo, Try seems to be green, to you think we should run further test on
> this ?

This is ready to land! Thanks for the patch!

Comment 35

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/integration/mozilla-inbound/rev/4cac2345b058

Comment 36

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/mozilla-central/rev/4cac2345b058