1012386 - Canvas2D's getImageData/putImageData causes memory leak with huge values for "heap-unclassified"

Status: VERIFIED FIXED

(Core :: Graphics: Canvas2D, defect)

Comment 1

[Till Schneidereit [:till]]

(Accidentally submitted the form, sorry.)

Getting a 2D Canvas's image data and putting it back in a requestAnimationFrame loop leaks huge amounts of memory. Permanently until the browser is restarted.

A script as simple as this demonstrates the issue:


function loop() {
  context.putImageData(context.getImageData(0, 0, canvas.width, canvas.height), 0, 0);
       
  requestAnimationFrame(loop);
}

var context = document.getElementById('canvas').getContext('2d');

loop();


With a 1000*1000px canvas, that leaks a few GB of memory within seconds, as the linked URL demonstrates.

Happens on Aurora for, but not on Beta.

Comment 2

[Matt Woodrow (:mattwoodrow)]

Attachment: Release the image

Comment 3

[Milan Sreckovic [:milan] (needinfo for best results)]

Comment on attachment 8424522 [details] [diff] [review]
Release the image

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

::: gfx/2d/DrawTargetCG.cpp
@@ +1252,5 @@
>      }
>      CGContextDrawImage(mCg, flippedRect, image);
>  
>      CGContextRestoreGState(mCg);
> +    CGImageRelease(image);

Is there a a preferred ordering between CGContextRestoreGState and CGImageRelease? Are they fully independent, or should one always come before the other?

Comment 4

[Markus Stange [:mstange]]

They're fully independent. The image doesn't remember which context it was painted into, and the context has already finished painting the image so it has probably already forgotten about it, too. The image can be released at any point after the CGContextDrawImage call where it was used.

Comment 5

[Jeff Muizelaar [:jrmuizel]]

Comment on attachment 8424522 [details] [diff] [review]
Release the image

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

::: gfx/2d/DrawTargetCG.cpp
@@ +1252,5 @@
>      }
>      CGContextDrawImage(mCg, flippedRect, image);
>  
>      CGContextRestoreGState(mCg);
> +    CGImageRelease(image);

They should be independent.

Comment 6

[Matt Woodrow (:mattwoodrow)]

https://hg.mozilla.org/integration/mozilla-inbound/rev/6b57eca751f8

Comment 7

[Carsten Book [:Tomcat]]

https://hg.mozilla.org/mozilla-central/rev/6b57eca751f8

Comment 8

[Till Schneidereit [:till]]

Can this be uplifted to Aurora?

Comment 9

[Matt Woodrow (:mattwoodrow)]

Comment on attachment 8424522 [details] [diff] [review]
Release the image

[Approval Request Comment]
Bug caused by (feature/regressing bug #): Bug 990854
User impact if declined: Memory leak with canvas putImageData
Testing completed (on m-c, etc.): Locally confirmed it fixes the memory leak.
Risk to taking this patch (and alternatives if risky): Very low risk.
String or IDL/UUID changes made by this patch: None

Comment 10

[bryan.head]

I just ran into this in Nightly (32.0a1 (2014-05-21), not sure if this patch has made it in yet). It appears that its due entirely to putImageData. See this fiddle: http://jsfiddle.net/vHzmW/2/

var canvas = document.querySelector('canvas'),
    ctx = canvas.getContext('2d'),
    imageData = ctx.getImageData(0, 0, canvas.width, canvas.height);
 
setInterval(function () {
  ctx.putImageData(imageData, 0, 0);
}, 0)

Comment 11

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/releases/mozilla-aurora/rev/4652cf70d98f

Comment 12

[Paul Silaghi, QA [:pauly]]

Reproduced in nightly 2014-05-18, OS X 10.9.2.
Verified fixed 32.0a1 (2014-05-26).

Comment 13

[Paul Silaghi, QA [:pauly]]

"heap-unclassified" values stay low now.
Verified fixed FF 31b3 OS X 10.8.5.