OOM crash with repeatedly calling canvas getContext

RESOLVED FIXED in mozilla13

Status

()

Core
DOM
--
critical
RESOLVED FIXED
6 years ago
5 years ago

People

(Reporter: Alice0775 White, Assigned: smaug)

Tracking

({crash, regression, testcase})

12 Branch
mozilla13
x86
Windows 7
crash, regression, testcase
Points:
---

Firefox Tracking Flags

(firefox12- affected, firefox13-)

Details

(Whiteboard: [MemShrink:P2])

Attachments

(3 attachments)

(Reporter)

Description

6 years ago
Build Identifier:
http://hg.mozilla.org/mozilla-central/rev/ce20e9b47e9c
Mozilla/5.0 (Windows NT 6.1; WOW64; rv:13.0) Gecko/20120225 Firefox/13.0a1 ID:20120225031723

When I test Bug 730698 attachment 600788 [details], browser crashes bp-20846c43-1e86-459b-a638-e25832120226


Reproducible: Always

Steps to Reproduce:
1. Start Firefox with new profile
2. Run firefoxcrashtest.html of attachment 600788 [details]

Actual Results:
  Browser crashes

Expected Results:
  No crash

Regression window(m-c)
Browser does not crash for about 1 min. And I can barely operate it:
http://hg.mozilla.org/mozilla-central/rev/f410bdf30132
Mozilla/5.0 (Windows NT 6.1; WOW64; rv:12.0a1) Gecko/20120130 Firefox/12.0a1 ID:20120130122131
Browser crashes within 30sec:
http://hg.mozilla.org/mozilla-central/rev/89bb79343f73
Mozilla/5.0 (Windows NT 6.1; WOW64; rv:12.0a1) Gecko/20120130 Firefox/12.0a1 ID:20120130125731
Pushlog:
http://hg.mozilla.org/mozilla-central/pushloghtml?fromchange=f410bdf30132&tochange=89bb79343f73
This sounds like a CC regression...
tracking-firefox12: --- → ?
tracking-firefox13: --- → ?

Updated

6 years ago
Keywords: crash, regression, testcase
(Assignee)

Comment 2

6 years ago
My initial guess is that we end up calling CC fewer times, so we don't release memory
fast enough.
Assignee: nobody → bugs
(Assignee)

Comment 3

6 years ago
Unfortunately I can't reproduce this on linux.
(Assignee)

Comment 4

6 years ago
I'll post a patch to tryserver. Hopefully someone can test it.
(Assignee)

Comment 5

6 years ago
Created attachment 600877 [details] [diff] [review]
patch

This brings back similar behavior what we have in <FF12.
Yes, there can be some more CCs, but fortunately they are a lot faster.
http://mxr.mozilla.org/mozilla-beta/source/dom/base/nsJSEnvironment.cpp#3430
http://mxr.mozilla.org/mozilla-beta/source/dom/base/nsJSEnvironment.cpp#3339

https://tbpl.mozilla.org/?tree=Try&rev=26f8245eb5b0

Please test the tryserver build
Look for 26f8245eb5b0 in
http://ftp.mozilla.org/pub/mozilla.org/firefox/try-builds/
Attachment #600877 - Flags: review?(continuation)
Comment on attachment 600877 [details] [diff] [review]
patch

Review of attachment 600877 [details] [diff] [review]:
-----------------------------------------------------------------

Makes sense.  We really have no idea how much garbage a GC could have released, so we should err on the side of caution.

::: dom/base/nsJSEnvironment.cpp
@@ +180,5 @@
>  static PRUint32 sForgetSkippableBeforeCC = 0;
>  static PRUint32 sPreviousSuspectedCount = 0;
>  
>  static bool sCleanupSinceLastGC = true;
> +static bool sNeedsFullCC = false;

Do you need to set this to false in that init function, too?
Attachment #600877 - Flags: review?(continuation) → review+
(Assignee)

Comment 7

