diff --git a/pc/channel_manager.cc b/pc/channel_manager.cc index 2671c10411..d16e3712dc 100644 --- a/pc/channel_manager.cc +++ b/pc/channel_manager.cc @@ -41,6 +41,7 @@ ChannelManager::ChannelManager( } ChannelManager::~ChannelManager() { + RTC_DCHECK_RUN_ON(main_thread_); if (initialized_) { Terminate(); } @@ -50,6 +51,7 @@ ChannelManager::~ChannelManager() { } bool ChannelManager::SetVideoRtxEnabled(bool enable) { + RTC_DCHECK_RUN_ON(main_thread_); // To be safe, this call is only allowed before initialization. Apps like // Flute only have a singleton ChannelManager and we don't want this flag to // be toggled between calls or when there's concurrent calls. We expect apps @@ -119,7 +121,13 @@ void ChannelManager::GetSupportedDataCodecs( *codecs = data_engine_->data_codecs(); } +bool ChannelManager::initialized() const { + RTC_DCHECK_RUN_ON(main_thread_); + return initialized_; +} + bool ChannelManager::Init() { + RTC_DCHECK_RUN_ON(main_thread_); RTC_DCHECK(!initialized_); if (initialized_) { return false; @@ -171,6 +179,7 @@ ChannelManager::GetSupportedVideoRtpHeaderExtensions() const { } void ChannelManager::Terminate() { + RTC_DCHECK_RUN_ON(main_thread_); RTC_DCHECK(initialized_); if (!initialized_) { return; @@ -206,7 +215,6 @@ VoiceChannel* ChannelManager::CreateVoiceChannel( } RTC_DCHECK_RUN_ON(worker_thread_); - RTC_DCHECK(initialized_); RTC_DCHECK(call); if (!media_engine_) { return nullptr; @@ -241,8 +249,6 @@ void ChannelManager::DestroyVoiceChannel(VoiceChannel* voice_channel) { return; } - RTC_DCHECK(initialized_); - auto it = absl::c_find_if(voice_channels_, [&](const std::unique_ptr& p) { return p.get() == voice_channel; @@ -279,7 +285,6 @@ VideoChannel* ChannelManager::CreateVideoChannel( } RTC_DCHECK_RUN_ON(worker_thread_); - RTC_DCHECK(initialized_); RTC_DCHECK(call); if (!media_engine_) { return nullptr; @@ -315,8 +320,6 @@ void ChannelManager::DestroyVideoChannel(VideoChannel* video_channel) { return; } - RTC_DCHECK(initialized_); - auto it = absl::c_find_if(video_channels_, [&](const std::unique_ptr& p) { return p.get() == video_channel; @@ -346,7 +349,6 @@ RtpDataChannel* ChannelManager::CreateRtpDataChannel( } // This is ok to alloc from a thread other than the worker thread. - RTC_DCHECK(initialized_); DataMediaChannel* media_channel = data_engine_->CreateChannel(media_config); if (!media_channel) { RTC_LOG(LS_WARNING) << "Failed to create RTP data channel."; @@ -377,8 +379,6 @@ void ChannelManager::DestroyRtpDataChannel(RtpDataChannel* data_channel) { return; } - RTC_DCHECK(initialized_); - auto it = absl::c_find_if(data_channels_, [&](const std::unique_ptr& p) { return p.get() == data_channel; diff --git a/pc/channel_manager.h b/pc/channel_manager.h index dc5a113583..f2a11bcee1 100644 --- a/pc/channel_manager.h +++ b/pc/channel_manager.h @@ -52,25 +52,8 @@ class ChannelManager final { rtc::Thread* network_thread); ~ChannelManager(); - // Accessors for the worker thread, allowing it to be set after construction, - // but before Init. set_worker_thread will return false if called after Init. rtc::Thread* worker_thread() const { return worker_thread_; } - bool set_worker_thread(rtc::Thread* thread) { - if (initialized_) { - return false; - } - worker_thread_ = thread; - return true; - } rtc::Thread* network_thread() const { return network_thread_; } - bool set_network_thread(rtc::Thread* thread) { - if (initialized_) { - return false; - } - network_thread_ = thread; - return true; - } - MediaEngineInterface* media_engine() { return media_engine_.get(); } // Retrieves the list of supported audio & video codec types. @@ -88,7 +71,7 @@ class ChannelManager final { GetSupportedVideoRtpHeaderExtensions() const; // Indicates whether the media engine is started. - bool initialized() const { return initialized_; } + bool initialized() const; // Starts up the media engine. bool Init(); // Shuts down the media engine. @@ -165,10 +148,10 @@ class ChannelManager final { private: std::unique_ptr media_engine_; // Nullable. std::unique_ptr data_engine_; // Non-null. - bool initialized_ = false; - rtc::Thread* main_thread_; - rtc::Thread* worker_thread_; - rtc::Thread* network_thread_; + bool initialized_ RTC_GUARDED_BY(main_thread_) = false; + rtc::Thread* const main_thread_; + rtc::Thread* const worker_thread_; + rtc::Thread* const network_thread_; // Vector contents are non-null. std::vector> voice_channels_; diff --git a/pc/channel_manager_unittest.cc b/pc/channel_manager_unittest.cc index c0dddd89cf..947acbd9fe 100644 --- a/pc/channel_manager_unittest.cc +++ b/pc/channel_manager_unittest.cc @@ -55,33 +55,39 @@ class ChannelManagerTest : public ::testing::Test { cm_(new cricket::ChannelManager( std::unique_ptr(fme_), std::unique_ptr(fdme_), - rtc::Thread::Current(), - rtc::Thread::Current())), + worker_.get(), + network_.get())), fake_call_() { fme_->SetAudioCodecs(MAKE_VECTOR(kAudioCodecs)); fme_->SetVideoCodecs(MAKE_VECTOR(kVideoCodecs)); + network_->SetName("Network", this); + worker_->SetName("Worker", this); + network_->Start(); + worker_->Start(); } void TestCreateDestroyChannels(webrtc::RtpTransportInternal* rtp_transport) { - cricket::VoiceChannel* voice_channel = cm_->CreateVoiceChannel( - &fake_call_, cricket::MediaConfig(), rtp_transport, - rtc::Thread::Current(), cricket::CN_AUDIO, kDefaultSrtpRequired, - webrtc::CryptoOptions(), &ssrc_generator_, AudioOptions()); - EXPECT_TRUE(voice_channel != nullptr); - cricket::VideoChannel* video_channel = cm_->CreateVideoChannel( - &fake_call_, cricket::MediaConfig(), rtp_transport, - rtc::Thread::Current(), cricket::CN_VIDEO, kDefaultSrtpRequired, - webrtc::CryptoOptions(), &ssrc_generator_, VideoOptions(), - video_bitrate_allocator_factory_.get()); - EXPECT_TRUE(video_channel != nullptr); - cricket::RtpDataChannel* rtp_data_channel = cm_->CreateRtpDataChannel( - cricket::MediaConfig(), rtp_transport, rtc::Thread::Current(), - cricket::CN_DATA, kDefaultSrtpRequired, webrtc::CryptoOptions(), - &ssrc_generator_); - EXPECT_TRUE(rtp_data_channel != nullptr); - cm_->DestroyVideoChannel(video_channel); - cm_->DestroyVoiceChannel(voice_channel); - cm_->DestroyRtpDataChannel(rtp_data_channel); + worker_->Invoke(RTC_FROM_HERE, [this, rtp_transport]() { + cricket::VoiceChannel* voice_channel = cm_->CreateVoiceChannel( + &fake_call_, cricket::MediaConfig(), rtp_transport, + rtc::Thread::Current(), cricket::CN_AUDIO, kDefaultSrtpRequired, + webrtc::CryptoOptions(), &ssrc_generator_, AudioOptions()); + EXPECT_TRUE(voice_channel != nullptr); + cricket::VideoChannel* video_channel = cm_->CreateVideoChannel( + &fake_call_, cricket::MediaConfig(), rtp_transport, + rtc::Thread::Current(), cricket::CN_VIDEO, kDefaultSrtpRequired, + webrtc::CryptoOptions(), &ssrc_generator_, VideoOptions(), + video_bitrate_allocator_factory_.get()); + EXPECT_TRUE(video_channel != nullptr); + cricket::RtpDataChannel* rtp_data_channel = cm_->CreateRtpDataChannel( + cricket::MediaConfig(), rtp_transport, rtc::Thread::Current(), + cricket::CN_DATA, kDefaultSrtpRequired, webrtc::CryptoOptions(), + &ssrc_generator_); + EXPECT_TRUE(rtp_data_channel != nullptr); + cm_->DestroyVideoChannel(video_channel); + cm_->DestroyVoiceChannel(voice_channel); + cm_->DestroyRtpDataChannel(rtp_data_channel); + }); cm_->Terminate(); } @@ -100,7 +106,6 @@ class ChannelManagerTest : public ::testing::Test { // Test that we startup/shutdown properly. TEST_F(ChannelManagerTest, StartupShutdown) { EXPECT_FALSE(cm_->initialized()); - EXPECT_EQ(rtc::Thread::Current(), cm_->worker_thread()); EXPECT_TRUE(cm_->Init()); EXPECT_TRUE(cm_->initialized()); cm_->Terminate(); @@ -109,19 +114,11 @@ TEST_F(ChannelManagerTest, StartupShutdown) { // Test that we startup/shutdown properly with a worker thread. TEST_F(ChannelManagerTest, StartupShutdownOnThread) { - network_->Start(); - worker_->Start(); EXPECT_FALSE(cm_->initialized()); - EXPECT_EQ(rtc::Thread::Current(), cm_->worker_thread()); - EXPECT_TRUE(cm_->set_network_thread(network_.get())); EXPECT_EQ(network_.get(), cm_->network_thread()); - EXPECT_TRUE(cm_->set_worker_thread(worker_.get())); EXPECT_EQ(worker_.get(), cm_->worker_thread()); EXPECT_TRUE(cm_->Init()); EXPECT_TRUE(cm_->initialized()); - // Setting the network or worker thread while initialized should fail. - EXPECT_FALSE(cm_->set_network_thread(rtc::Thread::Current())); - EXPECT_FALSE(cm_->set_worker_thread(rtc::Thread::Current())); cm_->Terminate(); EXPECT_FALSE(cm_->initialized()); } @@ -166,21 +163,6 @@ TEST_F(ChannelManagerTest, SetVideoRtxEnabled) { } TEST_F(ChannelManagerTest, CreateDestroyChannels) { - EXPECT_TRUE(cm_->Init()); - auto rtp_dtls_transport = std::make_unique( - "fake_dtls_transport", cricket::ICE_CANDIDATE_COMPONENT_RTP); - auto dtls_srtp_transport = std::make_unique( - /*rtcp_mux_required=*/true); - dtls_srtp_transport->SetDtlsTransports(rtp_dtls_transport.get(), - /*rtcp_dtls_transport=*/nullptr); - TestCreateDestroyChannels(dtls_srtp_transport.get()); -} - -TEST_F(ChannelManagerTest, CreateDestroyChannelsOnThread) { - network_->Start(); - worker_->Start(); - EXPECT_TRUE(cm_->set_worker_thread(worker_.get())); - EXPECT_TRUE(cm_->set_network_thread(network_.get())); EXPECT_TRUE(cm_->Init()); auto rtp_dtls_transport = std::make_unique( "fake_dtls_transport", cricket::ICE_CANDIDATE_COMPONENT_RTP,