1670560 - Intermittent deadlock in WebrtcMediaDataDecoder::InitDecode

Status: RESOLVED FIXED

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

Description

[Jean-Yves Avenard [:jya]]

Spawned from https://bugzilla.mozilla.org/show_bug.cgi?id=1631178#c17 as it is likely affecting more tests than this one.

A deadlock could occur when we attempt to start the RDD process if at the same time the JS' main thread is stopping a webrct VideoReceiveStream following a call to PeerConnection::Close()

Starting the RDD process is done via a sync dispatch to the main thread, if the main thread is already waiting for the decoder to shutdown we will deadlock.

The simplest approach would be to unconditionally start the RDD process when we create the WebrtcMediaDataDecoder decoder.
This would prevent the sync dispatch to the main thread as the RDD would already be running.

The 2nd easiest option would be to assume that the MediaDataDecoder initialisation will always succeed, similar to what the GMP decoder is currently doing:
The downside is that we would no longer error if the codec isn't actually supported: not an issue with vp8 and vp9, but for h264 it could be.

But seeing that it's good enough for the GMP initialisation which too could fail it may not be an issue.

Comment 1

[Jean-Yves Avenard [:jya]]

Attachment: Bug 1670560 - P1. Always ensure the RDD process has been started before attempting to create the decoder. r?mattwoodrow

The RDD launch is done by dispatching a synchronous task to the main thread and sending to the parent a LaunchRDD() IPC message.

Unfortunately, the VideoConduit is managed on the main thread and it is possible that it's currently blocked waiting for the decoder thread to shutdown.

Ideally, we would want to move the VideoConduit outside the main thread, but this would be a very expansive change.

Comment 2

[Jean-Yves Avenard [:jya]]

Attachment: Bug 1670560 - Don't use the same thread pool for both decoding and waiting for the decode to complete. r?mattwoodrow

Unlikely to happen, but we could have exhausted all our thread in the decoder thread pool and be deadlock.

Depends on D93201

Comment 3

[Jean-Yves Avenard [:jya]]

I will be taking a different approach than first thought, moving the launch of the RDD over the PBackground channel rather than PContent which much runs on the main thread.

P2 is still worthy of inclusion as it could also potentially lead to a deadlock if we run out of thread in our thread pool.

Comment 4

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/f8fa11cab287
Don't use the same thread pool for both decoding and waiting for the decode to complete. r=mattwoodrow

Comment 5

[Cristina Coroiu [:ccoroiu]]

https://hg.mozilla.org/mozilla-central/rev/f8fa11cab287