VM: Fix memory leak during shutdown

runtime/bin/socket.cc

 // Copyright (c) 2013, the Dart project authors.  Please see the AUTHORS file
 // for details. All rights reserved. Use of this source code is governed by a
 // BSD-style license that can be found in the LICENSE file.
 
 #if !defined(DART_IO_DISABLED)
 
 #include "bin/socket.h"
 
 #include "bin/dartutils.h"
 #include "bin/io_buffer.h"
@@ skipping 72 matching lines @@
 
 
 void ListeningSocketRegistry::InsertByFd(intptr_t fd, OSSocket* socket) {
   HashMap::Entry* entry = sockets_by_fd_.Lookup(
       GetHashmapKeyFromIntptr(fd), GetHashmapHashFromIntptr(fd), true);
   ASSERT(entry != NULL);
   entry->value = reinterpret_cast<void*>(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);
+  }
 }
 
 
 Dart_Handle ListeningSocketRegistry::CreateBindListen(Dart_Handle socket_object,
                                                       RawAddr addr,
                                                       intptr_t backlog,
                                                       bool v6_only,
                                                       bool shared) {
   MutexLocker ml(mutex_);
 
@@ skipping 83 matching lines @@
   InsertByPort(allocated_port, os_socket);
   InsertByFd(socketfd, os_socket);
 
   // We set as a side-effect the port on the dart socket_object.
   Socket::SetSocketIdNativeField(socket_object, socketfd);
 
   return Dart_True();
 }
 
 
-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);
   os_socket->ref_count--;
   if (os_socket->ref_count > 0) {
     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);
   while (current != os_socket) {
     ASSERT(current != NULL);
     prev = current;
     current = current->next;
   }
 
   if ((prev == NULL) && (current->next == NULL)) {
     // Remove last element from the list.
     RemoveByPort(os_socket->port);
   } else if (prev == NULL) {
     // Remove first element of the list.
     InsertByPort(os_socket->port, current->next);
   } else {
     // Remove element from the list which is not the first one.
     prev->next = os_socket->next;
   }
 
   ASSERT(os_socket->ref_count == 0);
   delete os_socket;
   return true;
 }
 
 
 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);
+    }
   }
 }
 
 
 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.
     UNREACHABLE();
     return false;
   }
 }
 
 
 void FUNCTION_NAME(InternetAddress_Parse)(Dart_NativeArguments args) {
@@ skipping 682 matching lines @@
   if (Dart_IsError(err)) {
     Dart_PropagateError(err);
   }
   return socket;
 }
 
 }  // namespace bin
 }  // namespace dart
 
 #endif  // !defined(DART_IO_DISABLED)