Closed Bug 730698 Opened 11 years ago Closed 7 years ago

repeatedly calling canvas getContext causes crash in Firefox

Categories

(Core :: Graphics: Canvas2D, defect)

10 Branch
x86_64
Windows 7
defect
Not set
critical

Tracking

()

RESOLVED INCOMPLETE

People

(Reporter: aaronklick, Assigned: smaug)

Details

(Keywords: crash, hang, testcase)

Attachments

(2 files)

Repeatedly calling canvas getContext in tight interval loop for particle system causes firefox to crash.

Firefox overloads in memory and crashes from the above.

What should happen is that, firefox properly handles garbage collection of out of scope javascript variables.

The same code can run in Chrome and Safari just fine without issues.

See attached example code project.

Run the firefoxcrashtest.html to see the crash happen. Run the index.html to see how it should work without issues.

the index.html uses particles.js. Particles.js only stores the canvas context once after the window has finished loading, that is why it works. It stores the context in a global javascript variable.

firefoxcrashtest.html uses firefoxCrashTest.js which as you can see on lines: 121 and 94 the context is repeatable stored in a variable in the function, instead of only once in global scope. Those two function are repeatedly called in the update() function which is called by an interval at every 16ms. 

The updateParticleDisplay() function is called for every particle. The total number of particles does not matter, it will crash with just 50 particles objects or 1000 particles. Of course, it crashes quicker with 1000 particles than with 50.
My Firefox (nightly on osx) doesn't crash (at least, not in the 10+ minutes I've had two of them open), but it does get sluggish and trigger slow script warning.

It also runs the script very slow, but I'm tempted to attribute this to the very high number of unneeded DOM operations, as your isCanvasSupported() function is run 60000+ times per second, and each time it creates a canvas element, and calls getContext() on it. This is on top of the 60000+ times per second it calls getElementById() (and getContext()) for no apparent reason.

