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

RESOLVED FIXED in Firefox 40

Status

()

defect
RESOLVED FIXED
4 years ago
4 years ago

People

(Reporter: diego, Assigned: sotaro)

Tracking

({crash})

unspecified
mozilla40
ARM
Gonk (Firefox OS)
Points:
---
Dependency tree / graph

Firefox Tracking Flags

(blocking-b2g:2.2+, firefox38 wontfix, firefox39 wontfix, firefox40 fixed, b2g-v2.2 fixed, b2g-master fixed)

Details

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

Attachments

(3 attachments)

Reporter

Description

4 years ago
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
Reporter

Comment 1

4 years ago
[Blocking Requested - why for this release]:
blocking-b2g: --- → 2.0?
Reporter

Updated

4 years ago
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
Reporter

Comment 5

4 years ago
Sotaro,

Looks like there's a race condition somewhere here. Does the mCameraControl null check make sense?
Flags: needinfo?(sotaro.ikeda.g)

Updated

4 years ago
blocking-b2g: 2.0? → 2.2?
blocking-b2g: 2.2? → 2.2+
Assignee

Comment 6

4 years ago
(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)
Assignee

Comment 8

4 years ago
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)
Assignee

Comment 9

4 years ago
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.
Assignee

Comment 11

4 years ago
Comment 9 is just one possibility of the crash. Therefore I am going to create a new bug for it.
Assignee

Updated

4 years ago
Depends on: 1153050
Assignee

Comment 12

4 years ago
Diego, can you confirm if the problem still happens?
Flags: needinfo?(dwilson)
Reporter

Comment 13

4 years ago
Sotaro,

We are currently patching gecko to avoid this crash
Flags: needinfo?(dwilson)
Assignee

Comment 14

4 years ago
(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)
Reporter

Comment 15

4 years ago
OK, I'll remove the patch from our builds and wait and see if the crash comes up again.
Flags: needinfo?(dwilson)
Assignee

Comment 16

4 years ago
Thanks!
Reporter

Comment 17

4 years ago
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: 4 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla40
Assignee

Comment 18

4 years ago
(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.