372766 - cycle collector shutdown dump based on known should-have-been-freed objects

Status: RESOLVED FIXED

(Core :: XPCOM, defect, P1)

Description

[David Baron :dbaron: (⌚️UTC-4, no longer working on Mozilla)]

I have code to do a shutdown dump of interesting objects that the cycle collector hasn't collected.

The basic idea is the following:
 * we register certain known objects with the cycle collector if we think
   they should be garbage.  (This isn't necessarily completely accurate;
   however, given that the dump is currently only at shutdown, it's unlikely
   to produce false positives.)
 * at shutdown, after the last cycle collection, the collector builds
   another graph using the expected-garbage objects as roots and then,
   instead of running collection, prints out warnings about:
   + any set of objects that are marked to be collected (which implies a
     missing suspect call)
   + any objects that have unaccounted-for references
   There should be at least one such warning for every expected-garbage
   object or related set of expected-garbage objects.

So far, most of the warnings I've been seeing are related to the global window (C++ object) having two unaccounted-for references.  I'm guessing at least one of them is XPCWrappedNativeScope::mScriptObjectPrincipal; it's probably not worth spending too much time analyzing until we've figured out how we want to fix the XPConnect traversal code.

I'm also seeing a bunch of JS objects having unaccounted-for references after loading http://www.google.com/ig .

Comment 1

[David Baron :dbaron: (⌚️UTC-4, no longer working on Mozilla)]

Attachment: patch

Comment 2

[David Baron :dbaron: (⌚️UTC-4, no longer working on Mozilla)]

Note that I don't necessarily think we should leave this in forever, but I think it will be useful while we try to fix the cycle collector leaks.

Comment 3

[Peter Van der Beken [:peterv]]

(In reply to comment #0)
> So far, most of the warnings I've been seeing are related to the global window
> (C++ object) having two unaccounted-for references.  I'm guessing at least one
> of them is XPCWrappedNativeScope::mScriptObjectPrincipal;

The odd thing is that in a tree with a couple of patches, looking at the graph after the last cycle collection the nsGlobalWindow objects seem to have all the references accounted for (2/2 for all of them), but your code complains about 2 missing external references.

Comment 4

[Graydon Hoare :graydon]

Comment on attachment 257443 [details] [diff] [review]
patch

Looks reasonable to me.

Comment 5

[Jesse Ruderman]

Shouldn't those printfs be assertions?

Comment 6

[Peter Van der Beken [:peterv]]

Comment on attachment 257443 [details] [diff] [review]
patch

>+            printf("nsCycleCollector: %s %p was not collected due to\n"
>+                   "  missing call to suspect\n",

This could also be due to not enough unlinking?

Comment 7

[David Baron :dbaron: (⌚️UTC-4, no longer working on Mozilla)]

(In reply to comment #5)
> Shouldn't those printfs be assertions?

Not yet, certainly.

Comment 8

[David Baron :dbaron: (⌚️UTC-4, no longer working on Mozilla)]

Checked in to trunk.