From 9dfed79f3fb307e4bc3bd5a2da059de7940ec081 Mon Sep 17 00:00:00 2001 From: honghaiz Date: Fri, 29 Jan 2016 13:22:31 -0800 Subject: [PATCH] Stop processing any incoming packets when turn port is disconnected. If it still handle packets, when a ping arrives, it will pass the packet to p2ptransportchannel, eventually causing an ASSERT error there (when p2ptransportchannel tries to create a connection from the ping request from unknown address). BUG= Review URL: https://codereview.webrtc.org/1649493006 Cr-Commit-Position: refs/heads/master@{#11430} --- webrtc/p2p/base/turnport.cc | 6 ++++++ webrtc/p2p/base/turnport.h | 5 +++-- webrtc/p2p/base/turnport_unittest.cc | 25 +++++++++++++++++++++++++ 3 files changed, 34 insertions(+), 2 deletions(-) diff --git a/webrtc/p2p/base/turnport.cc b/webrtc/p2p/base/turnport.cc index 5ed93dd1d8..4f5fef1d53 100644 --- a/webrtc/p2p/base/turnport.cc +++ b/webrtc/p2p/base/turnport.cc @@ -555,6 +555,12 @@ void TurnPort::OnReadPacket( return; } + if (state_ == STATE_DISCONNECTED) { + LOG_J(LS_WARNING, this) + << "Received TURN message while the Turn port is disconnected"; + return; + } + // Check the message type, to see if is a Channel Data message. // The message will either be channel data, a TURN data indication, or // a response to a previous request. diff --git a/webrtc/p2p/base/turnport.h b/webrtc/p2p/base/turnport.h index 4d83806a37..9faf0644b0 100644 --- a/webrtc/p2p/base/turnport.h +++ b/webrtc/p2p/base/turnport.h @@ -151,6 +151,9 @@ class TurnPort : public Port { // Finds the turn entry with |address| and sets its channel id. // Returns true if the entry is found. bool SetEntryChannelId(const rtc::SocketAddress& address, int channel_id); + // Visible for testing. + // Shuts down the turn port, usually because of some fatal errors. + void Close(); protected: TurnPort(rtc::Thread* thread, @@ -201,8 +204,6 @@ class TurnPort : public Port { } } - // Shuts down the turn port, usually because of some fatal errors. - void Close(); void OnTurnRefreshError(); bool SetAlternateServer(const rtc::SocketAddress& address); void ResolveTurnAddress(const rtc::SocketAddress& address); diff --git a/webrtc/p2p/base/turnport_unittest.cc b/webrtc/p2p/base/turnport_unittest.cc index 916162575f..a503c2baa1 100644 --- a/webrtc/p2p/base/turnport_unittest.cc +++ b/webrtc/p2p/base/turnport_unittest.cc @@ -713,6 +713,31 @@ TEST_F(TurnPortTest, TestRefreshRequestGetsErrorResponse) { EXPECT_FALSE(turn_port_->HasRequests()); } +// Test that TurnPort will not handle any incoming packets once it has been +// closed. +TEST_F(TurnPortTest, TestStopProcessingPacketsAfterClosed) { + CreateTurnPort(kTurnUsername, kTurnPassword, kTurnUdpProtoAddr); + PrepareTurnAndUdpPorts(); + Connection* conn1 = turn_port_->CreateConnection(udp_port_->Candidates()[0], + Port::ORIGIN_MESSAGE); + Connection* conn2 = udp_port_->CreateConnection(turn_port_->Candidates()[0], + Port::ORIGIN_MESSAGE); + ASSERT_TRUE(conn1 != NULL); + ASSERT_TRUE(conn2 != NULL); + // Make sure conn2 is writable. + conn2->Ping(0); + EXPECT_EQ_WAIT(Connection::STATE_WRITABLE, conn2->write_state(), kTimeout); + + turn_port_->Close(); + rtc::Thread::Current()->ProcessMessages(0); + turn_unknown_address_ = false; + conn2->Ping(0); + rtc::Thread::Current()->ProcessMessages(500); + // Since the turn port does not handle packets any more, it should not + // SignalUnknownAddress. + EXPECT_FALSE(turn_unknown_address_); +} + // Test that CreateConnection will return null if port becomes disconnected. TEST_F(TurnPortTest, TestCreateConnectionWhenSocketClosed) { turn_server_.AddInternalSocket(kTurnTcpIntAddr, cricket::PROTO_TCP);