Fixing leaked reference from SCTP transport to DTLS/ICE transport.

This was causing ICE pings to continue going out on PeerConnections
that use DataChannels, even after closing the PeerConnection.

This CL adds a two-line fix, and an integration test that will catch
this and similar issues.

Bug: webrtc:7655
Change-Id: I589a2a1aaf6433c1d65be69a1267e1b52a33534b
Reviewed-on: https://webrtc-review.googlesource.com/37145
Commit-Queue: Taylor Brandstetter <deadbeef@webrtc.org>
Reviewed-by: Steve Anton <steveanton@webrtc.org>
Cr-Commit-Position: refs/heads/master@{#21488}
This commit is contained in:
Taylor Brandstetter 2018-01-03 16:26:06 -08:00 committed by Commit Bot
parent 1b1798112b
commit 389a97c0f4
4 changed files with 36 additions and 0 deletions

View File

@ -5102,6 +5102,8 @@ void PeerConnection::ChangeSctpTransport_n(const std::string& transport_name) {
void PeerConnection::DestroySctpTransport_n() {
RTC_DCHECK(network_thread()->IsCurrent());
sctp_transport_.reset(nullptr);
transport_controller_->DestroyDtlsTransport_n(
*sctp_transport_name_, cricket::ICE_CANDIDATE_COMPONENT_RTP);
sctp_content_name_.reset();
sctp_transport_name_.reset();
sctp_invoker_.reset(nullptr);

View File

@ -3659,6 +3659,31 @@ TEST_F(PeerConnectionIntegrationTest, DisableAndEnableAudioRecording) {
kMaxWaitForFramesMs);
}
// Test that after closing PeerConnections, they stop sending any packets (ICE,
// DTLS, RTP...).
TEST_F(PeerConnectionIntegrationTest, ClosingConnectionStopsPacketFlow) {
// Set up audio/video/data, wait for some frames to be received.
ASSERT_TRUE(CreatePeerConnectionWrappers());
ConnectFakeSignaling();
caller()->AddAudioVideoMediaStream();
#ifdef HAVE_SCTP
caller()->CreateDataChannel();
#endif
caller()->CreateAndSetAndSignalOffer();
ASSERT_TRUE_WAIT(SignalingStateStable(), kDefaultTimeout);
ExpectNewFramesReceivedWithWait(0, 0, kDefaultExpectedAudioFrameCount,
kDefaultExpectedAudioFrameCount,
kMaxWaitForFramesMs);
// Close PeerConnections.
caller()->pc()->Close();
callee()->pc()->Close();
// Pump messages for a second, and ensure no new packets end up sent.
uint32_t sent_packets_a = virtual_socket_server()->sent_packets();
WAIT(false, 1000);
uint32_t sent_packets_b = virtual_socket_server()->sent_packets();
EXPECT_EQ(sent_packets_a, sent_packets_b);
}
} // namespace
#endif // if !defined(THREAD_SANITIZER)

View File

@ -834,6 +834,7 @@ bool VirtualSocketServer::Disconnect(VirtualSocket* socket) {
int VirtualSocketServer::SendUdp(VirtualSocket* socket,
const char* data, size_t data_size,
const SocketAddress& remote_addr) {
++sent_packets_;
if (sending_blocked_) {
CritScope cs(&socket->crit_);
socket->ready_to_send_ = false;
@ -897,6 +898,7 @@ int VirtualSocketServer::SendUdp(VirtualSocket* socket,
}
void VirtualSocketServer::SendTcp(VirtualSocket* socket) {
++sent_packets_;
if (sending_blocked_) {
// Eventually the socket's buffer will fill and VirtualSocket::SendTcp will
// set EWOULDBLOCK.

View File

@ -148,6 +148,10 @@ class VirtualSocketServer : public SocketServer, public sigslot::has_slots<> {
bool CloseTcpConnections(const SocketAddress& addr_local,
const SocketAddress& addr_remote);
// Number of packets that clients have attempted to send through this virtual
// socket server. Intended to be used for test assertions.
uint32_t sent_packets() const { return sent_packets_; }
// For testing purpose only. Fired when a client socket is created.
sigslot::signal1<VirtualSocket*> SignalSocketCreated;
@ -282,6 +286,9 @@ class VirtualSocketServer : public SocketServer, public sigslot::has_slots<> {
uint32_t delay_stddev_;
uint32_t delay_samples_;
// Used for testing.
uint32_t sent_packets_ = 0;
std::map<rtc::IPAddress, int> delay_by_ip_;
std::map<rtc::IPAddress, rtc::IPAddress> alternative_address_mapping_;
std::unique_ptr<Function> delay_dist_;