Fixes memory leaks in the eventhandler

runtime/bin/socket.cc

Index: runtime/bin/socket.cc
diff --git a/runtime/bin/socket.cc b/runtime/bin/socket.cc
index 745306d0e53aef80dffdde95f79f249bda1bfe4d..d5ebd7da863dacbcd116e9c2190e6afb04b64f3e 100644
--- a/runtime/bin/socket.cc
+++ b/runtime/bin/socket.cc
@@ -127,42 +127,56 @@ Dart_Handle ListeningSocketRegistry::CreateBindListen(Dart_Handle socket_object,
 }
 
 
+bool ListeningSocketRegistry::CloseOneSafe(OSSocket* os_socket) {

[siva, 2016/08/09 21:41:43]

ASSERT(mutex_ is owned by current thread);
ASSERT(os_socket != NULL);

[zra, 2016/08/09 22:19:27]

For the mutexes under bin there is no "is owned by current thread", but TryLock
will apparently fail if the lock is already held.

+  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.
+  sockets_by_fd_.erase(os_socket->socketfd);
+
+  OSSocket *prev = NULL;
+  OSSocket *current = sockets_by_port_[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.
+    sockets_by_port_.erase(os_socket->port);
+  } else if (prev == NULL) {
+    // Remove first element of the list.
+    sockets_by_port_[os_socket->port] = current->next;
+  } else {
+    // Remove element from the list which is not the first one.
+    prev->next = os_socket->next;
+  }
+

[siva, 2016/08/09 21:41:43]

ASSERT(os_socket->ref_count == 0);

[zra, 2016/08/09 22:19:27]

Done.

+  delete os_socket;
+  return true;
+}
+
+
+void ListeningSocketRegistry::CloseAllSafe() {
+  MutexLocker ml(mutex_);
+  SocketsIterator it = sockets_by_fd_.begin();
+  while (it != sockets_by_fd_.end()) {
+    CloseOneSafe(it->second);
+    it++;
+  }
+}
+
+
 bool ListeningSocketRegistry::CloseSafe(intptr_t socketfd) {
   ASSERT(!mutex_->TryLock());
 
   SocketsIterator it = sockets_by_fd_.find(socketfd);
   if (it != sockets_by_fd_.end()) {
     OSSocket *os_socket = it->second;
-
-    ASSERT(os_socket->ref_count > 0);
-    os_socket->ref_count--;
-    if (os_socket->ref_count == 0) {
-      // We free the OS socket by removing it from two datastructures.
-      sockets_by_fd_.erase(socketfd);
-
-      OSSocket *prev = NULL;
-      OSSocket *current = sockets_by_port_[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.
-        sockets_by_port_.erase(os_socket->port);
-      } else if (prev == NULL) {
-        // Remove first element of the list.
-        sockets_by_port_[os_socket->port] = current->next;
-      } else {
-        // Remove element from the list which is not the first one.
-        prev->next = os_socket->next;
-      }
-
-      delete os_socket;
-      return true;
-    }
-    return false;
+    return CloseOneSafe(os_socket);

[siva, 2016/08/09 21:41:43]

why not
CloseOneSafe(it->second);

[zra, 2016/08/09 22:19:27]

Done.

   } else {
     // It should be impossible for the event handler to close something that
     // hasn't been created before.