admin/webrtc_m130
1
0
Fork 0
Code Issues Pull Requests Actions Packages Projects Releases Wiki Activity

Delete a connection only if it has timed out on writing and not receiving for 10 seconds.

Browse Source 
BUG=webrtc:5034,webrtc:4937
R=pthatcher@webrtc.org

Review URL: https://codereview.webrtc.org/1371623003 .

Cr-Commit-Position: refs/heads/master@{#10119}
This commit is contained in:
Honghai Zhang 2015-09-30 09:51:58 -07:00
parent 27551c9574
commit 2b342bf99c
5 changed files with 170 additions and 57 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

46
webrtc/p2p/base/p2ptransportchannel.cc
View File

@ -286,23 +286,26 @@ void P2PTransportChannel::SetIceTiebreaker(uint64 tiebreaker) {
tiebreaker_ = tiebreaker;
}
// Currently a channel is considered ICE completed once there is no
// more than one connection per Network. This works for a single NIC
// with both IPv4 and IPv6 enabled. However, this condition won't
// happen when there are multiple NICs and all of them have
// connectivity.
// TODO(guoweis): Change Completion to be driven by a channel level
// timer.
// A channel is considered ICE completed once there is at most one active
// connection per network and at least one active connection.
TransportChannelState P2PTransportChannel::GetState() const {
std::set<rtc::Network*> networks;
if (connections_.empty()) {
return had_connection_ ? TransportChannelState::STATE_FAILED
: TransportChannelState::STATE_INIT;
if (!had_connection_) {
return TransportChannelState::STATE_INIT;
}
for (uint32 i = 0; i < connections_.size(); ++i) {
rtc::Network* network = connections_[i]->port()->Network();
std::vector<Connection*> active_connections;
for (Connection* connection : connections_) {
if (connection->active()) {
active_connections.push_back(connection);
}
}
if (active_connections.empty()) {
return TransportChannelState::STATE_FAILED;
}
std::set<rtc::Network*> networks;
for (Connection* connection : active_connections) {
rtc::Network* network = connection->port()->Network();
if (networks.find(network) == networks.end()) {
networks.insert(network);
} else {
@ -312,8 +315,8 @@ TransportChannelState P2PTransportChannel::GetState() const {
return TransportChannelState::STATE_CONNECTING;
}
}
LOG_J(LS_VERBOSE, this) << "Ice is completed for this channel.";
LOG_J(LS_VERBOSE, this) << "Ice is completed for this channel.";
return TransportChannelState::STATE_COMPLETED;
}
@ -872,8 +875,7 @@ bool P2PTransportChannel::GetStats(ConnectionInfos *infos) {
infos->clear();
std::vector<Connection *>::const_iterator it;
for (it = connections_.begin(); it != connections_.end(); ++it) {
Connection* connection = *it;
for (Connection* connection : connections_) {
ConnectionInfo info;
info.best_connection = (best_connection_ == connection);
info.receiving = connection->receiving();
@ -1016,7 +1018,7 @@ void P2PTransportChannel::PruneConnections() {
Connection* premier = GetBestConnectionOnNetwork(network);
// Do not prune connections if the current best connection is weak on this
// network. Otherwise, it may delete connections prematurely.
if (!premier || premier->Weak()) {
if (!premier || premier->weak()) {
continue;
}
@ -1105,8 +1107,8 @@ void P2PTransportChannel::HandleAllTimedOut() {
HandleNotWritable();
}
bool P2PTransportChannel::Weak() const {
return !best_connection_ || best_connection_->Weak();
bool P2PTransportChannel::weak() const {
return !best_connection_ || best_connection_->weak();
}
// If we have a best connection, return it, otherwise return top one in the
@ -1154,7 +1156,7 @@ void P2PTransportChannel::OnCheckAndPing() {
UpdateConnectionStates();
// When the best connection is either not receiving or not writable,
// switch to weak ping delay.
int ping_delay = Weak() ? WEAK_PING_DELAY : STRONG_PING_DELAY;
int ping_delay = weak() ? WEAK_PING_DELAY : STRONG_PING_DELAY;
if (rtc::Time() >= last_ping_sent_ms_ + ping_delay) {
Connection* conn = FindNextPingableConnection();
if (conn) {
@ -1186,7 +1188,7 @@ bool P2PTransportChannel::IsPingable(Connection* conn) {
// If the channel is weak, ping all candidates. Otherwise, we only
// want to ping connections that have not timed out on writing.
return Weak() || conn->write_state() != Connection::STATE_WRITE_TIMEOUT;
return weak() || conn->write_state() != Connection::STATE_WRITE_TIMEOUT;
}
// Returns the next pingable connection to ping. This will be the oldest

2
webrtc/p2p/base/p2ptransportchannel.h
View File

@ -164,7 +164,7 @@ class P2PTransportChannel : public TransportChannelImpl,
// A transport channel is weak if the current best connection is either
// not receiving or not writable, or if there is no best connection at all.
bool Weak() const;
bool weak() const;
void UpdateConnectionStates();
void RequestSort();
void SortConnections();

119
webrtc/p2p/base/p2ptransportchannel_unittest.cc
View File

@ -1112,17 +1112,24 @@ TEST_F(P2PTransportChannelTest, GetStats) {
TestSendRecv(1);
cricket::ConnectionInfos infos;
ASSERT_TRUE(ep1_ch1()->GetStats(&infos));
ASSERT_EQ(1U, infos.size());
EXPECT_TRUE(infos[0].new_connection);
EXPECT_TRUE(infos[0].best_connection);
EXPECT_TRUE(infos[0].receiving);
EXPECT_TRUE(infos[0].writable);
EXPECT_FALSE(infos[0].timeout);
EXPECT_EQ(10U, infos[0].sent_total_packets);
EXPECT_EQ(0U, infos[0].sent_discarded_packets);
EXPECT_EQ(10 * 36U, infos[0].sent_total_bytes);
EXPECT_EQ(10 * 36U, infos[0].recv_total_bytes);
EXPECT_GT(infos[0].rtt, 0U);
ASSERT_TRUE(infos.size() >= 1);
cricket::ConnectionInfo* best_conn_info = nullptr;
for (cricket::ConnectionInfo& info : infos) {
if (info.best_connection) {
best_conn_info = &info;
break;
}
}
ASSERT_TRUE(best_conn_info != nullptr);
EXPECT_TRUE(best_conn_info->new_connection);
EXPECT_TRUE(best_conn_info->receiving);
EXPECT_TRUE(best_conn_info->writable);
EXPECT_FALSE(best_conn_info->timeout);
EXPECT_EQ(10U, best_conn_info->sent_total_packets);
EXPECT_EQ(0U, best_conn_info->sent_discarded_packets);
EXPECT_EQ(10 * 36U, best_conn_info->sent_total_bytes);
EXPECT_EQ(10 * 36U, best_conn_info->recv_total_bytes);
EXPECT_GT(best_conn_info->rtt, 0U);
DestroyChannels();
}
@ -1620,6 +1627,20 @@ TEST_F(P2PTransportChannelMultihomedTest, TestFailoverControllingSide) {
DestroyChannels();
}
TEST_F(P2PTransportChannelMultihomedTest, TestGetState) {
AddAddress(0, kAlternateAddrs[0]);
AddAddress(0, kPublicAddrs[0]);
AddAddress(1, kPublicAddrs[1]);
// Create channels and let them go writable, as usual.
CreateChannels(1);
// Both transport channels will reach STATE_COMPLETED quickly.
EXPECT_EQ_WAIT(cricket::TransportChannelState::STATE_COMPLETED,
ep1_ch1()->GetState(), 1000);
EXPECT_EQ_WAIT(cricket::TransportChannelState::STATE_COMPLETED,
ep2_ch1()->GetState(), 1000);
}
/*
TODO(pthatcher): Once have a way to handle network interfaces changes
@ -1796,9 +1817,11 @@ TEST_F(P2PTransportChannelPingTest, ConnectionResurrection) {
conn2->ReceivedPing();
conn2->ReceivedPingResponse();
// Wait for conn1 to be destroyed.
EXPECT_TRUE_WAIT(GetConnectionTo(&ch, "1.1.1.1", 1) == nullptr, 3000);
cricket::Port* port = GetPort(&ch);
// Wait for conn1 to be pruned.
EXPECT_TRUE_WAIT(conn1->pruned(), 3000);
// Destroy the connection to test SignalUnknownAddress.
conn1->Destroy();
EXPECT_TRUE_WAIT(GetConnectionTo(&ch, "1.1.1.1", 1) == nullptr, 1000);
// Create a minimal STUN message with prflx priority.
cricket::IceMessage request;
@ -1810,6 +1833,7 @@ TEST_F(P2PTransportChannelPingTest, ConnectionResurrection) {
cricket::STUN_ATTR_PRIORITY, prflx_priority));
EXPECT_NE(prflx_priority, remote_priority);
cricket::Port* port = GetPort(&ch);
// conn1 should be resurrected with original priority.
port->SignalUnknownAddress(port, rtc::SocketAddress("1.1.1.1", 1),
cricket::PROTO_UDP, &request, kIceUfrag[1], false);
@ -2068,3 +2092,70 @@ TEST_F(P2PTransportChannelPingTest, TestDontPruneWhenWeak) {
WAIT(conn3->pruned(), 1000);
EXPECT_FALSE(conn3->pruned());
}
// Test that GetState returns the state correctly.
TEST_F(P2PTransportChannelPingTest, TestGetState) {
cricket::FakePortAllocator pa(rtc::Thread::Current(), nullptr);
cricket::P2PTransportChannel ch("test channel", 1, nullptr, &pa);
PrepareChannel(&ch);
ch.Connect();
ch.MaybeStartGathering();
EXPECT_EQ(cricket::TransportChannelState::STATE_INIT, ch.GetState());
ch.AddRemoteCandidate(CreateCandidate("1.1.1.1", 1, 100));
ch.AddRemoteCandidate(CreateCandidate("2.2.2.2", 2, 1));
cricket::Connection* conn1 = WaitForConnectionTo(&ch, "1.1.1.1", 1);
cricket::Connection* conn2 = WaitForConnectionTo(&ch, "2.2.2.2", 2);
ASSERT_TRUE(conn1 != nullptr);
ASSERT_TRUE(conn2 != nullptr);
// Now there are two connections, so the transport channel is connecting.
EXPECT_EQ(cricket::TransportChannelState::STATE_CONNECTING, ch.GetState());
// |conn1| becomes writable and receiving; it then should prune |conn2|.
conn1->ReceivedPingResponse();
EXPECT_TRUE_WAIT(conn2->pruned(), 1000);
EXPECT_EQ(cricket::TransportChannelState::STATE_COMPLETED, ch.GetState());
conn1->Prune(); // All connections are pruned.
EXPECT_EQ(cricket::TransportChannelState::STATE_FAILED, ch.GetState());
}
// Test that when a low-priority connection is pruned, it is not deleted
// right away, and it can become active and be pruned again.
TEST_F(P2PTransportChannelPingTest, TestConnectionPrunedAgain) {
cricket::FakePortAllocator pa(rtc::Thread::Current(), nullptr);
cricket::P2PTransportChannel ch("test channel", 1, nullptr, &pa);
PrepareChannel(&ch);
ch.SetIceConfig(CreateIceConfig(1000, false));
ch.Connect();
ch.MaybeStartGathering();
ch.AddRemoteCandidate(CreateCandidate("1.1.1.1", 1, 100));
cricket::Connection* conn1 = WaitForConnectionTo(&ch, "1.1.1.1", 1);
ASSERT_TRUE(conn1 != nullptr);
EXPECT_EQ(conn1, ch.best_connection());
conn1->ReceivedPingResponse(); // Becomes writable and receiving
// Add a low-priority connection |conn2|, which will be pruned, but it will
// not be deleted right away. Once the current best connection becomes not
// receiving, |conn2| will start to ping and upon receiving the ping response,
// it will become the best connection.
ch.AddRemoteCandidate(CreateCandidate("2.2.2.2", 2, 1));
cricket::Connection* conn2 = WaitForConnectionTo(&ch, "2.2.2.2", 2);
ASSERT_TRUE(conn2 != nullptr);
EXPECT_TRUE_WAIT(!conn2->active(), 1000);
// |conn2| should not send a ping yet.
EXPECT_EQ(cricket::Connection::STATE_WAITING, conn2->state());
EXPECT_EQ(cricket::TransportChannelState::STATE_COMPLETED, ch.GetState());
// Wait for |conn1| becoming not receiving.
EXPECT_TRUE_WAIT(!conn1->receiving(), 3000);
// Make sure conn2 is not deleted.
conn2 = WaitForConnectionTo(&ch, "2.2.2.2", 2);
ASSERT_TRUE(conn2 != nullptr);
EXPECT_EQ_WAIT(cricket::Connection::STATE_INPROGRESS, conn2->state(), 1000);
conn2->ReceivedPingResponse();
EXPECT_EQ_WAIT(conn2, ch.best_connection(), 1000);
EXPECT_EQ(cricket::TransportChannelState::STATE_CONNECTING, ch.GetState());
// When |conn1| comes back again, |conn2| will be pruned again.
conn1->ReceivedPingResponse();
EXPECT_EQ_WAIT(conn1, ch.best_connection(), 1000);
EXPECT_TRUE_WAIT(!conn2->active(), 1000);
EXPECT_EQ(cricket::TransportChannelState::STATE_COMPLETED, ch.GetState());
}

41
webrtc/p2p/base/port.cc
View File

@ -801,7 +801,8 @@ Connection::Connection(Port* port,
sent_packets_total_(0),
reported_(false),
state_(STATE_WAITING),
receiving_timeout_(WEAK_CONNECTION_RECEIVE_TIMEOUT) {
receiving_timeout_(WEAK_CONNECTION_RECEIVE_TIMEOUT),
time_created_ms_(rtc::Time()) {
// All of our connections start in WAITING state.
// TODO(mallinath) - Start connections from STATE_FROZEN.
// Wire up to send stun packets
@ -849,7 +850,6 @@ void Connection::set_write_state(WriteState value) {
LOG_J(LS_VERBOSE, this) << "set_write_state from: " << old_value << " to "
<< value;
SignalStateChange(this);
CheckTimeout();
}
}
@ -858,7 +858,6 @@ void Connection::set_receiving(bool value) {
LOG_J(LS_VERBOSE, this) << "set_receiving to " << value;
receiving_ = value;
SignalStateChange(this);
CheckTimeout();
}
}
@ -999,7 +998,7 @@ void Connection::OnReadyToSend() {
}
void Connection::Prune() {
if (!pruned_) {
if (!pruned_ || active()) {
LOG_J(LS_VERBOSE, this) << "Connection pruned";
pruned_ = true;
requests_.Clear();
@ -1089,6 +1088,9 @@ void Connection::UpdateState(uint32 now) {
uint32 last_recv_time = last_received();
bool receiving = now <= last_recv_time + receiving_timeout_;
set_receiving(receiving);
if (dead(now)) {
Destroy();
}
}
void Connection::Ping(uint32 now) {
@ -1119,6 +1121,28 @@ void Connection::ReceivedPingResponse() {
last_ping_response_received_ = rtc::Time();
}
bool Connection::dead(uint32 now) const {
if (now < (time_created_ms_ + MIN_CONNECTION_LIFETIME)) {
// A connection that hasn't passed its minimum lifetime is still alive.
// We do this to prevent connections from being pruned too quickly
// during a network change event when two networks would be up
// simultaneously but only for a brief period.
return false;
}
if (receiving_) {
// A connection that is receiving is alive.
return false;
}
// A connection is alive until it is inactive.
return !active();
// TODO(honghaiz): Move from using the write state to using the receiving
// state with something like the following:
// return (now > (last_received() + DEAD_CONNECTION_RECEIVE_TIMEOUT));
}
std::string Connection::ToDebugId() const {
std::stringstream ss;
ss << std::hex << this;
@ -1230,7 +1254,7 @@ void Connection::OnConnectionRequestErrorResponse(ConnectionRequest* request,
LOG_J(LS_ERROR, this) << "Received STUN error response, code="
<< error_code << "; killing connection";
set_state(STATE_FAILED);
set_write_state(STATE_WRITE_TIMEOUT);
Destroy();
}
}
@ -1251,13 +1275,6 @@ void Connection::OnConnectionRequestSent(ConnectionRequest* request) {
<< ", use_candidate=" << use_candidate;
}
void Connection::CheckTimeout() {
// If write has timed out and it is not receiving, remove the connection.
if (!receiving_ && write_state_ == STATE_WRITE_TIMEOUT) {
Destroy();
}
}
void Connection::HandleRoleConflictFromPeer() {
port_->SignalRoleConflict(port_);
}

19
webrtc/p2p/base/port.h
View File

@ -50,9 +50,9 @@ extern const char TCPTYPE_ACTIVE_STR[];
extern const char TCPTYPE_PASSIVE_STR[];
extern const char TCPTYPE_SIMOPEN_STR[];
// If a connection does not receive anything for this long, it is considered
// dead.
const uint32 DEAD_CONNECTION_RECEIVE_TIMEOUT = 30 * 1000; // 30 seconds.
// The minimum time we will wait before destroying a connection after creating
// it.
const uint32 MIN_CONNECTION_LIFETIME = 10 * 1000; // 10 seconds.
// The timeout duration when a connection does not receive anything.
const uint32 WEAK_CONNECTION_RECEIVE_TIMEOUT = 2500; // 2.5 seconds
@ -435,8 +435,13 @@ class Connection : public rtc::MessageHandler,
// Determines whether the connection has finished connecting. This can only
// be false for TCP connections.
bool connected() const { return connected_; }
bool Weak() const { return !(writable() && receiving() && connected()); }
bool weak() const { return !(writable() && receiving() && connected()); }
bool active() const {
// TODO(honghaiz): Move from using |write_state_| to using |pruned_|.
return write_state_ != STATE_WRITE_TIMEOUT;
}
// A connection is dead if it can be safely deleted.
bool dead(uint32 now) const;
// Estimate of the round-trip time over this connection.
uint32 rtt() const { return rtt_; }
@ -572,9 +577,6 @@ class Connection : public rtc::MessageHandler,
void set_state(State state);
void set_connected(bool value);
// Checks if this connection is useless, and hence, should be destroyed.
void CheckTimeout();
void OnMessage(rtc::Message *pmsg);
Port* port_;
@ -615,6 +617,7 @@ class Connection : public rtc::MessageHandler,
State state_;
// Time duration to switch from receiving to not receiving.
uint32 receiving_timeout_;
uint32 time_created_ms_;
friend class Port;
friend class ConnectionRequest;