Multiple GCs required to clean up (strongparent, setUserData, addEventListener)

RESOLVED WORKSFORME

Status

()

defect
--
minor
RESOLVED WORKSFORME
8 years ago
2 months ago

People

(Reporter: jruderman, Unassigned)

Tracking

(Blocks 1 bug, {testcase})

Trunk
x86_64
macOS
Points:
---
Dependency tree / graph

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(1 attachment)

Reporter

Description

8 years ago
Posted file testcase
1. Apply the strongparent WIP6 patch (bug 335998 comment 105).
2. XPCOM_MEM_LEAK_LOG=2 firefox testcase.html
3. Quit Firefox.

Result: Leak nsDocument

Applying the shutdowngc patch (bug 664506) makes this leak go away, so it's not a severe bug, just a multiple-GCs-required thing.
Multiple GCs being required is expected when you have DOM-JS cycles.

- first GC marks JS part of the cycle as unreachable from a JS root (grey)
- cycle collection frees up the DOM part of the cycle
- second GC frees the JS part of the cycle

When I accidentally broke shutdown GC, only one GC ran on shutdown, so it is expected that there would be leaks.

Of course, the reality is that there is imperfect unlinking (fields aren't always unlinked, due to weirdness with destructors), so things may take longer than this to die, which is why there are 5 shutdown CCs (which include a GC).  It might be interesting to see what sticks around when you set DEFAULT_SHUTDOWN_COLLECTIONS to 2 or 3 instead of 5, but it would probably just turn up a bunch of known things that are difficult to fix.

Maybe this should be marked as invalid?

Updated

7 years ago
Depends on: 749981
Reporter

Comment 2

6 years ago
setUserData was removed for content in bug 842372.  So maybe this should be marked WFM.  mccr8, what do you think?
Flags: needinfo?(continuation)
Sure.  I think setUserData is on the way out in general.
Status: NEW → RESOLVED
Last Resolved: 6 years ago
Flags: needinfo?(continuation)
Resolution: --- → WORKSFORME
Component: DOM → DOM: Core & HTML
Product: Core → Core
You need to log in before you can comment on or make changes to this bug.