Issue 187

CHANGELOG.md

@@ -1,6 +1,10 @@
 # Changelog
 
 
+### 3.5.1
+
+* Fix deadlock on `PeerConnection` calls ([187](https://github.com/versatica/libmediasoupclient/issues/187)).
+
 ### 3.5.0
 
 * Update to libwebrtc M140/7339 ([#173](https://github.com/versatica/libmediasoupclient/pull/188)). Credits to @revidee.

src/PeerConnection.cpp

@@ -24,6 +24,32 @@
 
 using json = nlohmann::json;
 
+/**
+ * Waits for a future while pumping the current WebRTC thread's message queue
+ * if we are running on one.
+ *
+ * Without this, calling PeerConnection methods from the signaling thread
+ * deadlocks: future.get() blocks the thread, preventing it from processing
+ * the observer callback task that would resolve the future.
+ */
+template<typename T>
+static T waitForFuture(std::future<T>& future)
+{
+	auto* thread = webrtc::Thread::Current();
+
+	if (thread)
+	{
+		// We are on a WebRTC thread. Pump its message queue so observer
+		// callbacks (posted as tasks) can fire while we wait.
+		while (future.wait_for(std::chrono::milliseconds(0)) != std::future_status::ready)
+		{
+			thread->ProcessMessages(10);
+		}
+	}
+
+	return future.get();
+}
+
 namespace mediasoupclient
 {
 	/* Static. */
@@ -178,7 +204,7 @@ namespace mediasoupclient
 
 		this->pc->CreateOffer(sessionDescriptionObserver, options);
 
-		return future.get();
+		return waitForFuture(future);
 	}
 
 	std::string PeerConnection::CreateAnswer(
@@ -193,7 +219,7 @@ namespace mediasoupclient
 
 		this->pc->CreateAnswer(sessionDescriptionObserver, options);
 
-		return future.get();
+		return waitForFuture(future);
 	}
 
 	void PeerConnection::SetLocalDescription(webrtc::SdpType type, const std::string& sdp)
@@ -215,13 +241,13 @@ namespace mediasoupclient
 			  error.description.c_str());
 
 			observer->Reject(error.description);
-			future.get();
+			waitForFuture(future);
 
 			return;
 		}
 
 		this->pc->SetLocalDescription(std::move(sessionDescription), observer);
-		future.get();
+		waitForFuture(future);
 	}
 
 	void PeerConnection::SetRemoteDescription(webrtc::SdpType type, const std::string& sdp)
@@ -243,13 +269,12 @@ namespace mediasoupclient
 			  error.description.c_str());
 
 			observer->Reject(error.description);
-			future.get();
-
+			waitForFuture(future);
 			return;
 		}
 
 		this->pc->SetRemoteDescription(std::move(sessionDescription), observer);
-		future.get();
+		waitForFuture(future);
 	}
 
 	std::string PeerConnection::GetLocalDescription()
@@ -355,7 +380,7 @@ namespace mediasoupclient
 
 		this->pc->GetStats(callback.get());
 
-		return future.get();
+		return waitForFuture(future);
 	}
 
 	json PeerConnection::GetStats(webrtc::scoped_refptr<webrtc::RtpSenderInterface> selector)
@@ -369,7 +394,7 @@ namespace mediasoupclient
 
 		this->pc->GetStats(std::move(selector), callback);
 
-		return future.get();
+		return waitForFuture(future);
 	}
 
 	json PeerConnection::GetStats(webrtc::scoped_refptr<webrtc::RtpReceiverInterface> selector)
@@ -383,7 +408,7 @@ namespace mediasoupclient
 
 		this->pc->GetStats(std::move(selector), callback);
 
