Do not delete the turn port entry right away when the respective connection is deleted.
BUG=webrtc:5120
Review URL: https://codereview.webrtc.org/1426673007
Cr-Commit-Position: refs/heads/master@{#10641}
diff --git a/webrtc/p2p/base/turnport.cc b/webrtc/p2p/base/turnport.cc
index 32f63be..5e88365 100644
--- a/webrtc/p2p/base/turnport.cc
+++ b/webrtc/p2p/base/turnport.cc
@@ -140,6 +140,11 @@
const rtc::SocketAddress& address() const { return ext_addr_; }
BindState state() const { return state_; }
+ uint32_t destruction_timestamp() { return destruction_timestamp_; }
+ void set_destruction_timestamp(uint32_t destruction_timestamp) {
+ destruction_timestamp_ = destruction_timestamp;
+ }
+
// Helper methods to send permission and channel bind requests.
void SendCreatePermissionRequest(int delay);
void SendChannelBindRequest(int delay);
@@ -160,6 +165,11 @@
int channel_id_;
rtc::SocketAddress ext_addr_;
BindState state_;
+ // A non-zero value indicates that this entry is scheduled to be destroyed.
+ // It is also used as an ID of the event scheduling. When the destruction
+ // event actually fires, the TurnEntry will be destroyed only if the
+ // timestamp here matches the one in the firing event.
+ uint32_t destruction_timestamp_ = 0;
};
TurnPort::TurnPort(rtc::Thread* thread,
@@ -239,7 +249,7 @@
}
while (!entries_.empty()) {
- DestroyEntry(entries_.front()->address());
+ DestroyEntry(entries_.front());
}
if (resolver_) {
resolver_->Destroy(false);
@@ -438,7 +448,7 @@
}
// Create an entry, if needed, so we can get our permissions set up correctly.
- CreateEntry(address.address());
+ CreateOrRefreshEntry(address.address());
// A TURN port will have two candiates, STUN and TURN. STUN may not
// present in all cases. If present stun candidate will be added first
@@ -713,7 +723,6 @@
}
return;
}
-
Port::OnMessage(message);
}
@@ -898,24 +907,55 @@
return (it != entries_.end()) ? *it : NULL;
}
-TurnEntry* TurnPort::CreateEntry(const rtc::SocketAddress& addr) {
- ASSERT(FindEntry(addr) == NULL);
- TurnEntry* entry = new TurnEntry(this, next_channel_number_++, addr);
- entries_.push_back(entry);
- return entry;
+void TurnPort::CreateOrRefreshEntry(const rtc::SocketAddress& addr) {
+ TurnEntry* entry = FindEntry(addr);
+ if (entry == nullptr) {
+ entry = new TurnEntry(this, next_channel_number_++, addr);
+ entries_.push_back(entry);
+ } else {
+ // The channel binding request for the entry will be refreshed automatically
+ // until the entry is destroyed.
+ CancelEntryDestruction(entry);
+ }
}
-void TurnPort::DestroyEntry(const rtc::SocketAddress& addr) {
- TurnEntry* entry = FindEntry(addr);
+void TurnPort::DestroyEntry(TurnEntry* entry) {
ASSERT(entry != NULL);
entry->SignalDestroyed(entry);
entries_.remove(entry);
delete entry;
}
+void TurnPort::DestroyEntryIfNotCancelled(TurnEntry* entry,
+ uint32_t timestamp) {
+ bool cancelled = timestamp != entry->destruction_timestamp();
+ if (!cancelled) {
+ DestroyEntry(entry);
+ }
+}
+
void TurnPort::OnConnectionDestroyed(Connection* conn) {
- // Destroying TurnEntry for the connection, which is already destroyed.
- DestroyEntry(conn->remote_candidate().address());
+ // Schedule an event to destroy TurnEntry for the connection, which is
+ // already destroyed.
+ const rtc::SocketAddress& remote_address = conn->remote_candidate().address();
+ TurnEntry* entry = FindEntry(remote_address);
+ ASSERT(entry != NULL);
+ ScheduleEntryDestruction(entry);
+}
+
+void TurnPort::ScheduleEntryDestruction(TurnEntry* entry) {
+ ASSERT(entry->destruction_timestamp() == 0);
+ uint32_t timestamp = rtc::Time();
+ entry->set_destruction_timestamp(timestamp);
+ invoker_.AsyncInvokeDelayed<void>(
+ thread(),
+ rtc::Bind(&TurnPort::DestroyEntryIfNotCancelled, this, entry, timestamp),
+ TURN_PERMISSION_TIMEOUT);
+}
+
+void TurnPort::CancelEntryDestruction(TurnEntry* entry) {
+ ASSERT(entry->destruction_timestamp() != 0);
+ entry->set_destruction_timestamp(0);
}
TurnAllocateRequest::TurnAllocateRequest(TurnPort* port)
diff --git a/webrtc/p2p/base/turnport.h b/webrtc/p2p/base/turnport.h
index 3a1b432..db2a3ce 100644
--- a/webrtc/p2p/base/turnport.h
+++ b/webrtc/p2p/base/turnport.h
@@ -16,9 +16,10 @@
#include <set>
#include <string>
+#include "webrtc/base/asyncinvoker.h"
+#include "webrtc/base/asyncpacketsocket.h"
#include "webrtc/p2p/base/port.h"
#include "webrtc/p2p/client/basicportallocator.h"
-#include "webrtc/base/asyncpacketsocket.h"
namespace rtc {
class AsyncResolver;
@@ -122,6 +123,9 @@
return socket_;
}
+ // For testing only.
+ rtc::AsyncInvoker* invoker() { return &invoker_; }
+
// Signal with resolved server address.
// Parameters are port, server address and resolved server address.
// This signal will be sent only if server address is resolved successfully.
@@ -215,8 +219,13 @@
bool HasPermission(const rtc::IPAddress& ipaddr) const;
TurnEntry* FindEntry(const rtc::SocketAddress& address) const;
TurnEntry* FindEntry(int channel_id) const;
- TurnEntry* CreateEntry(const rtc::SocketAddress& address);
- void DestroyEntry(const rtc::SocketAddress& address);
+ void CreateOrRefreshEntry(const rtc::SocketAddress& address);
+ void DestroyEntry(TurnEntry* entry);
+ // Destroys the entry only if |timestamp| matches the destruction timestamp
+ // in |entry|.
+ void DestroyEntryIfNotCancelled(TurnEntry* entry, uint32_t timestamp);
+ void ScheduleEntryDestruction(TurnEntry* entry);
+ void CancelEntryDestruction(TurnEntry* entry);
void OnConnectionDestroyed(Connection* conn);
ProtocolAddress server_address_;
@@ -244,6 +253,8 @@
// The number of retries made due to allocate mismatch error.
size_t allocate_mismatch_retries_;
+ rtc::AsyncInvoker invoker_;
+
friend class TurnEntry;
friend class TurnAllocateRequest;
friend class TurnRefreshRequest;
diff --git a/webrtc/p2p/base/turnport_unittest.cc b/webrtc/p2p/base/turnport_unittest.cc
index 484b152..dcc88bb 100644
--- a/webrtc/p2p/base/turnport_unittest.cc
+++ b/webrtc/p2p/base/turnport_unittest.cc
@@ -385,6 +385,48 @@
EXPECT_TRUE(conn2->receiving());
}
+ void TestDestroyTurnConnection() {
+ turn_port_->PrepareAddress();
+ ASSERT_TRUE_WAIT(turn_ready_, kTimeout);
+ // Create a remote UDP port
+ CreateUdpPort();
+ udp_port_->PrepareAddress();
+ ASSERT_TRUE_WAIT(udp_ready_, kTimeout);
+
+ // Create connections on both ends.
+ Connection* conn1 = udp_port_->CreateConnection(turn_port_->Candidates()[0],
+ Port::ORIGIN_MESSAGE);
+ Connection* conn2 = turn_port_->CreateConnection(udp_port_->Candidates()[0],
+ Port::ORIGIN_MESSAGE);
+ ASSERT_TRUE(conn2 != NULL);
+ ASSERT_TRUE_WAIT(turn_create_permission_success_, kTimeout);
+ // Make sure turn connection can receive.
+ conn1->Ping(0);
+ EXPECT_EQ_WAIT(Connection::STATE_WRITABLE, conn1->write_state(), kTimeout);
+ EXPECT_FALSE(turn_unknown_address_);
+
+ // Destroy the connection on the turn port. The TurnEntry is still
+ // there. So the turn port gets ping from unknown address if it is pinged.
+ conn2->Destroy();
+ conn1->Ping(0);
+ EXPECT_TRUE_WAIT(turn_unknown_address_, kTimeout);
+
+ // Flush all requests in the invoker to destroy the TurnEntry.
+ // Now the turn port cannot receive the ping.
+ turn_unknown_address_ = false;
+ turn_port_->invoker()->Flush(rtc::Thread::Current());
+ conn1->Ping(0);
+ rtc::Thread::Current()->ProcessMessages(500);
+ EXPECT_FALSE(turn_unknown_address_);
+
+ // If the connection is created again, it will start to receive pings.
+ conn2 = turn_port_->CreateConnection(udp_port_->Candidates()[0],
+ Port::ORIGIN_MESSAGE);
+ conn1->Ping(0);
+ EXPECT_TRUE_WAIT(conn2->receiving(), kTimeout);
+ EXPECT_FALSE(turn_unknown_address_);
+ }
+
void TestTurnSendData() {
turn_port_->PrepareAddress();
EXPECT_TRUE_WAIT(turn_ready_, kTimeout);
@@ -694,6 +736,20 @@
TestTurnConnection();
}
+// Test that if a connection on a TURN port is destroyed, the TURN port can
+// still receive ping on that connection as if it is from an unknown address.
+// If the connection is created again, it will be used to receive ping.
+TEST_F(TurnPortTest, TestDestroyTurnConnection) {
+ CreateTurnPort(kTurnUsername, kTurnPassword, kTurnUdpProtoAddr);
+ TestDestroyTurnConnection();
+}
+
+// Similar to above, except that this test will use the shared socket.
+TEST_F(TurnPortTest, TestDestroyTurnConnectionUsingSharedSocket) {
+ CreateSharedTurnPort(kTurnUsername, kTurnPassword, kTurnUdpProtoAddr);
+ TestDestroyTurnConnection();
+}
+
// Test that we fail to create a connection when we want to use TLS over TCP.
// This test should be removed once we have TLS support.
TEST_F(TurnPortTest, TestTurnTlsTcpConnectionFails) {
