Last Comment Bug 651273 - cycle collector doesn't add marked JS objects correctly with WANT_ALL_TRACES
: cycle collector doesn't add marked JS objects correctly with WANT_ALL_TRACES
Status: RESOLVED FIXED
:
Product: Core
Classification: Components
Component: XPCOM (show other bugs)
: Trunk
: All All
: -- normal (vote)
: ---
Assigned To: Andrew McCreight [:mccr8]
:
Mentors:
Depends on:
Blocks: 466157
  Show dependency treegraph
 
Reported: 2011-04-19 16:06 PDT by Andrew McCreight [:mccr8]
Modified: 2011-05-04 17:32 PDT (History)
6 users (show)
See Also:
Crash Signature:
(edit)
QA Whiteboard:
Iteration: ---
Points: ---
Has Regression Range: ---
Has STR: ---


Attachments
fix WANT_ALL_TRACES CC by reverting nsXPConnect changes (1.26 KB, patch)
2011-04-22 12:09 PDT, Andrew McCreight [:mccr8]
peterv: review+
Details | Diff | Review

Description Andrew McCreight [:mccr8] 2011-04-19 16:06:11 PDT
In Bug 641910 I added code to prevent marked JS objects from being added to the cycle collector graph, so I removed the corresponding check for these objects from nsXPConnect::Traverse.  Unfortunately, I forgot that when WANT_ALL_TRACES is enabled, marked JS objects are still added to the cycle collector graph.  In a non-DEBUG_CC build of Firefox, WANT_ALL_TRACES is only enabled when there is a listener, so the main consequence of this change is that it breaks the cycle collector graph dumping code added by Bug 466157.

This could be fixed by adding in some kind of check for WantAllTraces into the test, but I think it makes more sense to just back out the changes I made to nsXPConnect::Traverse, because the mark bit checking should be quick.
Comment 1 Andrew McCreight [:mccr8] 2011-04-22 12:09:10 PDT
Created attachment 527826 [details] [diff] [review]
fix WANT_ALL_TRACES CC by reverting nsXPConnect changes
Comment 2 Andrew McCreight [:mccr8] 2011-05-02 13:35:51 PDT
I made a try server push against the latest mozilla-central.

http://tbpl.mozilla.org/?tree=Try&rev=4934df3ec265

Everything passed, except a couple of Android and JP tests that don't seem to be visible on m-c, and for whatever reason Windows opt hasn't even finished building yet.
Comment 3 Andrew McCreight [:mccr8] 2011-05-02 15:48:01 PDT
Mochitest 1 failed on WinXP opt, but otherwise the Windows opt builds passed.  I guess I should figure out what the deal is with that.
Comment 4 Andrew McCreight [:mccr8] 2011-05-03 07:52:51 PDT
I reran the Windows opt Mochitest 1 and they both came out clean, so I think this patch is good to go.
Comment 5 Andrew McCreight [:mccr8] 2011-05-04 08:24:09 PDT
http://hg.mozilla.org/mozilla-central/rev/a92024952aab

Note You need to log in before you can comment on or make changes to this bug.