Cease all future TURN requests when a TURN refresh request fails for a given TURN port.
This fixes an assert error in Turnport::OnSendStunPacket
BUG=webrtc:5388
Review URL: https://codereview.webrtc.org/1547373002
Cr-Commit-Position: refs/heads/master@{#11152}
diff --git a/webrtc/p2p/base/stunrequest.cc b/webrtc/p2p/base/stunrequest.cc
index 0a0b1a8..ce0364e 100644
--- a/webrtc/p2p/base/stunrequest.cc
+++ b/webrtc/p2p/base/stunrequest.cc
@@ -53,11 +53,13 @@
}
}
-void StunRequestManager::Flush() {
+void StunRequestManager::Flush(int msg_type) {
for (const auto kv : requests_) {
StunRequest* request = kv.second;
- thread_->Clear(request, MSG_STUN_SEND);
- thread_->Send(request, MSG_STUN_SEND, NULL);
+ if (msg_type == kAllRequests || msg_type == request->type()) {
+ thread_->Clear(request, MSG_STUN_SEND);
+ thread_->Send(request, MSG_STUN_SEND, NULL);
+ }
}
}
diff --git a/webrtc/p2p/base/stunrequest.h b/webrtc/p2p/base/stunrequest.h
index 15ea0c7..44c1ebf 100644
--- a/webrtc/p2p/base/stunrequest.h
+++ b/webrtc/p2p/base/stunrequest.h
@@ -21,6 +21,8 @@
class StunRequest;
+const int kAllRequests = 0;
+
// Manages a set of STUN requests, sending and resending until we receive a
// response or determine that the request has timed out.
class StunRequestManager {
@@ -32,8 +34,10 @@
void Send(StunRequest* request);
void SendDelayed(StunRequest* request, int delay);
- // Sends all pending requests right away. Only for testing.
- void Flush();
+ // If |msg_type| is kAllRequests, sends all pending requests right away.
+ // Otherwise, sends those that have a matching type right away.
+ // Only for testing.
+ void Flush(int msg_type);
// Removes a stun request that was added previously. This will happen
// automatically when a request succeeds, fails, or times out.
diff --git a/webrtc/p2p/base/turnport.cc b/webrtc/p2p/base/turnport.cc
index 328683b..022207a 100644
--- a/webrtc/p2p/base/turnport.cc
+++ b/webrtc/p2p/base/turnport.cc
@@ -409,11 +409,7 @@
void TurnPort::OnSocketClose(rtc::AsyncPacketSocket* socket, int error) {
LOG_J(LS_WARNING, this) << "Connection with server failed, error=" << error;
ASSERT(socket == socket_);
- if (!ready()) {
- OnAllocateError();
- }
- request_manager_.Clear();
- state_ = STATE_DISCONNECTED;
+ Close();
}
void TurnPort::OnAllocateMismatch() {
@@ -717,7 +713,18 @@
thread()->Post(this, MSG_ALLOCATE_ERROR);
}
+void TurnPort::OnTurnRefreshError() {
+ // Need to Close the port asynchronously because otherwise, the refresh
+ // request may be deleted twice: once at the end of the message processing
+ // and the other in Close().
+ thread()->Post(this, MSG_REFRESH_ERROR);
+}
+
void TurnPort::Close() {
+ if (!ready()) {
+ OnAllocateError();
+ }
+ request_manager_.Clear();
// Stop the port from creating new connections.
state_ = STATE_DISCONNECTED;
// Delete all existing connections; stop sending data.
@@ -734,6 +741,9 @@
case MSG_ALLOCATE_MISMATCH:
OnAllocateMismatch();
break;
+ case MSG_REFRESH_ERROR:
+ Close();
+ break;
case MSG_TRY_ALTERNATE_SERVER:
if (server_address().proto == PROTO_UDP) {
// Send another allocate request to alternate server, with the received
diff --git a/webrtc/p2p/base/turnport.h b/webrtc/p2p/base/turnport.h
index 4be9249..a19f676 100644
--- a/webrtc/p2p/base/turnport.h
+++ b/webrtc/p2p/base/turnport.h
@@ -141,7 +141,8 @@
sigslot::signal2<TurnPort*, int> SignalTurnRefreshResult;
sigslot::signal3<TurnPort*, const rtc::SocketAddress&, int>
SignalCreatePermissionResult;
- void FlushRequests() { request_manager_.Flush(); }
+ void FlushRequests(int msg_type) { request_manager_.Flush(msg_type); }
+ bool HasRequests() { return !request_manager_.empty(); }
void set_credentials(RelayCredentials& credentials) {
credentials_ = credentials;
}
@@ -178,7 +179,8 @@
enum {
MSG_ALLOCATE_ERROR = MSG_FIRST_AVAILABLE,
MSG_ALLOCATE_MISMATCH,
- MSG_TRY_ALTERNATE_SERVER
+ MSG_TRY_ALTERNATE_SERVER,
+ MSG_REFRESH_ERROR
};
typedef std::list<TurnEntry*> EntryList;
@@ -199,7 +201,7 @@
// Shuts down the turn port, usually because of some fatal errors.
void Close();
- void OnTurnRefreshError() { Close(); }
+ void OnTurnRefreshError();
bool SetAlternateServer(const rtc::SocketAddress& address);
void ResolveTurnAddress(const rtc::SocketAddress& address);
void OnResolveResult(rtc::AsyncResolverInterface* resolver);
diff --git a/webrtc/p2p/base/turnport_unittest.cc b/webrtc/p2p/base/turnport_unittest.cc
index eac3474..d0dc487 100644
--- a/webrtc/p2p/base/turnport_unittest.cc
+++ b/webrtc/p2p/base/turnport_unittest.cc
@@ -309,7 +309,7 @@
}
bool CheckConnectionDestroyed() {
- turn_port_->FlushRequests();
+ turn_port_->FlushRequests(cricket::kAllRequests);
rtc::Thread::Current()->ProcessMessages(50);
return connection_destroyed_;
}
@@ -693,8 +693,9 @@
TEST_F(TurnPortTest, TestRefreshRequestGetsErrorResponse) {
CreateTurnPort(kTurnUsername, kTurnPassword, kTurnUdpProtoAddr);
- turn_port_->PrepareAddress();
- EXPECT_TRUE_WAIT(turn_ready_, kTimeout);
+ PrepareTurnAndUdpPorts();
+ turn_port_->CreateConnection(udp_port_->Candidates()[0],
+ Port::ORIGIN_MESSAGE);
// Set bad credentials.
cricket::RelayCredentials bad_credentials("bad_user", "bad_pwd");
turn_port_->set_credentials(bad_credentials);
@@ -702,13 +703,14 @@
// This sends out the first RefreshRequest with correct credentials.
// When this succeeds, it will schedule a new RefreshRequest with the bad
// credential.
- turn_port_->FlushRequests();
+ turn_port_->FlushRequests(cricket::TURN_REFRESH_REQUEST);
EXPECT_TRUE_WAIT(turn_refresh_success_, kTimeout);
// Flush it again, it will receive a bad response.
- turn_port_->FlushRequests();
+ turn_port_->FlushRequests(cricket::TURN_REFRESH_REQUEST);
EXPECT_TRUE_WAIT(!turn_refresh_success_, kTimeout);
+ EXPECT_TRUE_WAIT(!turn_port_->connected(), kTimeout);
EXPECT_TRUE(turn_port_->connections().empty());
- EXPECT_FALSE(turn_port_->connected());
+ EXPECT_FALSE(turn_port_->HasRequests());
}
// Test that CreateConnection will return null if port becomes disconnected.
@@ -840,10 +842,10 @@
// another request with bad_ufrag and bad_pwd.
cricket::RelayCredentials bad_credentials("bad_user", "bad_pwd");
turn_port_->set_credentials(bad_credentials);
- turn_port_->FlushRequests();
+ turn_port_->FlushRequests(cricket::kAllRequests);
ASSERT_TRUE_WAIT(turn_create_permission_success_, kTimeout);
// Flush the requests again; the create-permission-request will fail.
- turn_port_->FlushRequests();
+ turn_port_->FlushRequests(cricket::kAllRequests);
EXPECT_TRUE_WAIT(!turn_create_permission_success_, kTimeout);
EXPECT_TRUE_WAIT(connection_destroyed_, kTimeout);
}
