B2G camera code doesn't hold monitor when calling Notify

RESOLVED FIXED in Firefox 32

Status

()

defect
RESOLVED FIXED
5 years ago
5 years ago

People

(Reporter: jesup, Assigned: jesup)

Tracking

({regression})

Trunk
mozilla32
ARM
Gonk (Firefox OS)
Points:
---
Dependency tree / graph

Firefox Tracking Flags

(blocking-b2g:1.4+, firefox29 wontfix, firefox30 wontfix, firefox31 wontfix, firefox32 fixed, b2g-v1.4 fixed, b2g-v2.0 fixed)

Details

Attachments

(1 attachment, 2 obsolete attachments)

This is in AllocImpl(), which calls mCallbackMonitor.Notify() without holding the monitor.

I'm not sure this ever worked right, except by luck.  A debug run should always have caught it.  Note that for a long while I couldn't build debug B2G locally due to a GCC crash.

Notify() requires holding the monitor.
WIP patch - I also had to disable camera rotation code to get debug builds to run; though really that's a separate bug and will be filed.  With this patch, gum Video works on a Flame
now without patch to disable rotation check (bug 1003006)
Attachment #8414275 - Attachment is obsolete: true
Attachment #8414280 - Attachment is obsolete: true
Attachment #8414281 - Flags: review?(mhabicher)
Assignee: nobody → rjesup
Comment on attachment 8414281 [details] [diff] [review]
Hold B2G camera callback monitor if we're going to call Notify() on it

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

Looks good to me.
Attachment #8414281 - Flags: review?(mhabicher) → review+
Keywords: regression
https://hg.mozilla.org/mozilla-central/rev/67184a49505d
Status: NEW → RESOLVED
Closed: 5 years ago
Resolution: --- → FIXED
I think we know this is a regression, but what's the impact of this on 1.4 if we don't fix this?
(In reply to Jason Smith [:jsmith] from comment #7)
> I think we know this is a regression, but what's the impact of this on 1.4
> if we don't fix this?

Clarified in IRC - makes sense to block on this now.
blocking-b2g: 1.4? → 1.4+
Whiteboard: [webrtc-uplift]
You need to log in before you can comment on or make changes to this bug.