Use a WeakMap to hold cross-zone wrappers for FinalizationRecordObjects
Categories
(Core :: JavaScript: GC, defect, P1)
Tracking
()
Tracking | Status | |
---|---|---|
firefox-esr91 | --- | unaffected |
firefox97 | --- | unaffected |
firefox98 | --- | unaffected |
firefox99 | --- | fixed |
People
(Reporter: jonco, Assigned: jonco)
References
(Regression)
Details
(Keywords: regression, sec-high, Whiteboard: [sec-survey])
Crash Data
Attachments
(4 files)
In bug 1749298 we added a map to count cross-zone wrappers between FinalizationRegistry targets and FinalizationRecordObjects so that we could calculate sweep groups correctly.
If we change this to put the wrappers in a WeakMap (used as a weak set) we will get this behaviour automatically and also preserve the wrappers. This will allow us to remove the non-standard tracing code for the record objects which traces cross-zone wrappers and has to have a comment explaining why this is safe when it normally wouldn't be.
Further, using the approach for WeakRefs will fix bug 1755725.
Assignee | ||
Updated•3 years ago
|
Assignee | ||
Comment 1•3 years ago
|
||
The non-standard tracing code turns out to be a problem for other reasons too. Marking s-s before duping bug 1756590 to this one.
Assignee | ||
Comment 3•3 years ago
|
||
Assignee | ||
Comment 4•3 years ago
|
||
This has two effects:
- it keeps the wrapper alive as long as the record is alive (because it's a
WeakMap key delegate) - it ensures the target zone and registry zone are swept in the same sweep
group.
This allows us to remove the special tracing of cross compartment wrappers for
records and that caused problems in bug 1755874. This becomes normal tracing of
finalization record objects from their own global.
It also allows us to remove the sweep group handling code from
FinalizationRegistry.cpp because the WeakMap will now handle this for us by
adding sweep group edges which are the reverse of those of the
cross-compartment wrappers.
Depends on D139397
Assignee | ||
Comment 5•3 years ago
|
||
Depends on D139398
Assignee | ||
Comment 6•3 years ago
|
||
Depends on D139399
Updated•3 years ago
|
Updated•3 years ago
|
Comment 7•3 years ago
|
||
I'm going to mark this sec-high because the dupe had a giant pile of GC-related crashes, so maybe something in there is bad.
Updated•3 years ago
|
Comment 8•3 years ago
|
||
Set release status flags based on info from the regressing bug 1749298
Updated•3 years ago
|
Comment 9•3 years ago
|
||
Part 1: Allows constructing a WeakMap without requiring a JSContext and some tidyup r=sfink
https://hg.mozilla.org/integration/autoland/rev/5a90c739791a3784973edb2a9ce05a00ba1a9045
https://hg.mozilla.org/mozilla-central/rev/5a90c739791a
Part 2: Use a WeakMap to hold cross-compartment wrappers to finalization record objects from their target's zone r=sfink
https://hg.mozilla.org/integration/autoland/rev/b12d3950f1b99b2402119f1be389c49a893eb2e4
https://hg.mozilla.org/mozilla-central/rev/b12d3950f1b9
Part 3: Check cross zone wrapper set was correct before clearing r=sfink
https://hg.mozilla.org/integration/autoland/rev/5a6d3934a62347983a0751dd5073f46bf8501ea6
https://hg.mozilla.org/mozilla-central/rev/5a6d3934a623
Part 4: Add test case reported in Bug 1756590 r=sfink
https://hg.mozilla.org/integration/autoland/rev/cdd59ce2a2dcfd5629b8c851722cf57a77610e71
https://hg.mozilla.org/mozilla-central/rev/cdd59ce2a2dc
Comment 10•3 years ago
|
||
As part of a security bug pattern analysis, we are requesting your help with a high level analysis of this bug. It is our hope to develop static analysis (or potentially runtime/dynamic analysis) in the future to identify classes of bugs.
Please visit this google form to reply.
Updated•3 years ago
|
Updated•2 years ago
|
Assignee | ||
Updated•7 months ago
|
Description
•