commit 824f58621315a8b39193e06f960d8ff15f84fc13
author deadbeef <deadbeef@webrtc.org> Wed Aug 24 22:06:53 2016
committer Commit bot <commit-bot@chromium.org> Wed Aug 24 22:06:58 2016
tree fc498efa9175d0bf75f9d47247cd223154ee7beb
parent 1d7a637340ea097ea55061af7b566ceccc2678c8

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

diff --git a/webrtc/p2p/base/turnserver.cc b/webrtc/p2p/base/turnserver.cc
index 7dd9e19..3a57467 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 @@
   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)

webrtc/p2p/base/turnserver.h

diff --git a/webrtc/p2p/base/turnserver.h b/webrtc/p2p/base/turnserver.h
index 09b553e..608e043 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 @@
   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 @@
 
   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;