From 824f58621315a8b39193e06f960d8ff15f84fc13 Mon Sep 17 00:00:00 2001
From: deadbeef <deadbeef@webrtc.org>
Date: Wed, 24 Aug 2016 15:06:53 -0700
Subject: [PATCH] Fixing segfault caused by TurnServer.

TURN server sockets were being destroyed asynchronously, which could
happen after the TurnServer itself (and even the VirtualSocketServer
used by the sockets) were destroyed.

This is fixed easily by using an AsyncInvoker (to ensure the async
operation doesn't occur after its initiator is destroyed), and keeping
the objects waiting for deletion in a unique_ptr vector.

Review-Url: https://codereview.webrtc.org/2264343002
Cr-Commit-Position: refs/heads/master@{#13907}
---
 webrtc/p2p/base/turnserver.cc | 15 ++++++++++++---
 webrtc/p2p/base/turnserver.h  |  9 +++++++++
 2 files changed, 21 insertions(+), 3 deletions(-)

diff --git a/webrtc/p2p/base/turnserver.cc b/webrtc/p2p/base/turnserver.cc
index 7dd9e19f63..3a574673c3 100644
--- a/webrtc/p2p/base/turnserver.cc
+++ b/webrtc/p2p/base/turnserver.cc
@@ -16,6 +16,7 @@
 #include "webrtc/p2p/base/common.h"
 #include "webrtc/p2p/base/packetsocketfactory.h"
 #include "webrtc/p2p/base/stun.h"
+#include "webrtc/base/bind.h"
 #include "webrtc/base/bytebuffer.h"
 #include "webrtc/base/helpers.h"
 #include "webrtc/base/logging.h"
@@ -525,13 +526,21 @@ void TurnServer::DestroyInternalSocket(rtc::AsyncPacketSocket* socket) {
   InternalSocketMap::iterator iter = server_sockets_.find(socket);
   if (iter != server_sockets_.end()) {
     rtc::AsyncPacketSocket* socket = iter->first;
-    // We must destroy the socket async to avoid invalidating the sigslot
-    // callback list iterator inside a sigslot callback.
-    rtc::Thread::Current()->Dispose(socket);
     server_sockets_.erase(iter);
+    // We must destroy the socket async to avoid invalidating the sigslot
+    // callback list iterator inside a sigslot callback. (In other words,
+    // deleting an object from within a callback from that object).
+    sockets_to_delete_.push_back(
+        std::unique_ptr<rtc::AsyncPacketSocket>(socket));
+    invoker_.AsyncInvoke<void>(RTC_FROM_HERE, rtc::Thread::Current(),
+                               rtc::Bind(&TurnServer::FreeSockets, this));
   }
 }
 
+void TurnServer::FreeSockets() {
+  sockets_to_delete_.clear();
+}
+
 TurnServerConnection::TurnServerConnection(const rtc::SocketAddress& src,
                                            ProtocolType proto,
                                            rtc::AsyncPacketSocket* socket)
diff --git a/webrtc/p2p/base/turnserver.h b/webrtc/p2p/base/turnserver.h
index 09b553ef17..608e0434e4 100644
--- a/webrtc/p2p/base/turnserver.h
+++ b/webrtc/p2p/base/turnserver.h
@@ -16,8 +16,10 @@
 #include <memory>
 #include <set>
 #include <string>
+#include <vector>
 
 #include "webrtc/p2p/base/portinterface.h"
+#include "webrtc/base/asyncinvoker.h"
 #include "webrtc/base/asyncpacketsocket.h"
 #include "webrtc/base/messagequeue.h"
 #include "webrtc/base/sigslot.h"
@@ -258,6 +260,9 @@ class TurnServer : public sigslot::has_slots<> {
   void OnAllocationDestroyed(TurnServerAllocation* allocation);
   void DestroyInternalSocket(rtc::AsyncPacketSocket* socket);
 
+  // Just clears |sockets_to_delete_|; called asynchronously.
+  void FreeSockets();
+
   typedef std::map<rtc::AsyncPacketSocket*,
                    ProtocolType> InternalSocketMap;
   typedef std::map<rtc::AsyncSocket*,
@@ -278,11 +283,15 @@ class TurnServer : public sigslot::has_slots<> {
 
   InternalSocketMap server_sockets_;
   ServerSocketMap server_listen_sockets_;
+  // Used when we need to delete a socket asynchronously.
+  std::vector<std::unique_ptr<rtc::AsyncPacketSocket>> sockets_to_delete_;
   std::unique_ptr<rtc::PacketSocketFactory> external_socket_factory_;
   rtc::SocketAddress external_addr_;
 
   AllocationMap allocations_;
 
+  rtc::AsyncInvoker invoker_;
+
   // For testing only. If this is non-zero, the next NONCE will be generated
   // from this value, and it will be reset to 0 after generating the NONCE.
   int64_t ts_for_next_nonce_ = 0;