"Assertion failure: !mIsMapped (Someone forgot to call Unmap())" with canvas getImageData(huge)

RESOLVED FIXED in Firefox 41

Status

()

defect
--
critical
RESOLVED FIXED
4 years ago
4 years ago

People

(Reporter: jruderman, Assigned: lsalzman)

Tracking

(Blocks 1 bug, {assertion, testcase})

Trunk
mozilla42
Points:
---
Dependency tree / graph

Firefox Tracking Flags

(firefox41 fixed, firefox42 fixed)

Details

Attachments

(2 attachments)

Posted file testcase
Assertion failure: !mIsMapped (Someone forgot to call Unmap())

This assertion was added in https://hg.mozilla.org/mozilla-central/rev/856705b0c63f, not sure if the bug is a more recent regression.
Assignee: nobody → lsalzman
Status: NEW → ASSIGNED
The testcase causes an OOM allocating the destination array inside CanvasRenderingContext2D::GetImageDataArray. However, the source is mapped before this occurs, and not unmapped when the OOM is returned.

To avoid this, this just moves the destination array allocation up before the map, so that if an OOM happens and returns, there is no mapped source lying around to be unmapped in the first place.

With this patch, the test case no longer crashes.
Attachment #8637957 - Flags: review?(bas)
Attachment #8637957 - Flags: review?(bas) → review+
Keywords: checkin-needed
Comment on attachment 8637957 [details] [diff] [review]
check for OOM on destination array before mapping source to avoid needing unmap

Approval Request Comment
[Feature/regressing bug #]: https://hg.mozilla.org/mozilla-central/rev/536bd9910bc2
[User impact if declined]: JavaScript canvas2d code can be used to crash Firefox.
[Describe test coverage new/current, TreeHerder]: dom/canvas/test
[Risks and why]: Doesn't change code behavior, just causes OOM to be signaled earlier so as to avoid triggering assertion. Since the crash goes away once patched, we can assume the OOM is being handled properly now since no assertions are getting hit anymore.
[String/UUID change made/needed]: None
Attachment #8637957 - Flags: approval-mozilla-aurora?
can we get a try run here, thanks!
Flags: needinfo?(lsalzman)
Keywords: checkin-needed
(In reply to Carsten Book [:Tomcat] from comment #3)
> can we get a try run here, thanks!

https://treeherder.mozilla.org/#/jobs?repo=try&revision=35567d4763d6
Flags: needinfo?(lsalzman)
Keywords: checkin-needed
https://hg.mozilla.org/mozilla-central/rev/93994b700026
Status: ASSIGNED → RESOLVED
Closed: 4 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla42
Comment on attachment 8637957 [details] [diff] [review]
check for OOM on destination array before mapping source to avoid needing unmap

Simple fix. Let's uplift to Aurora and hope it helps.
Attachment #8637957 - Flags: approval-mozilla-aurora? → approval-mozilla-aurora+
You need to log in before you can comment on or make changes to this bug.