Closed Bug 1012386 Opened 6 years ago Closed 6 years ago

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

Categories

(Core :: Canvas: 2D, defect)

31 Branch
x86
macOS
defect
Not set

Tracking

()

VERIFIED FIXED
mozilla32
Tracking Status
firefox30 --- unaffected
firefox31 + verified
firefox32 --- verified

People

(Reporter: till, Assigned: mattwoodrow)

References

()

Details

(Keywords: regression, Whiteboard: [MemShrink])

Attachments

(1 file)

No description provided.
(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.
Keywords: regression
Summary: Canvas2D's getImageData/putImageData causes memory leak with huge values for "heap unclassified → Canvas2D's getImageData/putImageData causes memory leak with huge values for "heap-unclassified"
Whiteboard: [MemShrink]
Flags: needinfo?(matt.woodrow)
Assignee: nobody → matt.woodrow
Attachment #8424522 - Flags: review?(jmuizelaar)
Flags: needinfo?(matt.woodrow)
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?
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 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.
Attachment #8424522 - Flags: review?(jmuizelaar) → review+
https://hg.mozilla.org/mozilla-central/rev/6b57eca751f8
Status: NEW → RESOLVED
Closed: 6 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla32
Can this be uplifted to Aurora?
Flags: needinfo?(matt.woodrow)
Flags: needinfo?(matt.woodrow)
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
Attachment #8424522 - Flags: approval-mozilla-aurora?
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)
Attachment #8424522 - Flags: approval-mozilla-aurora? → approval-mozilla-aurora+
Keywords: verifyme
Reproduced in nightly 2014-05-18, OS X 10.9.2.
Verified fixed 32.0a1 (2014-05-26).
Status: RESOLVED → VERIFIED
"heap-unclassified" values stay low now.
Verified fixed FF 31b3 OS X 10.8.5.
You need to log in before you can comment on or make changes to this bug.