From 7e5b380437ee64f50c9dde9f88768bf708ba14f8 Mon Sep 17 00:00:00 2001 From: "jiayl@webrtc.org" Date: Thu, 22 Jan 2015 21:28:39 +0000 Subject: [PATCH] Fix a crash in AllocationSequence. Internal bug 19074679. BUG= R=juberti@webrtc.org Review URL: https://webrtc-codereview.appspot.com/38699004 git-svn-id: http://webrtc.googlecode.com/svn/trunk@8130 4adac7df-926f-26a2-2b94-8c16560cd09d --- webrtc/p2p/client/basicportallocator.cc | 10 ++++++++-- webrtc/p2p/client/portallocator_unittest.cc | 15 +++++++++++++++ 2 files changed, 23 insertions(+), 2 deletions(-) diff --git a/webrtc/p2p/client/basicportallocator.cc b/webrtc/p2p/client/basicportallocator.cc index a763791aad..14cd4a21fd 100644 --- a/webrtc/p2p/client/basicportallocator.cc +++ b/webrtc/p2p/client/basicportallocator.cc @@ -906,6 +906,7 @@ void AllocationSequence::CreateUDPPorts() { // UDPPort. if (IsFlagSet(PORTALLOCATOR_ENABLE_SHARED_SOCKET)) { udp_port_ = port; + port->SignalDestroyed.connect(this, &AllocationSequence::OnPortDestroyed); // If STUN is not disabled, setting stun server address to port. if (!IsFlagSet(PORTALLOCATOR_DISABLE_STUN)) { @@ -926,7 +927,6 @@ void AllocationSequence::CreateUDPPorts() { } session_->AddAllocatedPort(port, this, true); - port->SignalDestroyed.connect(this, &AllocationSequence::OnPortDestroyed); } } @@ -1123,7 +1123,13 @@ void AllocationSequence::OnPortDestroyed(PortInterface* port) { return; } - turn_ports_.erase(std::find(turn_ports_.begin(), turn_ports_.end(), port)); + auto it = std::find(turn_ports_.begin(), turn_ports_.end(), port); + if (it != turn_ports_.end()) { + turn_ports_.erase(it); + } else { + LOG(LS_ERROR) << "Unexpected OnPortDestroyed for nonexistent port."; + ASSERT(false); + } } // PortConfiguration diff --git a/webrtc/p2p/client/portallocator_unittest.cc b/webrtc/p2p/client/portallocator_unittest.cc index 5dcc8f9881..0cee3bde7c 100644 --- a/webrtc/p2p/client/portallocator_unittest.cc +++ b/webrtc/p2p/client/portallocator_unittest.cc @@ -1064,3 +1064,18 @@ TEST(HttpPortAllocatorTest, TestSessionRequestUrl) { EXPECT_EQ(kIceUfrag0, args["username"]); EXPECT_EQ(kIcePwd0, args["password"]); } + +// Tests that destroying ports with non-shared sockets does not crash. +// b/19074679. +TEST_F(PortAllocatorTest, TestDestroyPortsNonSharedSockets) { + AddInterface(kClientAddr); + EXPECT_TRUE(CreateSession(cricket::ICE_CANDIDATE_COMPONENT_RTP)); + session_->StartGettingPorts(); + ASSERT_EQ_WAIT(7U, candidates_.size(), kDefaultAllocationTimeout); + EXPECT_EQ(4U, ports_.size()); + + auto it = ports_.begin(); + for (; it != ports_.end(); ++it) { + (reinterpret_cast(*it))->Destroy(); + } +}