1052169 - webrtc::ViEEncoder crashes while WebRTC session is closing

Status: RESOLVED FIXED

(Core :: WebRTC, defect)

Description

[Diego Wilson [:diego]]

I see two crashes (attached) repeated several times while closing a device-to-device WebRTC session.

STR:

1. Set up WebRTC session on phone 1 and phone 2.
2. Close session on phone 1.
3. Attempt to close session on phone 2.
4. App crashes!

Comment 1

[Diego Wilson [:diego]]

Attachment: webrtc::ViEEncoder::SendStatistics() crash minidump

Comment 2

[Diego Wilson [:diego]]

Attachment: webrtc::ViEEncoder::SendStatistics() crash extra

Comment 3

[Diego Wilson [:diego]]

Attachment: webrtc::ViEEncoder::~ViEEncoder() crash minidump

Comment 4

[Diego Wilson [:diego]]

Attachment: webrtc::ViEEncoder::~ViEEncoder() crash extra

Comment 5

[Diego Wilson [:diego]]

[Blocking Requested - why for this release]: This is a major stability issue on one of the main features for V2.0

Comment 6

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

I presume these were 2.0 builds; were they running on a KK base?  Were they using H.264?  Opt or debug?

"Set up WebRTC session on phone 1 and phone 2" - session using?  talky.io?  apptrtc?  Loop app?

"2. Close session on phone 1.
3. Attempt to close session on phone 2.
4. App crashes!"

Implies to me that it's the loop app.  Is there a timing component?

Thanks

Comment 7

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

Also, if it's H.264, can you try with VP8?

Comment 8

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

One more question: is this a (recent?) regression?  Since I haven't seen this before or heard it reported; I'm guessing it's a regression (unless it's a timing bug we hadn't hit before)

Comment 9

[Diego Wilson [:diego]]

Attachment: Logcat during crashes

Comment 10

[Diego Wilson [:diego]]

These crashes happened on a KK 8x10 build with H.264 enabled (no debug).

I'm using my own homegrown test app based on SimpleWebRTC library [1]. It's not very different from the sample app in the project itself.

I want to say it's a recent regression, but I would have to do some extra testing to confirm.

https://github.com/HenrikJoreteg/SimpleWebRTC

Comment 11

[Diego Wilson [:diego]]

This is only reproducible with H.264. It works fine when I configure for VP8.

Comment 12

[Diego Wilson [:diego]]

Also, this is not reproducible on talky.io with the H.264 config in comment 10

Comment 13

[Diego Wilson [:diego]]

As per discussion with Randell, this may be related to bug 1047442

Comment 14

[[:SalvadorR]]

This will need a new test case to be covered.

Comment 15

[KTucker [:KTucker][Inactive 3/4/2016]]

Test case added in moztrap:

https://moztrap.mozilla.org/manage/case/14330/

Comment 16

[Diego Wilson [:diego]]

I tested attachment 8466234 [details] [diff] [review] in bug 1047442. It didn't help :(

Comment 17

[Diego Wilson [:diego]]

Attachment: Logcat during crashes v2

Added "NSPR_LOG_MODULES=signaling:5,webrtc_trace:65535 WEBRTC_TRACE_FILE=nspr" for extra logging as per Randell's request

Comment 18

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

Attachment: Ignore decode-complete callbacks during OMX decoder shutdown WIP

Diego: give this a try.  It appears it would resolve the problem I saw in the last log you uploaded; I'm not sure it would cover every crash you've seen.  Also, I modified the logging so that you can turn on signaling:5 and get all the OMXH264 logs without zillions of audio logs

Comment 19

[Diego Wilson [:diego]]

Attachment: Logcat with patch in attachment 8474173

Comment 20

[Diego Wilson [:diego]]

I see the same two crashes with the WIP patch in attachment 8474173 [details] [diff] [review]. I attached the log.

Comment 21

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

Attachment: Proxy thread destruction to avoid recursing event loops within OMX H264 shutdown

Diego: try this.....

Comment 22

[Diego Wilson [:diego]]

Comment on attachment 8474924 [details] [diff] [review]
Proxy thread destruction to avoid recursing event loops within OMX H264 shutdown

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

20 calls with no crash with this patch!

Comment 23

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

Comment on attachment 8474924 [details] [diff] [review]
Proxy thread destruction to avoid recursing event loops within OMX H264 shutdown

Note: Clone of the patch we did for GMP plugins to proxy the thread destruction.  The Conduit code didn't expect to recurse the event loop when shutting down a ViE (communications) channel (causing other conduits it interacts with to shut down while we're in mid-shutdown).

Comment 24

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

Attachment: Ignore decode-complete callbacks during OMX decoder shutdown WIP

Cleaned up - boot and suspenders patch to deal with async returns of decode buffers

Comment 25

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

Comment on attachment 8475341 [details] [diff] [review]
Ignore decode-complete callbacks during OMX decoder shutdown WIP

I'll take a review from whomever feels good doing so.  This is a pretty simple "safety" patch to deal with delayed frame decodes after we've started to shut down.

Comment 26

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

https://hg.mozilla.org/integration/mozilla-inbound/rev/4bb05704044d

Comment 27

[Ed Morley [:emorley]]

https://hg.mozilla.org/mozilla-central/rev/4bb05704044d

Comment 28

[Maire Reavy [:mreavy]]

Forgot to mark this leave-open.  We actually need the other patch that's up for review to bsmedberg to fix the crash found by Diego.  Once that lands, we can close this bug.

Comment 29

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

https://hg.mozilla.org/integration/mozilla-inbound/rev/89e3e198baad

Comment 30

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/mozilla-central/rev/89e3e198baad

Comment 31

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/releases/mozilla-b2g32_v2_0/rev/82b04a3ec0f4
https://hg.mozilla.org/releases/mozilla-b2g32_v2_0/rev/7faa4942d230