Closed
Bug 699796
Opened 13 years ago
Closed 13 years ago
Note objects holding JSScript as roots in the CC
Categories
(Core :: XPConnect, defect)
Core
XPConnect
Tracking
()
RESOLVED
FIXED
mozilla9
Tracking | Status | |
---|---|---|
firefox8 | --- | unaffected |
firefox9 | - | fixed |
firefox10 | - | fixed |
People
(Reporter: peterv, Assigned: peterv)
References
Details
(Whiteboard: [MemShrink:P1][qa-])
Attachments
(1 file)
3.72 KB,
patch
|
igor
:
review+
akeybl
:
approval-mozilla-aurora+
akeybl
:
approval-mozilla-beta+
|
Details | Diff | Splinter Review |
Bug 678830 made us add JSScript objects to the cycle collector, but it didn't change CheckParticipatesInCycleCollection to deal with script objects. We're not adding XPCOM objects holding a JSScript as roots to the cycle collector and as a result we're missing collectable cycles and leaking. I spent a lot of time tracking this down :-/. This is probably the cause of bug 687857, which shouldn't have been ignored as a regression from bug 678830.
Attachment #571968 -
Flags: review?(igor)
Comment 1•13 years ago
|
||
Peter, why do only some people see the leak in bug 687857? How does one trigger this?
Updated•13 years ago
|
Attachment #571968 -
Flags: review?(igor) → review+
Assignee | ||
Comment 2•13 years ago
|
||
(In reply to Justin Lebar [:jlebar] from comment #1) > Peter, why do only some people see the leak in bug 687857? How does one > trigger this? I don't know. The patch from bug 637099 made it almost always reproducible for me, but I haven't tried much without it.
Assignee | ||
Comment 3•13 years ago
|
||
(In reply to Peter Van der Beken [:peterv] from comment #2) > The patch from bug 637099 That patch makes us not break certain cycles manually, so we rely more on the CC. It might be order of unlinking or other randomness that saves us sometimes without it.
Comment 4•13 years ago
|
||
I see, thanks. Fingers crossed!
Updated•13 years ago
|
status-firefox10:
--- → affected
status-firefox8:
--- → unaffected
status-firefox9:
--- → affected
tracking-firefox10:
--- → ?
tracking-firefox9:
--- → ?
Whiteboard: [MemShrink:P1]
Comment 5•13 years ago
|
||
Fantastic work, Peter!
Comment 6•13 years ago
|
||
I'll try to figure out whether these edges were introduced in the initial scripts-as-gcthings or in the followup that got rid of the object wrappers, as that will affect whether this needs to go in 9 or just 10.
Assignee | ||
Comment 7•13 years ago
|
||
https://hg.mozilla.org/integration/mozilla-inbound/rev/848eaac421d9
Target Milestone: --- → mozilla10
Comment 8•13 years ago
|
||
https://hg.mozilla.org/mozilla-central/rev/848eaac421d9
Status: ASSIGNED → RESOLVED
Closed: 13 years ago
Resolution: --- → FIXED
Updated•13 years ago
|
Comment 9•13 years ago
|
||
Comment on attachment 571968 [details] [diff] [review] v1 [Triage Comment] Approving for aurora and beta, in the hopes that this low risk fix helps resolve 687857 and the FF9-specific negative input feedback around memory issues.
Attachment #571968 -
Flags: approval-mozilla-beta+
Attachment #571968 -
Flags: approval-mozilla-aurora+
Comment 10•13 years ago
|
||
I'll land this in beta tomorrow. It should already be in 10.
Comment 11•13 years ago
|
||
https://hg.mozilla.org/releases/mozilla-beta/rev/92f39c8a9cf3
Target Milestone: mozilla10 → mozilla9
Updated•13 years ago
|
Comment 12•13 years ago
|
||
Is there a testcase to verify this fix?
Whiteboard: [MemShrink:P1] → [MemShrink:P1][qa?]
Comment 13•13 years ago
|
||
Not really.
You need to log in
before you can comment on or make changes to this bug.
Description
•