Closed Bug 1150271 Opened 5 years ago Closed 5 years ago

WebRTC session crashes in mozilla::MediaEngineGonkVideoSource::StartImpl()

Categories

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

ARM
Gonk (Firefox OS)
defect
Not set
normal

Tracking

()

RESOLVED FIXED
mozilla40
blocking-b2g 2.2+
Tracking Status
firefox38 --- wontfix
firefox39 --- wontfix
firefox40 --- fixed
b2g-v2.2 --- fixed
b2g-master --- fixed

People

(Reporter: diego, Assigned: sotaro)

References

Details

(Keywords: crash, Whiteboard: [b2g-crash][caf-crash 595][caf priority: p1][CR 815094])

Attachments

(3 files)

There's a hard to reproduce null dereference crash in MediaEngineGonkVideoSource.

My best guess is it's we need to null check mCameraControl [1]

[1] https://mxr.mozilla.org/mozilla-central/source/dom/media/webrtc/MediaEngineGonkVideoSource.cpp#473
[Blocking Requested - why for this release]:
blocking-b2g: --- → 2.0?
Whiteboard: [CR 815094]
Whiteboard: [CR 815094] → [caf priority: p1][CR 815094]
Whiteboard: [caf priority: p1][CR 815094] → [b2g-crash][caf-crash 595][caf priority: p1][CR 815094]
Keywords: crash
Sotaro,

Looks like there's a race condition somewhere here. Does the mCameraControl null check make sense?
Flags: needinfo?(sotaro.ikeda.g)
blocking-b2g: 2.0? → 2.2?
blocking-b2g: 2.2? → 2.2+
(In reply to Diego Wilson [:diego] from comment #5)
> Looks like there's a race condition somewhere here. Does the mCameraControl
> null check make sense?

I just looked the source, the implementation seems very tricky. I am going to look into more.
Assignee: nobody → sotaro.ikeda.g
Flags: needinfo?(sotaro.ikeda.g)
Could we know if there is any update? Thanks.
Flags: needinfo?(sotaro.ikeda.g)
The following functions could be related to the crash. They have a common characteristics. They are called on media thread(MediaManager's thread) and actual task is done on main thread. And They synchronously wait the task's completion.

- MediaEngineGonkVideoSource::Allocate()
    - allocate ICameraControl
- MediaEngineGonkVideoSource::Start()
    - Trigger MediaEngineGonkVideoSource::StartImpl() call.
- MediaEngineGonkVideoSource::Deallocate()
    - Clear ICameraControl pointer.
Flags: needinfo?(sotaro.ikeda.g)
ICameraControl does not call valid callback function until ICameraControl::Start() is called. Therefore, it seems not possible that ICameraControl is cleared between MediaEngineGonkVideoSource::Start() and MediaEngineGonkVideoSource::StartImpl().

Instead it seems possible that ICameraControl is nullptr when MediaEngineGonkVideoSource::Start() is called. MediaEngineGonkVideoSource::Start() does not check ICameraControl pointer.
Comment 9 is just one possibility of the crash. Therefore I am going to create a new bug for it.
Depends on: 1153050
Diego, can you confirm if the problem still happens?
Flags: needinfo?(dwilson)
Sotaro,

We are currently patching gecko to avoid this crash
Flags: needinfo?(dwilson)
(In reply to Diego Wilson [:diego] from comment #13)
> Created attachment 8592308 [details] [diff] [review]
> 0001-Bug-1150271-WebRTC-session-crashes-in-mozilla-MediaE.patch
> 
> Sotaro,
> 
> We are currently patching gecko to avoid this crash

diego, can it be removed? If the code of MediaEngineGonkVideoSource works as expected, this bugs fix should prevent the crash.
Flags: needinfo?(dwilson)
OK, I'll remove the patch from our builds and wait and see if the crash comes up again.
Flags: needinfo?(dwilson)
OK. I removed the patch from our builds. I'll mark this bug as resolved and reopen if I see the crash again. Thanks for your help sotaro!
Status: NEW → RESOLVED
Closed: 5 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla40
(In reply to Diego Wilson [:diego] from comment #17)
> OK. I removed the patch from our builds. I'll mark this bug as resolved and
> reopen if I see the crash again. Thanks for your help sotaro!

No problem :-)
You need to log in before you can comment on or make changes to this bug.