Threading and Deadlock of PeerConnection calls

[revidee]

I'm encountering a Deadlock when using WebRTC in the 'expected' way alongside libmediasoupclient:

• Use a Device with custom PeerConnection::Options with a custom PeerConnectionFactory (with already provided signaling/networking/worker threads)
• Call device_->Load(...) on the signaling_thread() of the PeerConnectionFactory
  • 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 via future.get() 2 lines down
• Thus, the SdpOfferAnswerHandler in 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::CreateOffer is 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::PeerConnection methods that internally call any this->pc methods

[ibc]

Thanks. We will handle this when we come back to work on this and also update libwebrtc version.

[revidee]

Good to hear. Regarding the libwebrtc version bump, you might want to look at #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.