VM: Fix memory leak during shutdown

runtime/bin/socket.cc

Index: runtime/bin/socket.cc
diff --git a/runtime/bin/socket.cc b/runtime/bin/socket.cc
index d5205723cc8c52d9831b09efafc93658ce30c9ad..2077cc71f7d6e4bfa68377880e3146201dfb0d8c 100644
--- a/runtime/bin/socket.cc
+++ b/runtime/bin/socket.cc
@@ -90,9 +90,17 @@ void ListeningSocketRegistry::InsertByFd(intptr_t fd, OSSocket* socket) {
 }
 
 
-void ListeningSocketRegistry::RemoveByFd(intptr_t fd) {
-  sockets_by_fd_.Remove(GetHashmapKeyFromIntptr(fd),
-                        GetHashmapHashFromIntptr(fd));
+void ListeningSocketRegistry::RemoveByFd(intptr_t fd,
+                                         HashMap::Entry** map_cursor) {
+  void* key = GetHashmapKeyFromIntptr(fd);
+  uint32_t hash_code = GetHashmapHashFromIntptr(fd);
+  if (map_cursor != NULL) {
+    HashMap::Entry* entry = sockets_by_fd_.Lookup(key, hash_code, false);
+    ASSERT(entry != NULL);
+    *map_cursor = sockets_by_fd_.Remove(entry);
+  } else {
+    sockets_by_fd_.Remove(key, hash_code);
+  }
 }
 
 
@@ -196,7 +204,8 @@ Dart_Handle ListeningSocketRegistry::CreateBindListen(Dart_Handle socket_object,
 }
 
 
-bool ListeningSocketRegistry::CloseOneSafe(OSSocket* os_socket) {
+bool ListeningSocketRegistry::CloseOneSafe(OSSocket* os_socket,

[Vyacheslav Egorov (Google), 2016/11/30 12:41:56]

I suggest a much simple approach: pass a boolean here that says whether to
remove it from the map or not. 

Then CloseAll can just call this function with this boolean set to false and
then clear the whole hash map.

+                                           HashMap::Entry** fd_map_cursor) {
   ASSERT(!mutex_->TryLock());
   ASSERT(os_socket != NULL);
   ASSERT(os_socket->ref_count > 0);
@@ -205,7 +214,7 @@ bool ListeningSocketRegistry::CloseOneSafe(OSSocket* os_socket) {
     return false;
   }
   // We free the OS socket by removing it from two datastructures.
-  RemoveByFd(os_socket->socketfd);
+  RemoveByFd(os_socket->socketfd, fd_map_cursor);
 
   OSSocket* prev = NULL;
   OSSocket* current = LookupByPort(os_socket->port);
@@ -234,9 +243,14 @@ bool ListeningSocketRegistry::CloseOneSafe(OSSocket* os_socket) {
 
 void ListeningSocketRegistry::CloseAllSafe() {
   MutexLocker ml(mutex_);
-  for (HashMap::Entry* p = sockets_by_fd_.Start(); p != NULL;
-       p = sockets_by_fd_.Next(p)) {
-    CloseOneSafe(reinterpret_cast<OSSocket*>(p->value));
+
+  HashMap::Entry* fd_map_cursor = sockets_by_fd_.Start();
+  while (fd_map_cursor != NULL) {
+    OSSocket* os_socket = reinterpret_cast<OSSocket*>(fd_map_cursor->value);
+    bool removed = CloseOneSafe(os_socket, &fd_map_cursor);
+    if (!removed) {
+      fd_map_cursor = sockets_by_fd_.Next(fd_map_cursor);
+    }
   }
 }
 
@@ -245,7 +259,7 @@ bool ListeningSocketRegistry::CloseSafe(intptr_t socketfd) {
   ASSERT(!mutex_->TryLock());
   OSSocket* os_socket = LookupByFd(socketfd);
   if (os_socket != NULL) {
-    return CloseOneSafe(os_socket);
+    return CloseOneSafe(os_socket, NULL);
   } else {
     // It should be impossible for the event handler to close something that
     // hasn't been created before.