6 years ago
(In reply to Andrew McCreight [:mccr8] from comment #6)
> > +static bool sNeedsFullCC = false;
> 
> Do you need to set this to false in that init function, too?
I could do that, just to be consistent.

(Someone should cleanup nsJSEnvironment.cpp)
(Assignee)

Comment 8

6 years ago
Created attachment 600897 [details] [diff] [review]
patch
(Assignee)

Comment 9

6 years ago
Comment on attachment 600897 [details] [diff] [review]
patch

[Approval Request Comment]
Regression caused by (bug #): bug 721543
User impact if declined: possible OOM in rare cases
Testing completed (on m-c, etc.): just landed to m-c
Risk to taking this patch (and alternatives if risky): bringing back old behavior,
should be low risk.
String changes made by this patch: N/A
Attachment #600897 - Flags: approval-mozilla-aurora?
(Assignee)

Comment 10

6 years ago
https://hg.mozilla.org/mozilla-central/rev/8ea5c983743f
(Assignee)

Comment 11

6 years ago
Could someone please test http://ftp.mozilla.org/pub/mozilla.org/firefox/try-builds/opettay@mozilla.com-26f8245eb5b0/
I wonder if this will help with the intermittent hang I was getting in bug 727965.
(Reporter)

Comment 13

6 years ago
(In reply to Olli Pettay [:smaug] from comment #11)
> Could someone please test
> http://ftp.mozilla.org/pub/mozilla.org/firefox/try-builds/opettay@mozilla.
> com-26f8245eb5b0/

The try build crashes after 15-30sec :(
(Assignee)

Comment 14

6 years ago
Boo. Then it is about something else.
We should still use the patch.

Could you check if GC and/or CC runs while running the test.
Set javascript.options.mem.log to true and check error console Messages.
(Reporter)

Comment 15

6 years ago
And latest m-c tinderbox-builds also crashes after 15-30sec.
bp-2176ffdf-c6e3-4216-9f32-625232120227

http://hg.mozilla.org/mozilla-central/rev/8ea5c983743f
Mozilla/5.0 (Windows NT 6.1; WOW64; rv:13.0) Gecko/20120227 Firefox/13.0a1 ID:20120227065949
tracking-firefox12: ? → ---
tracking-firefox13: ? → ---
(Reporter)

Comment 16

6 years ago
Sorry, Accidentally removed flags
tracking-firefox12: --- → ?
tracking-firefox13: --- → ?
(Reporter)

Comment 17

6 years ago
(In reply to Olli Pettay [:smaug] from comment #14)
> Could you check if GC and/or CC runs while running the test.
> Set javascript.options.mem.log to true and check error console Messages.

It is difficult to capture the log because the browser crashes and error console closes at same time.


GC(T+12.8) TotalTime: 3.4ms, Type: compartment, MMU(20ms): 83%, MMU(50ms): 93%, Reason: TOO_MUCH_MALLOC, +chunks: 0, -chunks: 0
mark: 1.5, mark-roots: 1.1, mark-other: 0.0, sweep: 1.6, sweep-obj: 0.5, sweep-string: 0.0, sweep-script: 0.0, sweep-shape: 0.0, discard-code: 0.0, discard-analysis: 0.1, xpconnect: 0.6, deallocate: 0.0

GC(T+13.6) TotalTime: 2.7ms, Type: compartment, MMU(20ms): 86%, MMU(50ms): 94%, Reason: TOO_MUCH_MALLOC, +chunks: 0, -chunks: 0
mark: 1.2, mark-roots: 0.9, mark-other: 0.0, sweep: 1.3, sweep-obj: 0.4, sweep-string: 0.0, sweep-script: 0.0, sweep-shape: 0.0, discard-code: 0.0, discard-analysis: 0.1, xpconnect: 0.4, deallocate: 0.0

GC(T+14.5) TotalTime: 2.9ms, Type: compartment, MMU(20ms): 85%, MMU(50ms): 94%, Reason: TOO_MUCH_MALLOC, +chunks: 0, -chunks: 0
mark: 1.2, mark-roots: 0.9, mark-other: 0.0, sweep: 1.5, sweep-obj: 0.5, sweep-string: 0.0, sweep-script: 0.0, sweep-shape: 0.0, discard-code: 0.0, discard-analysis: 0.1, xpconnect: 0.5, deallocate: 0.0
(Reporter)

Comment 18

6 years ago
Comment #17 is the Nightly nightly build.
http://hg.mozilla.org/mozilla-central/rev/d1b2fd680235
Mozilla/5.0 (Windows NT 6.1; WOW64; rv:13.0) Gecko/20120227 Firefox/13.0a1 ID:20120227031120
(Assignee)

Comment 19

6 years ago
Oh, only compartment GC. That doesn't trigger CC.
(Assignee)

Comment 20

6 years ago
Have we changed when full GC is triggered?
Whiteboard: [MemShrink]
Depends on: 730853
Comment on attachment 600897 [details] [diff] [review]
patch

[Triage Comment]
Not clear this is absolutely necessary, but restores old behavior so approving for Aurora 12.
Attachment #600897 - Flags: approval-mozilla-aurora? → approval-mozilla-aurora+
(Assignee)

Comment 22

6 years ago
Created attachment 601219 [details] [diff] [review]
for aurora

Comment 23

6 years ago
Is it fixed in the trunk?
status-firefox12: --- → affected
Target Milestone: --- → mozilla13
(Assignee)

Comment 24

6 years ago
This depends on bug 730853. The patch fixes one issue.
(Assignee)

Comment 25

6 years ago
https://hg.mozilla.org/releases/mozilla-aurora/rev/04c1060b1a97
With this patch (well, and my patch in bug 728460), I'm seeing about 80% of CCs being triggered by this force CC.  It kind of makes the CC hostage to the GC if we want to try to call the CC less.  Kind of a shame, but I think this patch is probably the way to go, because otherwise you'll hit situations like this that are problematic.
Whiteboard: [MemShrink] → [MemShrink:P2]
Olli: would it make sense to trigger a CC every 5 or 10 compartmental GCs?
This probably belongs in another bug, but another approach to getting the GC to trigger a CC would be to dump all wrapped natives (etc.) into the purple buffer at the conclusion of a GC.  Then we could filter out everything held by a marked object using the normal methods.  That would probably work for this particular case, where it is allocating a huge number of cross-compartment cycles.

Updated

6 years ago
status-firefox12: affected → fixed
tracking-firefox12: ? → +
tracking-firefox13: ? → -

Comment 29

6 years ago
Bug 730853 didn't land in 12.0 so it's not fully fixed.
status-firefox12: fixed → affected
(In reply to Scoobidiver from comment #29)
> Bug 730853 didn't land in 12.0 so it's not fully fixed.

Given the original risk evaluation that this would only occur in rare cases, untracking for 12 since we wouldn't take any more forward change for this.
tracking-firefox12: + → -
I think this is finished, correct?
Status: NEW → RESOLVED
Last Resolved: 5 years ago
Resolution: --- → FIXED
You need to log in before you can comment on or make changes to this bug.