diff --git a/webrtc/pc/channel.cc b/webrtc/pc/channel.cc index 7e4d5d2dac..67c16ec64a 100644 --- a/webrtc/pc/channel.cc +++ b/webrtc/pc/channel.cc @@ -183,8 +183,6 @@ BaseChannel::~BaseChannel() { ASSERT(worker_thread_ == rtc::Thread::Current()); Deinit(); StopConnectionMonitor(); - // Send any outstanding RTCP packets. - network_thread_->Invoke(Bind(&BaseChannel::FlushRtcpMessages_n, this)); // Eats any outstanding messages or packets. worker_thread_->Clear(&invoker_); worker_thread_->Clear(this); @@ -194,21 +192,40 @@ BaseChannel::~BaseChannel() { delete media_channel_; // Note that we don't just call SetTransportChannel_n(nullptr) because that // would call a pure virtual method which we can't do from a destructor. - network_thread_->Invoke(Bind(&BaseChannel::DeinitNetwork_n, this)); + network_thread_->Invoke( + Bind(&BaseChannel::DestroyTransportChannels_n, this)); LOG(LS_INFO) << "Destroyed channel"; } -void BaseChannel::DeinitNetwork_n() { +void BaseChannel::DisconnectTransportChannels_n() { + // Send any outstanding RTCP packets. + FlushRtcpMessages_n(); + + // Stop signals from transport channels, but keep them alive because + // media_channel may use them from a different thread. if (transport_channel_) { DisconnectFromTransportChannel(transport_channel_); + } + if (rtcp_transport_channel_) { + DisconnectFromTransportChannel(rtcp_transport_channel_); + } + + // Clear pending read packets/messages. + network_thread_->Clear(&invoker_); + network_thread_->Clear(this); +} + +void BaseChannel::DestroyTransportChannels_n() { + if (transport_channel_) { transport_controller_->DestroyTransportChannel_n( transport_name_, cricket::ICE_CANDIDATE_COMPONENT_RTP); } if (rtcp_transport_channel_) { - DisconnectFromTransportChannel(rtcp_transport_channel_); transport_controller_->DestroyTransportChannel_n( transport_name_, cricket::ICE_CANDIDATE_COMPONENT_RTCP); } + // Clear pending send packets/messages. + network_thread_->Clear(&invoker_); network_thread_->Clear(this); } @@ -243,6 +260,11 @@ bool BaseChannel::InitNetwork_n() { void BaseChannel::Deinit() { RTC_DCHECK(worker_thread_->IsCurrent()); media_channel_->SetInterface(NULL); + // Packets arrive on the network thread, processing packets calls virtual + // functions, so need to stop this process in Deinit that is called in + // derived classes destructor. + network_thread_->Invoke( + Bind(&BaseChannel::DisconnectTransportChannels_n, this)); } bool BaseChannel::SetTransport(const std::string& transport_name) { diff --git a/webrtc/pc/channel.h b/webrtc/pc/channel.h index df75245f27..2f66f12c13 100644 --- a/webrtc/pc/channel.h +++ b/webrtc/pc/channel.h @@ -314,7 +314,8 @@ class BaseChannel private: bool InitNetwork_n(); - void DeinitNetwork_n(); + void DisconnectTransportChannels_n(); + void DestroyTransportChannels_n(); void SignalSentPacket_n(TransportChannel* channel, const rtc::SentPacket& sent_packet); void SignalSentPacket_w(const rtc::SentPacket& sent_packet); diff --git a/webrtc/pc/channel_unittest.cc b/webrtc/pc/channel_unittest.cc index d15748ffbb..81339cfeda 100644 --- a/webrtc/pc/channel_unittest.cc +++ b/webrtc/pc/channel_unittest.cc @@ -1000,6 +1000,19 @@ class ChannelTest : public testing::Test, public sigslot::has_slots<> { EXPECT_TRUE(CheckNoRtp2()); } + void TestDeinit() { + CreateChannels(RTCP, RTCP); + EXPECT_TRUE(SendInitiate()); + EXPECT_TRUE(SendAccept()); + SendRtp1(); + SendRtp2(); + SendRtcp1(); + SendRtcp2(); + // Do not wait, destroy channels. + channel1_.reset(nullptr); + channel2_.reset(nullptr); + } + // Check that RTCP is not transmitted if both sides don't support RTCP. void SendNoRtcpToNoRtcp() { CreateChannels(0, 0); @@ -2076,6 +2089,10 @@ TEST_F(VoiceChannelSingleThreadTest, TestInit) { EXPECT_TRUE(media_channel1_->dtmf_info_queue().empty()); } +TEST_F(VoiceChannelSingleThreadTest, TestDeinit) { + Base::TestDeinit(); +} + TEST_F(VoiceChannelSingleThreadTest, TestSetContents) { Base::TestSetContents(); } @@ -2402,6 +2419,10 @@ TEST_F(VoiceChannelDoubleThreadTest, TestInit) { EXPECT_TRUE(media_channel1_->dtmf_info_queue().empty()); } +TEST_F(VoiceChannelDoubleThreadTest, TestDeinit) { + Base::TestDeinit(); +} + TEST_F(VoiceChannelDoubleThreadTest, TestSetContents) { Base::TestSetContents(); } @@ -2726,6 +2747,10 @@ TEST_F(VideoChannelSingleThreadTest, TestInit) { Base::TestInit(); } +TEST_F(VideoChannelSingleThreadTest, TestDeinit) { + Base::TestDeinit(); +} + TEST_F(VideoChannelSingleThreadTest, TestSetContents) { Base::TestSetContents(); } @@ -2966,6 +2991,10 @@ TEST_F(VideoChannelDoubleThreadTest, TestInit) { Base::TestInit(); } +TEST_F(VideoChannelDoubleThreadTest, TestDeinit) { + Base::TestDeinit(); +} + TEST_F(VideoChannelDoubleThreadTest, TestSetContents) { Base::TestSetContents(); } @@ -3277,6 +3306,10 @@ TEST_F(DataChannelSingleThreadTest, TestInit) { EXPECT_FALSE(media_channel1_->IsStreamMuted(0)); } +TEST_F(DataChannelSingleThreadTest, TestDeinit) { + Base::TestDeinit(); +} + TEST_F(DataChannelSingleThreadTest, TestSetContents) { Base::TestSetContents(); } @@ -3417,6 +3450,10 @@ TEST_F(DataChannelDoubleThreadTest, TestInit) { EXPECT_FALSE(media_channel1_->IsStreamMuted(0)); } +TEST_F(DataChannelDoubleThreadTest, TestDeinit) { + Base::TestDeinit(); +} + TEST_F(DataChannelDoubleThreadTest, TestSetContents) { Base::TestSetContents(); }