Closed Bug 983399 Opened 6 years ago Closed 6 years ago

Make the component loader support moving GC

Categories

(Core :: JavaScript Engine, defect)

defect
Not set

Tracking

()

RESOLVED DUPLICATE of bug 1026860

People

(Reporter: terrence, Assigned: sfink)

References

Details

Attachments

(2 files)

I think we're green on maple currently because these objects happen to always be tenured. Sadly we need to switch to the fallible JS::HashMap to allow JS::Heap as a value or this would be totally trivial.
Attachment #8390832 - Flags: review?(sphink)
This probably deserves a try run, since I am touching browserish stuff.

https://tbpl.mozilla.org/?tree=Try&rev=8027af0c37dd
Attachment #8390832 - Flags: review?(sphink) → review+
It turns out the table is unused everywhere but B2G, which would also explain the lack of failures.

https://hg.mozilla.org/integration/mozilla-inbound/rev/db542ae460d9
It is, however, apparently used in b2g, so next time you'll probably want to run tests there ;)

Backed out in https://hg.mozilla.org/integration/mozilla-inbound/rev/ec684591268b for breaking debug b2g xpcshell, mostly dom/system/gonk/tests/test_ril_worker_mmi.js timeouts but also the odd test_ril_worker_icc.js.
This is what causes b2g ggc to break. With this patch applied, things look good: https://tbpl.mozilla.org/?tree=Try&rev=2706ba646a43

Ignore the SM(Hf) failure. It's because I built with a patch for the hazard build applied (a different compile of sixgill that shouldn't make a difference to the build, but will make it easier for people to run the analysis locally, except it hasn't been uploaded to the tooltool server yet.)
Attachment #8434607 - Flags: review?(terrence)
Assignee: terrence → sphink
Blocks: 1020751
Er, ok, https://tbpl.mozilla.org/?tree=Try&rev=2706ba646a43 isn't looking so good now. ICS emulator M(3) is unhappy.
Still, this looks better than the similar patch I crafted before. I guess we're currently just leaking these scripts and not touching them later?
Pushed https://tbpl.mozilla.org/?tree=Try&rev=d811e2e2eccc since bholley would like to see if it'll help bug 950653.
(In reply to Terrence Cole [:terrence] from comment #6)
> Still, this looks better than the similar patch I crafted before. I guess
> we're currently just leaking these scripts and not touching them later?

Doesn't this patch make us mark stuff we weren't marking? I would not expect that to fix a leak. It should fix use-after-free or use-after-move.
Attachment #8434607 - Flags: review?(terrence) → review?(bobbyholley)
Depends on: 1026860
Comment on attachment 8434607 [details] [diff] [review]
Make mozComponentLoader work with moving GC

I'm going to try removing this map altogether in bug 1026860.
Attachment #8434607 - Flags: review?(bobbyholley)
I think bug 1026860 did the trick here.
Status: ASSIGNED → RESOLVED
Closed: 6 years ago
Resolution: --- → DUPLICATE
Duplicate of bug: 1026860
You need to log in before you can comment on or make changes to this bug.