All that being said, while My Google Chrome doesn't manage to run the crashtest smoothly, it does run it much faster than Firefox does, and it might be worth trying to figure out why that is.
OOM crash bp-20846c43-1e86-459b-a638-e25832120226 on
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
(In reply to Christian Sonne [:cers] from comment #1)
> My Firefox (nightly on osx) doesn't crash (at least, not in the 10+ minutes
> I've had two of them open), but it does get sluggish and trigger slow script
> warning.
> 
> It also runs the script very slow, but I'm tempted to attribute this to the
> very high number of unneeded DOM operations, as your isCanvasSupported()
> function is run 60000+ times per second, and each time it creates a canvas
> element, and calls getContext() on it. This is on top of the 60000+ times
> per second it calls getElementById() (and getContext()) for no apparent
> reason.
> 
> All that being said, while My Google Chrome doesn't manage to run the
> crashtest smoothly, it does run it much faster than Firefox does, and it
> might be worth trying to figure out why that is.

I know that the DOM operations were very high in the firefoxCrashTest.js. Did you run the regular index.html? The regular particles.js, in the index.html, only calls isCanvasSupported() and getContext() one time after the window is loaded. The rest of the time it is simply checking for a null state on a variable. So that cuts out the huge DOM operations. I just stumbled upon this issue today, while writing the original version which was firefoxCrashTest.js. However, I optimized it in the particles.js to cut out all those DOM operations.

Though, this is a major concern that firefox is not cleaning up and releasing memory on DOM operations / variables that have fallen out of scope.
Keywords: crash, hang
Product: Firefox → Core
QA Contact: general → general
Summary: repeatedly calling canvas getContext causes crash in Firefox 10.0.2 → repeatedly calling canvas getContext causes crash in Firefox
Version: 10 Branch → Trunk
Attached file WinDbg Log
WinDbg Log against Mozilla/5.0 (Windows NT 6.1; WOW64; rv:13.0) Gecko/20120226 Firefox/13.0a1 ID:20120226031015 for the OOM Issue.
The canvas code is calling JS_updateMallocCounter for its surface allocation.  Why is jseng not triggering GC here?
Assignee: nobody → general
Component: General → JavaScript Engine
QA Contact: general → general
Alice, would it be possible to get a regression range? This may be a regression from bug 679026.
That fix hasn't shipped in a release yet...  aaronklick@gmail.com, were you testing a nightly or a release build?
It was firefox 10.0.2 release. I never tried it on the nightly build.
(In reply to Bill McCloskey (:billm) from comment #6)
> Alice, would it be possible to get a regression range?

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
Alice0775, could you file that as a separate bug?  It's clearly different from whatever aaronklick@gmail.com is seeing in 10.0.2.

aaronklick@gmail.com, just to double-check, are you testing in a profile with no extensions installed?
(In reply to Boris Zbarsky (:bz) from comment #10)
> Alice0775, could you file that as a separate bug? 

Field Bug 730753
Severity: major → critical
Version: Trunk → 10 Branch
(In reply to Boris Zbarsky (:bz) from comment #10)
> Alice0775, could you file that as a separate bug?  It's clearly different
> from whatever aaronklick@gmail.com is seeing in 10.0.2.
> 
> aaronklick@gmail.com, just to double-check, are you testing in a profile
> with no extensions installed?

Yes, I just now tested it on a more powerful computer and with no extensions installed. It still crashes within a few seconds. I can get this to happen repeatedly. The first computer I tested on had 6gigs of ram. The second 8gigs.
Hmm.  I definitely can't reproduce on 10.0.2 over here (on Mac, though).  Memory use stabilizes around 500MB (with a peak of about 700MB at one point).

ccing some graphics folks, in case this is specific to the Azure context (possibly only with hardware acceleration?) and moving back to canvas....
Assignee: general → nobody
Component: JavaScript Engine → Canvas: 2D
QA Contact: general → canvas.2d
Whiteboard: [MemShrink]
(In reply to Boris Zbarsky (:bz) from comment #13)
> Hmm.  I definitely can't reproduce on 10.0.2 over here (on Mac, though). 
> Memory use stabilizes around 500MB (with a peak of about 700MB at one point).
> 
> ccing some graphics folks, in case this is specific to the Azure context
> (possibly only with hardware acceleration?) and moving back to canvas....

Well, I am running Windows 7 x64 both computers that it happens in. Both of them have nvidia cards. One has SLI 260s GTX, and the other has a 470 GTX. 

I also just tested on a third Windows 7 x64 laptop with a Nvidia 260m. The laptop just runs real slow but does not crash. The major difference that I can see is Nvidia drivers those computers are running. Laptop: Nvidia Driver Version 186.31. Desktop One: Nvidia Driver Version 295.73. Desktop Two: Nvidia Driver Version 295.73. So, from what I can tell the ones crashing are running the latest Nvidia Driver, where as the laptop is running a really old driver and it is not crashing.
Whiteboard: [MemShrink] → [MemShrink:P2]
Assignee: nobody → bugs
Whiteboard: [MemShrink:P2]
/zd ,c.d,c z/.mc d;lc';dsl 'cdscs
sc

d 
sd
c 
ds
ds
Shoot, :nical is also affected.
wooops sorry, was trying to reproduce a bug that happens in text areas so i took a random bug and wrote random stuff in the area. Looks like I accidentally hit tab and enter...
I, for one, don't use C/C++ (but I have some ideas about it from having used a further evolution, C#) so I couldn't personally fix this, but what I do know:
A.) Most developers who are smart will cache the rendering context, not throw it away for GC and get a new one next function call.
B.) Internally, you don't have to throw the object away either. getContext should return the same underlying C/C++ object every time (maybe even the same JS object every time?). This will probably make things easier on GC and overall make everything run faster.
> getContext should return the same underlying C/C++ object every time 

That's what it does, yes.  Which is why it's confusing to me that calling it every time vs just getting the value once and caching it makes a difference.

Of course I also can't reproduce the problem at all...

On the other hand, is that really the only difference between the two testcases?  The summary claims so, but it looks to me like the crashing one actually does a bunch of drawing and whatnot whereas the other one does absolutely nothing with the canvas, no?
Flags: needinfo?(aaronklick)
It's been over a year with no update. Does this issue persist?
Whiteboard: [CLOSEME:2016-01-05]
Resolved per whiteboard
Status: UNCONFIRMED → RESOLVED
Closed: 7 years ago
Flags: needinfo?(aaronklick)
Resolution: --- → INCOMPLETE
Whiteboard: [CLOSEME:2016-01-05]
You need to log in before you can comment on or make changes to this bug.