-		return future.get();
+		return waitForFuture(future);
 	}
 
 	webrtc::scoped_refptr<webrtc::DataChannelInterface> PeerConnection::CreateDataChannel(

test/src/Device.test.cpp

@@ -1,8 +1,22 @@
 #include "Device.hpp"
 #include "FakeTransportListener.hpp"
 #include "MediaSoupClientErrors.hpp"
+#include "api/audio_codecs/builtin_audio_decoder_factory.h"
+#include "api/audio_codecs/builtin_audio_encoder_factory.h"
+#include "api/create_peerconnection_factory.h"
+#include "api/video_codecs/video_decoder_factory_template.h"
+#include "api/video_codecs/video_decoder_factory_template_dav1d_adapter.h"
+#include "api/video_codecs/video_decoder_factory_template_libvpx_vp8_adapter.h"
+#include "api/video_codecs/video_decoder_factory_template_libvpx_vp9_adapter.h"
+#include "api/video_codecs/video_decoder_factory_template_open_h264_adapter.h"
+#include "api/video_codecs/video_encoder_factory_template.h"
+#include "api/video_codecs/video_encoder_factory_template_libaom_av1_adapter.h"
+#include "api/video_codecs/video_encoder_factory_template_libvpx_vp8_adapter.h"
+#include "api/video_codecs/video_encoder_factory_template_libvpx_vp9_adapter.h"
+#include "api/video_codecs/video_encoder_factory_template_open_h264_adapter.h"
 #include "fakeParameters.hpp"
 #include "ortc.hpp"
+#include "rtc_base/thread.h"
 #include <catch.hpp>
 
 TEST_CASE("Device", "[Device]")
@@ -105,3 +119,69 @@ TEST_CASE("Device", "[Device]")
 		  TransportRemoteParameters["dtlsParameters"]));
 	}
 }
+
+/**
+ * Regression test: device->Load() must not deadlock when called from the
+ * signaling thread of a custom PeerConnectionFactory.
+ *
+ * Root cause: PeerConnection methods used future.get() which blocked the
+ * calling thread. WebRTC observer callbacks are delivered on the signaling
+ * thread, so calling from that thread deadlocked.
+ *
+ * Fix: waitForSignalingCallback() pumps the current WebRTC thread's message
+ * queue while waiting, allowing observer callbacks to fire.
+ *
+ * https://github.com/versatica/libmediasoupclient/issues/187
+ */
+TEST_CASE("Device::Load from signaling thread does not deadlock", "[Device]")
+{
+	auto networkThread   = webrtc::Thread::CreateWithSocketServer();
+	auto workerThread    = webrtc::Thread::Create();
+	auto signalingThread = webrtc::Thread::Create();
+	networkThread->SetName("deadlock_test_network", nullptr);
+	workerThread->SetName("deadlock_test_worker", nullptr);
+	signalingThread->SetName("deadlock_test_signaling", nullptr);
+	networkThread->Start();
+	workerThread->Start();
+	signalingThread->Start();
+
+	auto factory = webrtc::CreatePeerConnectionFactory(
+	  networkThread.get(),
+	  workerThread.get(),
+	  signalingThread.get(),
+	  nullptr,
+	  webrtc::CreateBuiltinAudioEncoderFactory(),
+	  webrtc::CreateBuiltinAudioDecoderFactory(),
+	  std::make_unique<webrtc::VideoEncoderFactoryTemplate<
+	    webrtc::LibvpxVp8EncoderTemplateAdapter,
+	    webrtc::LibvpxVp9EncoderTemplateAdapter,
+	    webrtc::OpenH264EncoderTemplateAdapter,
+	    webrtc::LibaomAv1EncoderTemplateAdapter>>(),
+	  std::make_unique<webrtc::VideoDecoderFactoryTemplate<
+	    webrtc::LibvpxVp8DecoderTemplateAdapter,
+	    webrtc::LibvpxVp9DecoderTemplateAdapter,
+	    webrtc::OpenH264DecoderTemplateAdapter,
+	    webrtc::Dav1dDecoderTemplateAdapter>>(),
+	  nullptr,
+	  nullptr,
+	  nullptr,
+	  nullptr);
+
+	REQUIRE(factory != nullptr);
+
+	mediasoupclient::PeerConnection::Options pcOptions;
+	pcOptions.factory = factory.get();
+
+	// Call device->Load() FROM the signaling thread - the exact scenario
+	// from the bug report.
+	bool loaded{ false };
+	signalingThread->BlockingCall(
+	  [&]()
+	  {
+		  mediasoupclient::Device device;
+		  REQUIRE_NOTHROW(device.Load(generateRouterRtpCapabilities(), &pcOptions));
+		  loaded = device.IsLoaded();
+	  });
+
+	REQUIRE(loaded);
+}