This bug was filed from the Socorro interface and is report bp-9e630ceb-7253-40dd-9e8f-8ea882151201. ============================================================= - install Greasemonkey (restart) - install user script from attachment - goto to https://volafile.io/r/404 (does not exist, but triggers user script) --> CRASH Even if the script was buggy it should not crash the entire browser. Works in 42 Release therefore a regression in 43 Beta.

Recent regression. https://hg.mozilla.org/integration/mozilla-inbound/pushloghtml?fromchange=793916ec6dd4c589a8d589967158ccff532527ac&tochange=0773712473c9cea41fa3a063f97cbd2dc55d86a4

FF45 CR: https://crash-stats.mozilla.com/report/index/22d1ecf3-d5e8-4a53-bef9-2727c2151202

The problem here is that I gave the module classes cached prototypes and added them to the list of classes in jsprototypes.h. This effectively made them into standard classes and made their presence visible outside of the shell which was not intended. In this case XrayTraits::resolveOwnProperty() tries to lookup the name 'Module' on a global object and decides that it's a standard class because JS_IdToProtoKey() returns a valid key for that id. Then it tries to get its constructor, but this has not been set and we crash (or assert in debug builds). Instead, we should store the prototypes for these classes elsewhere on the global as we do in other cases. The patch eagerly creates them when a shell global is created. This doesn't happen lazily as is the case for most other prototypes since we will want to create objects of these classes off main thread and would otherwise have to synchronise when initialising the prototype slots on the global. I verified that this fixed the crash.

Comment on attachment 8694816 [details] [diff] [review] firefox-modules-crash Review of attachment 8694816 [details] [diff] [review]: ----------------------------------------------------------------- This looks fine to me. I wonder if it regresses any memory benchmarks though. ::: js/src/builtin/ModuleObject.cpp @@ +541,5 @@ > nullptr, /* setProperty */ > nullptr, /* enumerate */ > nullptr, /* resolve */ > nullptr, /* mayResolve */ > + ModuleObject::finalize, o wow was it just never set before?

(In reply to Shu-yu Guo [:shu] from comment #4) > This looks fine to me. I wonder if it regresses any memory benchmarks though. Should be fine, but I guess we'll find out. > o wow was it just never set before? Yeah, I just spotted it when fixing this.

Needinfo'ing myself as a reminder to request uplift once this has baked on central for a couple of days.

While this does not show a huge # of crashes on 44.0a2, given that a patch is almost ready, I will track for 44 and hopefully uplift to Aurora44/Beta44 soon.

Too late for 43.

Comment on attachment 8694816 [details] [diff] [review] firefox-modules-crash Approval Request Comment [Feature/regressing bug #]: Bug 930414. [User impact if declined]: Possible browser crashes. [Describe test coverage new/current, TreeHerder]: On central since 4th December. [Risks and why]: Low. [String/UUID change made/needed]: None.

Jon, while reviewing the aurora44 (new beta44) uplift request, I noticed that this crash still occurred on FF45 (45.0a1 BuildId: 20151213030241). Perhaps, the crash isn't really fixed. Thoughts? Is it still worth uplifting as is? Please let me know.

(In reply to Ritu Kothari (:ritu) from comment #12) > Jon, while reviewing the aurora44 (new beta44) uplift request, I noticed > that this crash still occurred on FF45 (45.0a1 BuildId: 20151213030241). > Perhaps, the crash isn't really fixed. Thoughts? Is it still worth uplifting > as is? Please let me know. IMHO, the crash signatures for this bug are generic, so FF45 is still crashing with the same signature for different reasons.

Yes I think this is still worth uplifting. I confirmed that the fix is present on central and on the aurora branch, and that nightly does not crash anymore with these STR. Developer edition seems to be version 44 still so I couldn't test that unfortunately. I think what you're seeing must be as Loic said a different crash.

Terrence, Jon: Could you please nominate a patch for uplift to Beta44 and Aurora45 soon? I would like to include this fix in 44.0b2 (pushed to beta channel on Tuesday) if possible.

Here's a patch for beta. The fix has already merged to aurora. Tested locally and pushed a try run here: https://treeherder.mozilla.org/#/jobs?repo=try&revision=cb5ab071193b Approval Request Comment [Feature/regressing bug #]: Bug 930414. [User impact if declined]: Possible browser crashes. [Describe test coverage new/current, TreeHerder]: On central since 4th December. [Risks and why]: Low. [String/UUID change made/needed]: None.

Comment on attachment 8700577 [details] [diff] [review] bug1229493-beta-patch This is a crash fix, that has been verified and has stabilized in Nightly for over 2 weeks, seems safe to uplift to Beta44.