Threading and Deadlock of PeerConnection calls
I'm encountering a Deadlock when using WebRTC in the 'expected' way alongside libmediasoupclient:
- Use a Device with custom
PeerConnection::Optionswith a customPeerConnectionFactory(with already provided signaling/networking/worker threads) - Call
device_->Load(...)on thesignaling_thread()of thePeerConnectionFactory- This is the 'expected' thread to call most (nearly all) functions of the webrtc::PeerConnectionInterface (i.e. compare pc->CreateOffer impl. https://webrtc.googlesource.com/src/+/refs/heads/lkgr/pc/peer_connection.cc#1489 it expects the signaling thread to be the current thread)
- This causes a deadlock, since we correctly call
pc->CreateOffer()(https://github.com/versatica/libmediasoupclient/blob/v3/src/PeerConnection.cpp#L178) on the signaling thread, but then deadlock it by waiting on the future viafuture.get()2 lines down - Thus, the
SdpOfferAnswerHandlerin webrtc (https://webrtc.googlesource.com/src/+/refs/heads/lkgr/pc/sdp_offer_answer.cc) can never invoke the observer on the signaling thread to resolve the future
When using the default built-in PeerConnectionFactory this doesn't cause a deadlock, however, in that case the 'expected' correct thread from webrtc (CreateOffer expects to be called on the signaling thread) is not used, thus potentially being unsafe inside webrtc.
- Right now, one cannot satisfy the Thread-Demands from WebRTC while being deadlock free in Mediasoup. We can be deadlock free, by using another thread for all MediaSoup functions, but that violates the thread checks from webrtc. Or we use the correct threads and face the Deadlock.
- IMHO, the threading design should be reconsidered, to ensure that the expected threads are used for all the PeerConnection functions
- The use of blocking futures should be carefully reconsidered to ensure such deadlock situations don't occur, i.e. by ensuring
mediasoup::PeerConnection::CreateOfferis not called on the signaling thread, posting a task to the actual signaling thread (which has to be saved somewhere) and then waiting on the calling thread for the signaling thread to invoke the observer and thus resolving the future - This affects all
mediasoup::PeerConnectionmethods that internally call anythis->pcmethods
Thanks. We will handle this when we come back to work on this and also update libwebrtc version.
Good to hear. Regarding the libwebrtc version bump, you might want to look at https://github.com/versatica/libmediasoupclient/pull/188 - The version bump was rather straight forward, however the same issues as in the JS lib regarding static extension/payload-type ids emerged. To fix, I closely mirrored the changes from the JS lib and included it in the pull request as the version bump would be rather useless without that.