Last Comment Bug 521853 - make cycle collector faults fatal
: make cycle collector faults fatal
Status: RESOLVED FIXED
:
Product: Core
Classification: Components
Component: XPCOM (show other bugs)
: Trunk
: All All
: -- normal (vote)
: mozilla14
Assigned To: Andrew McCreight [:mccr8]
:
Mentors:
: 733460 (view as bug list)
Depends on:
Blocks:
  Show dependency treegraph
 
Reported: 2009-10-12 11:45 PDT by Benjamin Smedberg [:bsmedberg]
Modified: 2012-03-16 06:33 PDT (History)
11 users (show)
See Also:
Crash Signature:
(edit)
QA Whiteboard:
Iteration: ---
Points: ---
Has Regression Range: ---
Has STR: ---
-


Attachments
make cycle collector faults fatal, plus a little cleanup of faults (4.82 KB, patch)
2012-03-12 14:37 PDT, Andrew McCreight [:mccr8]
no flags Details | Diff | Review
make cycle collector faults fatal (4.16 KB, patch)
2012-03-15 09:29 PDT, Andrew McCreight [:mccr8]
bugs: review+
Details | Diff | Review
make cycle collector faults fatal (4.17 KB, patch)
2012-03-15 10:22 PDT, Andrew McCreight [:mccr8]
continuation: review+
Details | Diff | Review

Description Benjamin Smedberg [:bsmedberg] 2009-10-12 11:45:30 PDT
In bug 521750 we're going to make the cycle collector fault (instead of crashing) if external code uses it incorrectly off the main thread. This will fire a cycle-collector-fault observer notification. The user's browser is likely to leak like a sieve after the cycle collector faults, and they should restart at the first opportunity (and probably disable some extensions). This bug is about whatever UI Firefox needs to make this happen.
Comment 1 Benjamin Smedberg [:bsmedberg] 2009-10-12 11:47:13 PDT
Nominating for 3.6 but since we're past string freeze that doesn't seem likely.
Comment 2 Mike Beltzner [:beltzner, not reading bugmail] 2009-10-26 09:31:21 PDT
Yeah, past string freeze (wish someone brought it up last week when Axel was asking if there were any strings outstanding, or that I'd noticed it before then) so I don't think there's a lot we can do about this.

Adding user-doc-needed to get SUMO attention; post Firefox 3.6 we might see memory ramping due to lack of cycle collection. A better trade off than crashing, but we might want to add this to any "Firefox is using a lot of memory" documentation.

Benjamin: can you quantify "like a sieve"? Firefox 2 era memory leaks?
Comment 3 Benjamin Smedberg [:bsmedberg] 2009-10-26 09:48:51 PDT
No, worse, as in "gmail or facebook will use all your memory in 5-10 minutes if not sooner".
Comment 4 Kyle Huey [:khuey] (khuey@mozilla.com) 2012-01-16 07:40:58 PST
Given how reliant we are on the CC these days, I think we should just abort.  As a bonus, we'll be able to see when this happens in Socorro.

bsmedberg, do you have objections to changing the cc fault to NS_RUNTIMEABORT?
Comment 5 Benjamin Smedberg [:bsmedberg] 2012-01-31 09:14:59 PST
No, I think you should go for it (on -central). Please ask crashkill to watch for upticks related to this.
Comment 6 Andrew McCreight [:mccr8] 2012-03-06 15:40:15 PST
*** Bug 733460 has been marked as a duplicate of this bug. ***
Comment 7 Andrew McCreight [:mccr8] 2012-03-06 15:41:58 PST
khuey, if you don't have time to do this in the next week or so let me know and I can do it.  Probably best to wait until Firefox 14 at this point.
Comment 8 Andrew McCreight [:mccr8] 2012-03-12 14:37:07 PDT
Created attachment 605136 [details] [diff] [review]
make cycle collector faults fatal, plus a little cleanup of faults
Comment 9 Andrew McCreight [:mccr8] 2012-03-15 09:29:34 PDT
Created attachment 606250 [details] [diff] [review]
make cycle collector faults fatal

I did a try run on Linux for an earlier version that was fine.  I'll do a full try run before I land it.
Comment 10 Andrew McCreight [:mccr8] 2012-03-15 09:50:54 PDT
Comment on attachment 606250 [details] [diff] [review]
make cycle collector faults fatal

Does this sound reasonable to you, Peter?
Comment 11 Andrew McCreight [:mccr8] 2012-03-15 10:22:21 PDT
Created attachment 606279 [details] [diff] [review]
make cycle collector faults fatal

Gavin pointed out that I flipped the cases in the if.  I managed to mangle the code when I was undoing some changes I made before. Carrying forward smaug's review.
Comment 12 Andrew McCreight [:mccr8] 2012-03-15 21:16:41 PDT
https://hg.mozilla.org/integration/mozilla-inbound/rev/11660ce3640a

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