Fix Turn TCP port issue.
Sometimes the port still try to send stun packet when the connection is disconnected,
causing an assertion error.
BUG=4859
Review URL: https://codereview.webrtc.org/1247573002
Cr-Commit-Position: refs/heads/master@{#9671}
diff --git a/webrtc/base/asynctcpsocket.cc b/webrtc/base/asynctcpsocket.cc
index 0f7abd5..97d1e17 100644
--- a/webrtc/base/asynctcpsocket.cc
+++ b/webrtc/base/asynctcpsocket.cc
@@ -127,10 +127,11 @@
int AsyncTCPSocketBase::SendTo(const void *pv, size_t cb,
const SocketAddress& addr,
const rtc::PacketOptions& options) {
- if (addr == GetRemoteAddress())
+ const SocketAddress& remote_address = GetRemoteAddress();
+ if (addr == remote_address)
return Send(pv, cb, options);
-
- ASSERT(false);
+ // Remote address may be empty if there is a sudden network change.
+ ASSERT(remote_address.IsNil());
socket_->SetError(ENOTCONN);
return -1;
}
diff --git a/webrtc/p2p/base/turnport.cc b/webrtc/p2p/base/turnport.cc
index 27f88dc..2c9cd13 100644
--- a/webrtc/p2p/base/turnport.cc
+++ b/webrtc/p2p/base/turnport.cc
@@ -172,8 +172,12 @@
const RelayCredentials& credentials,
int server_priority,
const std::string& origin)
- : Port(thread, factory, network, socket->GetLocalAddress().ipaddr(),
- username, password),
+ : Port(thread,
+ factory,
+ network,
+ socket->GetLocalAddress().ipaddr(),
+ username,
+ password),
server_address_(server_address),
credentials_(credentials),
socket_(socket),
@@ -181,7 +185,7 @@
error_(0),
request_manager_(thread),
next_channel_number_(TURN_CHANNEL_NUMBER_START),
- connected_(false),
+ state_(STATE_CONNECTING),
server_priority_(server_priority),
allocate_mismatch_retries_(0) {
request_manager_.SignalSendPacket.connect(this, &TurnPort::OnSendStunPacket);
@@ -200,8 +204,15 @@
const RelayCredentials& credentials,
int server_priority,
const std::string& origin)
- : Port(thread, RELAY_PORT_TYPE, factory, network, ip, min_port, max_port,
- username, password),
+ : Port(thread,
+ RELAY_PORT_TYPE,
+ factory,
+ network,
+ ip,
+ min_port,
+ max_port,
+ username,
+ password),
server_address_(server_address),
credentials_(credentials),
socket_(NULL),
@@ -209,7 +220,7 @@
error_(0),
request_manager_(thread),
next_channel_number_(TURN_CHANNEL_NUMBER_START),
- connected_(false),
+ state_(STATE_CONNECTING),
server_priority_(server_priority),
allocate_mismatch_retries_(0) {
request_manager_.SignalSendPacket.connect(this, &TurnPort::OnSendStunPacket);
@@ -221,7 +232,7 @@
// release the allocation by sending a refresh with
// lifetime 0.
- if (connected_) {
+ if (ready()) {
TurnRefreshRequest bye(this);
bye.set_lifetime(0);
SendRequest(&bye, 0);
@@ -261,8 +272,9 @@
} else {
// If protocol family of server address doesn't match with local, return.
if (!IsCompatibleAddress(server_address_.address)) {
- LOG(LS_ERROR) << "Server IP address family does not match with "
- << "local host address family type";
+ LOG(LS_ERROR) << "IP address family does not match: "
+ << "server: " << server_address_.address.family()
+ << "local: " << ip().family();
OnAllocateError();
return;
}
@@ -274,8 +286,11 @@
<< ProtoToString(server_address_.proto) << " @ "
<< server_address_.address.ToSensitiveString();
if (!CreateTurnClientSocket()) {
+ LOG(LS_ERROR) << "Failed to create TURN client socket";
OnAllocateError();
- } else if (server_address_.proto == PROTO_UDP) {
+ return;
+ }
+ if (server_address_.proto == PROTO_UDP) {
// If its UDP, send AllocateRequest now.
// For TCP and TLS AllcateRequest will be sent by OnSocketConnect.
SendRequest(new TurnAllocateRequest(this), 0);
@@ -319,9 +334,13 @@
socket_->SignalReadyToSend.connect(this, &TurnPort::OnReadyToSend);
+ // TCP port is ready to send stun requests after the socket is connected,
+ // while UDP port is ready to do so once the socket is created.
if (server_address_.proto == PROTO_TCP) {
socket_->SignalConnect.connect(this, &TurnPort::OnSocketConnect);
socket_->SignalClose.connect(this, &TurnPort::OnSocketClose);
+ } else {
+ state_ = STATE_CONNECTED;
}
return true;
}
@@ -360,6 +379,7 @@
}
}
+ state_ = STATE_CONNECTED; // It is ready to send stun requests.
if (server_address_.address.IsUnresolved()) {
server_address_.address = socket_->GetRemoteAddress();
}
@@ -372,10 +392,11 @@
void TurnPort::OnSocketClose(rtc::AsyncPacketSocket* socket, int error) {
LOG_J(LS_WARNING, this) << "Connection with server failed, error=" << error;
ASSERT(socket == socket_);
- if (!connected_) {
+ if (!ready()) {
OnAllocateError();
}
- connected_ = false;
+ request_manager_.Clear();
+ state_ = STATE_DISCONNECTED;
}
void TurnPort::OnAllocateMismatch() {
@@ -412,6 +433,10 @@
return NULL;
}
+ if (state_ == STATE_DISCONNECTED) {
+ return NULL;
+ }
+
// Create an entry, if needed, so we can get our permissions set up correctly.
CreateEntry(address.address());
@@ -462,12 +487,12 @@
bool payload) {
// Try to find an entry for this specific address; we should have one.
TurnEntry* entry = FindEntry(addr);
- ASSERT(entry != NULL);
if (!entry) {
+ LOG(LS_ERROR) << "Did not find the TurnEntry for address " << addr;
return 0;
}
- if (!connected()) {
+ if (!ready()) {
error_ = EWOULDBLOCK;
return SOCKET_ERROR;
}
@@ -536,7 +561,7 @@
}
void TurnPort::OnReadyToSend(rtc::AsyncPacketSocket* socket) {
- if (connected_) {
+ if (ready()) {
Port::OnReadyToSend();
}
}
@@ -616,6 +641,7 @@
void TurnPort::OnSendStunPacket(const void* data, size_t size,
StunRequest* request) {
+ ASSERT(connected());
rtc::PacketOptions options(DefaultDscpValue());
if (Send(data, size, options) < 0) {
LOG_J(LS_ERROR, this) << "Failed to send TURN message, err="
@@ -635,7 +661,7 @@
void TurnPort::OnAllocateSuccess(const rtc::SocketAddress& address,
const rtc::SocketAddress& stun_address) {
- connected_ = true;
+ state_ = STATE_READY;
rtc::SocketAddress related_address = stun_address;
if (!(candidate_filter() & CF_REFLEXIVE)) {
diff --git a/webrtc/p2p/base/turnport.h b/webrtc/p2p/base/turnport.h
index 5bb7558..52546e0 100644
--- a/webrtc/p2p/base/turnport.h
+++ b/webrtc/p2p/base/turnport.h
@@ -33,6 +33,12 @@
class TurnPort : public Port {
public:
+ enum PortState {
+ STATE_CONNECTING, // Initial state, cannot send any packets.
+ STATE_CONNECTED, // Socket connected, ready to send stun requests.
+ STATE_READY, // Received allocate success, can send any packets.
+ STATE_DISCONNECTED, // TCP connection died, cannot send any packets.
+ };
static TurnPort* Create(rtc::Thread* thread,
rtc::PacketSocketFactory* factory,
rtc::Network* network,
@@ -70,7 +76,10 @@
// Returns an empty address if the local address has not been assigned.
rtc::SocketAddress GetLocalAddress() const;
- bool connected() const { return connected_; }
+ bool ready() const { return state_ == STATE_READY; }
+ bool connected() const {
+ return state_ == STATE_READY || state_ == STATE_CONNECTED;
+ }
const RelayCredentials& credentials() const { return credentials_; }
virtual void PrepareAddress();
@@ -225,7 +234,7 @@
int next_channel_number_;
EntryList entries_;
- bool connected_;
+ PortState state_;
// By default the value will be set to 0. This value will be used in
// calculating the candidate priority.
int server_priority_;
diff --git a/webrtc/p2p/base/turnport_unittest.cc b/webrtc/p2p/base/turnport_unittest.cc
index 3172ba2..00a62fa 100644
--- a/webrtc/p2p/base/turnport_unittest.cc
+++ b/webrtc/p2p/base/turnport_unittest.cc
@@ -627,6 +627,28 @@
EXPECT_NE(first_addr, turn_port_->socket()->GetLocalAddress());
}
+// Test that CreateConnection will return null if port becomes disconnected.
+TEST_F(TurnPortTest, TestCreateConnectionWhenSocketClosed) {
+ turn_server_.AddInternalSocket(kTurnTcpIntAddr, cricket::PROTO_TCP);
+ CreateTurnPort(kTurnUsername, kTurnPassword, kTurnTcpProtoAddr);
+ turn_port_->PrepareAddress();
+ ASSERT_TRUE_WAIT(turn_ready_, kTimeout);
+
+ CreateUdpPort();
+ udp_port_->PrepareAddress();
+ ASSERT_TRUE_WAIT(udp_ready_, kTimeout);
+ // Create a connection.
+ Connection* conn1 = turn_port_->CreateConnection(udp_port_->Candidates()[0],
+ Port::ORIGIN_MESSAGE);
+ ASSERT_TRUE(conn1 != NULL);
+
+ // Close the socket and create a connection again.
+ turn_port_->OnSocketClose(turn_port_->socket(), 1);
+ conn1 = turn_port_->CreateConnection(udp_port_->Candidates()[0],
+ Port::ORIGIN_MESSAGE);
+ ASSERT_TRUE(conn1 == NULL);
+}
+
// Test try-alternate-server feature.
TEST_F(TurnPortTest, TestTurnAlternateServerUDP) {
TestTurnAlternateServer(cricket::PROTO_UDP);
