Closed Bug 420670 Opened 12 years ago Closed 11 years ago

warnings about insufficient cycle collection traversal related to editor

Categories

(Core :: Editor, defect, P2)

defect

Tracking

()

RESOLVED FIXED

People

(Reporter: dbaron, Assigned: dbaron)

References

Details

(Keywords: fixed1.9.1, memory-leak, Whiteboard: [depends on 423233+488799])

Attachments

(1 file)

When I:
 * start firefox
 * open a second window
 * close the first window
 * wait for cycle collection
 * close the remaining window
using a DEBUG_CC build, I often see the following warnings:

nsCycleCollector: nsJSEventListener 0x753b40 in component 19962
  was not collected due to missing call to suspect, failure to unlink,
  or deficiency in traverse that causes cycles referenced only from other
  cycles to require multiple rounds of cycle collection
nsCycleCollector: nsJSEventListener 0x753840 in component 19962
  was not collected due to missing call to suspect, failure to unlink,
  or deficiency in traverse that causes cycles referenced only from other
  cycles to require multiple rounds of cycle collection
nsCycleCollector: nsGenericElement 0x752eb0 in component 19962
  was not collected due to missing call to suspect, failure to unlink,
  or deficiency in traverse that causes cycles referenced only from other
  cycles to require multiple rounds of cycle collection
nsCycleCollector: nsEventListenerManager 0x753700 in component 19962
  was not collected due to missing call to suspect, failure to unlink,
  or deficiency in traverse that causes cycles referenced only from other
  cycles to require multiple rounds of cycle collection

This happens because the event listener manager is released during the unlink phase, yet the references that were released were not traversed, so the cycle collector doesn't know to break this additional cycle.  In other words, we have a cycle like this:

    (A) --> (C)<-,
     ^       |  (E)
     |       V   ^
     V      (D)--'
    (B)

where (A) and (B) are vastly simplified, (C) is the event listener manager, (D) is the nsJSEventListener, and (E) is the nsGenericElement.

Since we're not traversing the A->C link, this graph requires two cycles of cycle collection to clean up, one to clean up A and B, and the next to clean up C, D, and E.

However, if there were also a link from (E) to (B), we'd have an uncollectable cycle due to traversal deficiencies.  If this is actually possible, this bug becomes much more serious -- we need to figure out whether it is.

Probably the way to fix it would be to make editor participate in cycle collection.
Sorry, my diagnosis was incorrect.  Release of the editor caused a lot of refcount traffic on the event listener manager, but I just pulled the whole snippet out of the log, and the overall balance is 0.  That means, by a process of elimination (since I already examined the refcount traffic for both nsJSEventListener), that (C) is the element, (D) is the event listener manager, and (E) is the nsJSEventListener (or two).

The potential fix still stands, though.  (It's probably worth double-checking that the editor really is releasing the element, but it's pretty likely.)
Summary: warnings about insufficient cycle collection traversal related to editor event listeners → warnings about insufficient cycle collection traversal related to editor
If we want to get DEBUG_CC or leak monitor usage cleaned up for tinderboxes we probably need to get this sorted out.
Flags: blocking1.9?
Marking this wanted1.9.0.x.  We, do need this, and patch appreciated.
Flags: wanted1.9.0.x+
Flags: blocking1.9?
Flags: blocking1.9-
Flags: wanted1.9.1?
Flags: wanted1.9.1? → wanted1.9.1+
So I think this is the biggest blocker for making our various realtime leak debugging tools (DEBUG_CC, leak-monitor) useful again.  In other words, this is what's preventing us from getting better memory leak regression tests (see bug 435915) and probably from finding other leak bugs.
Flags: blocking1.9.1?
Depends on: 423233
Note that the patch in bug 423233 fixes this problem.

I'd also note that I'm a bit uncomfortable shipping without this fixed, but I'm even more uncomfortable shipping without it fixed on mozilla-central, because we don't know what other problems are lurking behind it.
Flags: blocking1.9.1? → blocking1.9.1+
I retested (two or three times) today and I don't see that patch fixing the problem, anymore, actually.  (But I'll try with a patch I wrote to also cycle collect transactions and see if *that* helps...)
Depends on: 488799
Whiteboard: [depends on 423233+488799]
Assignee: nobody → dbaron
Fixed on m-c by landings of patches in bug 423233 and bug 488799.
Status: NEW → RESOLVED
Closed: 11 years ago
Resolution: --- → FIXED
Fixed on 1.9.1 by landings of bug 423233 and bug 488799.
Keywords: fixed1.9.1
You need to log in before you can comment on or make changes to this bug.