From c309e0e3eaa90411ffedd83f50313b8b9d9826b5 Mon Sep 17 00:00:00 2001 From: skvlad Date: Thu, 28 Jul 2016 17:15:20 -0700 Subject: [PATCH] Don't stop sending media on EWOULDBLOCK This change makes WebRTC no longer stop sending video when we receive an EWOULDBLOCK error from the operating system. This was previously causing calls on a slow link (where the first hop is slow) to rapidly oscillate between starting and stopping video. We still do need to stop sending packets if there is no known good connection we can use for that. We used to generate a synthetic EWOULDBLOCK error in that case. This CL replaces it with a different code (ENOTCONN); EWOULDBLOCK no longer stops the stream but ENOTCONN does. I've updated all the places where we seemed to be generating EWOULDBLOCK for reasons other than some buffer been full; please give it a thorough look in case I missed something. R=pthatcher@webrtc.org Review URL: https://codereview.webrtc.org/2192963002 . Cr-Commit-Position: refs/heads/master@{#13566} --- webrtc/base/openssladapter.cc | 4 ++-- webrtc/p2p/base/p2ptransportchannel.cc | 6 +++--- webrtc/p2p/base/port_unittest.cc | 4 ++-- webrtc/p2p/base/relayport.cc | 2 +- webrtc/p2p/base/tcpport.cc | 2 +- webrtc/p2p/base/turnport.cc | 2 +- webrtc/pc/channel.cc | 4 ++-- 7 files changed, 12 insertions(+), 12 deletions(-) diff --git a/webrtc/base/openssladapter.cc b/webrtc/base/openssladapter.cc index 0a416cf03b..259215239e 100644 --- a/webrtc/base/openssladapter.cc +++ b/webrtc/base/openssladapter.cc @@ -468,7 +468,7 @@ OpenSSLAdapter::Send(const void* pv, size_t cb) { case SSL_WAIT: case SSL_CONNECTING: - SetError(EWOULDBLOCK); + SetError(ENOTCONN); return SOCKET_ERROR; case SSL_CONNECTED: @@ -534,7 +534,7 @@ int OpenSSLAdapter::Recv(void* pv, size_t cb, int64_t* timestamp) { case SSL_WAIT: case SSL_CONNECTING: - SetError(EWOULDBLOCK); + SetError(ENOTCONN); return SOCKET_ERROR; case SSL_CONNECTED: diff --git a/webrtc/p2p/base/p2ptransportchannel.cc b/webrtc/p2p/base/p2ptransportchannel.cc index c12e8dbf66..93b2e3dcc6 100644 --- a/webrtc/p2p/base/p2ptransportchannel.cc +++ b/webrtc/p2p/base/p2ptransportchannel.cc @@ -928,10 +928,10 @@ int P2PTransportChannel::SendPacket(const char *data, size_t len, error_ = EINVAL; return -1; } - // If we don't think the connection is working yet, return EWOULDBLOCK + // If we don't think the connection is working yet, return ENOTCONN // instead of sending a packet that will probably be dropped. if (!ReadyToSend()) { - error_ = EWOULDBLOCK; + error_ = ENOTCONN; return -1; } @@ -1292,7 +1292,7 @@ void P2PTransportChannel::SwitchSelectedConnection(Connection* conn) { << selected_connection_->ToString(); SignalRouteChange(this, selected_connection_->remote_candidate()); // This is a temporary, but safe fix to webrtc issue 5705. - // TODO(honghaiz): Make all EWOULDBLOCK error routed through the transport + // TODO(honghaiz): Make all ENOTCONN error routed through the transport // channel so that it knows whether the media channel is allowed to // send; then it will only signal ready-to-send if the media channel // has been disallowed to send. diff --git a/webrtc/p2p/base/port_unittest.cc b/webrtc/p2p/base/port_unittest.cc index e84af9f2be..12a37a237e 100644 --- a/webrtc/p2p/base/port_unittest.cc +++ b/webrtc/p2p/base/port_unittest.cc @@ -341,7 +341,7 @@ class TestChannel : public sigslot::has_slots<> { } private: - // ReadyToSend will only issue after a Connection recovers from EWOULDBLOCK. + // ReadyToSend will only issue after a Connection recovers from ENOTCONN void OnConnectionReadyToSend(Connection* conn) { ASSERT_EQ(conn, conn_); connection_ready_to_send_ = true; @@ -696,7 +696,7 @@ class PortTest : public testing::Test, public sigslot::has_slots<> { EXPECT_TRUE_WAIT(ch1.connection_ready_to_send(), kTcpReconnectTimeout); // Channel2 is the passive one so a new connection is created during - // reconnect. This new connection should never have issued EWOULDBLOCK + // reconnect. This new connection should never have issued ENOTCONN // hence the connection_ready_to_send() should be false. EXPECT_FALSE(ch2.connection_ready_to_send()); } else { diff --git a/webrtc/p2p/base/relayport.cc b/webrtc/p2p/base/relayport.cc index 21dc41a126..795fa5aad4 100644 --- a/webrtc/p2p/base/relayport.cc +++ b/webrtc/p2p/base/relayport.cc @@ -344,7 +344,7 @@ int RelayPort::SendTo(const void* data, size_t size, ASSERT(!entries_.empty()); entry = entries_[0]; if (!entry->connected()) { - error_ = EWOULDBLOCK; + error_ = ENOTCONN; return SOCKET_ERROR; } } diff --git a/webrtc/p2p/base/tcpport.cc b/webrtc/p2p/base/tcpport.cc index 9a1caa2554..9a788706aa 100644 --- a/webrtc/p2p/base/tcpport.cc +++ b/webrtc/p2p/base/tcpport.cc @@ -348,7 +348,7 @@ int TCPConnection::Send(const void* data, size_t size, // the connection a chance to reconnect. if (pretending_to_be_writable_ || write_state() != STATE_WRITABLE) { // TODO: Should STATE_WRITE_TIMEOUT return a non-blocking error? - error_ = EWOULDBLOCK; + error_ = ENOTCONN; return SOCKET_ERROR; } stats_.sent_total_packets++; diff --git a/webrtc/p2p/base/turnport.cc b/webrtc/p2p/base/turnport.cc index 573ca907d6..2d5e30e663 100644 --- a/webrtc/p2p/base/turnport.cc +++ b/webrtc/p2p/base/turnport.cc @@ -518,7 +518,7 @@ int TurnPort::SendTo(const void* data, size_t size, } if (!ready()) { - error_ = EWOULDBLOCK; + error_ = ENOTCONN; return SOCKET_ERROR; } diff --git a/webrtc/pc/channel.cc b/webrtc/pc/channel.cc index c1909a2829..cde1355d23 100644 --- a/webrtc/pc/channel.cc +++ b/webrtc/pc/channel.cc @@ -758,8 +758,8 @@ bool BaseChannel::SendPacket(bool rtcp, int ret = channel->SendPacket(packet->data(), packet->size(), updated_options, flags); if (ret != static_cast(packet->size())) { - if (channel->GetError() == EWOULDBLOCK) { - LOG(LS_WARNING) << "Got EWOULDBLOCK from socket."; + if (channel->GetError() == ENOTCONN) { + LOG(LS_WARNING) << "Got ENOTCONN from transport."; SetReadyToSend(rtcp, false); } return false;