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

Status: RESOLVED WORKSFORME

(Core :: DOM: Core & HTML, defect)

Description

[Jesse Ruderman]

Attachment: 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.

Comment 1

[Andrew McCreight [:mccr8]]

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?

Comment 2

[Jesse Ruderman]

setUserData was removed for content in bug 842372.  So maybe this should be marked WFM.  mccr8, what do you think?

Comment 3

[Andrew McCreight [:mccr8]]

Sure.  I think setUserData is on the way out in general.