1003003 - B2G camera code doesn't hold monitor when calling Notify

Status: RESOLVED FIXED

(Core :: WebRTC: Audio/Video, defect)

Description

[Randell Jesup [:jesup] (needinfo me)]

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.

Comment 1

[Randell Jesup [:jesup] (needinfo me)]

Attachment: Hold B2G camera callback monitor if we're going to call Notify() on it

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

Comment 2

[Randell Jesup [:jesup] (needinfo me)]

Attachment: Hold B2G camera callback monitor if we're going to call Notify() on it

now without patch to disable rotation check (bug 1003006)

Comment 3

[Randell Jesup [:jesup] (needinfo me)]

Attachment: Hold B2G camera callback monitor if we're going to call Notify() on it

Comment 4

[Mike Habicher [:mikeh] (high bugzilla latency)]

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.

Comment 5

[Randell Jesup [:jesup] (needinfo me)]

https://hg.mozilla.org/integration/mozilla-inbound/rev/67184a49505d

Comment 6

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/mozilla-central/rev/67184a49505d

Comment 7

[Jason Smith [:jsmith]]

I think we know this is a regression, but what's the impact of this on 1.4 if we don't fix this?

Comment 8

[Jason Smith [:jsmith]]

(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.

Comment 9

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/releases/mozilla-b2g30_v1_4/rev/70be3f0cec21