From 9794366ff0d8aa2cc4b07a130bae7d4987b232f7 Mon Sep 17 00:00:00 2001 From: deadbeef Date: Tue, 12 Jul 2016 11:04:50 -0700 Subject: [PATCH] Fixing memory leak in TurnServer. If the test TURN server received two allocate requests from the same address, it was replacing the old allocation but not deleting it. Also switching to std::unique_ptr to make it less likely for this to pop up again. Review-Url: https://codereview.webrtc.org/2114063002 Cr-Commit-Position: refs/heads/master@{#13449} --- webrtc/p2p/base/testturnserver.h | 2 +- webrtc/p2p/base/turnserver.cc | 13 +++++-------- webrtc/p2p/base/turnserver.h | 3 ++- 3 files changed, 8 insertions(+), 10 deletions(-) diff --git a/webrtc/p2p/base/testturnserver.h b/webrtc/p2p/base/testturnserver.h index 3d6074dc9b..43d363cb35 100644 --- a/webrtc/p2p/base/testturnserver.h +++ b/webrtc/p2p/base/testturnserver.h @@ -99,7 +99,7 @@ class TestTurnServer : public TurnAuthInterface { for (TurnServer::AllocationMap::const_iterator it = map.begin(); it != map.end(); ++it) { if (src == it->first.src()) { - return it->second; + return it->second.get(); } } return NULL; diff --git a/webrtc/p2p/base/turnserver.cc b/webrtc/p2p/base/turnserver.cc index b4510e637f..dac7e38184 100644 --- a/webrtc/p2p/base/turnserver.cc +++ b/webrtc/p2p/base/turnserver.cc @@ -124,11 +124,6 @@ TurnServer::TurnServer(rtc::Thread* thread) } TurnServer::~TurnServer() { - for (AllocationMap::iterator it = allocations_.begin(); - it != allocations_.end(); ++it) { - delete it->second; - } - for (InternalSocketMap::iterator it = server_sockets_.begin(); it != server_sockets_.end(); ++it) { rtc::AsyncPacketSocket* socket = it->first; @@ -429,7 +424,7 @@ bool TurnServer::ValidateNonce(const std::string& nonce) const { TurnServerAllocation* TurnServer::FindAllocation(TurnServerConnection* conn) { AllocationMap::const_iterator it = allocations_.find(*conn); - return (it != allocations_.end()) ? it->second : NULL; + return (it != allocations_.end()) ? it->second.get() : nullptr; } TurnServerAllocation* TurnServer::CreateAllocation(TurnServerConnection* conn, @@ -445,7 +440,7 @@ TurnServerAllocation* TurnServer::CreateAllocation(TurnServerConnection* conn, TurnServerAllocation* allocation = new TurnServerAllocation(this, thread_, *conn, external_socket, key); allocation->SignalDestroyed.connect(this, &TurnServer::OnAllocationDestroyed); - allocations_[*conn] = allocation; + allocations_[*conn].reset(allocation); return allocation; } @@ -518,8 +513,10 @@ void TurnServer::OnAllocationDestroyed(TurnServerAllocation* allocation) { } AllocationMap::iterator it = allocations_.find(*(allocation->conn())); - if (it != allocations_.end()) + if (it != allocations_.end()) { + it->second.release(); allocations_.erase(it); + } } void TurnServer::DestroyInternalSocket(rtc::AsyncPacketSocket* socket) { diff --git a/webrtc/p2p/base/turnserver.h b/webrtc/p2p/base/turnserver.h index ed281b46f0..09b553ef17 100644 --- a/webrtc/p2p/base/turnserver.h +++ b/webrtc/p2p/base/turnserver.h @@ -161,7 +161,8 @@ class TurnRedirectInterface { // Not yet wired up: TCP support. class TurnServer : public sigslot::has_slots<> { public: - typedef std::map AllocationMap; + typedef std::map> + AllocationMap; explicit TurnServer(rtc::Thread* thread); ~TurnServer();