Revert: Fixes leak of duplicate command line environment strings

For bot crashes on Mac

Review URL: https://codereview.chromium.org/2229973002 .

Author: zanderso

runtime/bin/eventhandler.h

@@ -132,8 +132,6 @@ class CircularLinkedList {
  public:
   CircularLinkedList() : head_(NULL) {}
 
-  typedef void (*ClearFun) (void* value);
-
   // Returns true if the list was empty.
   bool Add(T t) {
     Entry* e = new Entry(t);
@@ -153,7 +151,7 @@ class CircularLinkedList {
     }
   }
 
-  void RemoveHead(ClearFun clear = NULL) {
+  void RemoveHead() {
     ASSERT(head_ != NULL);
 
     Entry* e = head_;
@@ -164,9 +162,6 @@ class CircularLinkedList {
       e->next_->prev_ = e->prev_;
       head_ = e->next_;
     }
-    if (clear != NULL) {
-      clear(reinterpret_cast<void*>(e->t));
-    }
     delete e;
   }
 
@@ -200,9 +195,9 @@ class CircularLinkedList {
     }
   }
 
-  void RemoveAll(ClearFun clear = NULL) {
+  void RemoveAll() {
     while (HasHead()) {
-      RemoveHead(clear);
+      RemoveHead();
     }
   }
 
@@ -418,9 +413,7 @@ class DescriptorInfoMultipleMixin : public DI {
       : DI(fd), tokens_map_(&SamePortValue, kTokenCount),
         disable_tokens_(disable_tokens) {}
 
-  virtual ~DescriptorInfoMultipleMixin() {
-    RemoveAllPorts();
-  }
+  virtual ~DescriptorInfoMultipleMixin() {}
 
   virtual bool IsListeningSocket() const { return true; }
 
@@ -504,16 +497,14 @@ class DescriptorInfoMultipleMixin : public DI {
   }
 
   virtual void RemoveAllPorts() {
+    active_readers_.RemoveAll();
     for (HashMap::Entry *entry = tokens_map_.Start();
          entry != NULL;
          entry = tokens_map_.Next(entry)) {
       PortEntry* pentry = reinterpret_cast<PortEntry*>(entry->value);
-      entry->value = NULL;
-      active_readers_.Remove(pentry);
       delete pentry;
     }
     tokens_map_.Clear();
-    active_readers_.RemoveAll(DeletePortEntry);
   }
 
   virtual Dart_Port NextNotifyDartPort(intptr_t events_ready) {
@@ -594,11 +585,6 @@ class DescriptorInfoMultipleMixin : public DI {
   }
 
  private:
-  static void DeletePortEntry(void* data) {
-    PortEntry* entry = reinterpret_cast<PortEntry*>(data);
-    delete entry;
-  }
-
   // The [Dart_Port]s which are not paused (i.e. are interested in read events,
   // i.e. `mask == (1 << kInEvent)`) and we have enough tokens to communicate
   // with them.

runtime/bin/eventhandler_android.cc

@@ -118,15 +118,7 @@ EventHandlerImplementation::EventHandlerImplementation()
 }
 
 
-static void DeleteDescriptorInfo(void* info) {
-  DescriptorInfo* di = reinterpret_cast<DescriptorInfo*>(info);
-  di->Close();
-  delete di;
-}
-
-
 EventHandlerImplementation::~EventHandlerImplementation() {
-  socket_map_.Clear(DeleteDescriptorInfo);
   VOID_TEMP_FAILURE_RETRY(close(epoll_fd_));
   VOID_TEMP_FAILURE_RETRY(close(interrupt_fds_[0]));
   VOID_TEMP_FAILURE_RETRY(close(interrupt_fds_[1]));

runtime/bin/eventhandler_linux.cc

@@ -127,15 +127,7 @@ EventHandlerImplementation::EventHandlerImplementation()
 }
 
 
-static void DeleteDescriptorInfo(void* info) {
-  DescriptorInfo* di = reinterpret_cast<DescriptorInfo*>(info);
-  di->Close();
-  delete di;
-}
-
-
 EventHandlerImplementation::~EventHandlerImplementation() {
-  socket_map_.Clear(DeleteDescriptorInfo);
   VOID_TEMP_FAILURE_RETRY(close(epoll_fd_));
   VOID_TEMP_FAILURE_RETRY(close(timer_fd_));
   VOID_TEMP_FAILURE_RETRY(close(interrupt_fds_[0]));

runtime/bin/eventhandler_macos.cc

@@ -140,15 +140,7 @@ EventHandlerImplementation::EventHandlerImplementation()
 }
 
 
-static void DeleteDescriptorInfo(void* info) {
-  DescriptorInfo* di = reinterpret_cast<DescriptorInfo*>(info);
-  di->Close();
-  delete di;
-}
-
-
 EventHandlerImplementation::~EventHandlerImplementation() {
-  socket_map_.Clear(DeleteDescriptorInfo);
   VOID_TEMP_FAILURE_RETRY(close(kqueue_fd_));
   VOID_TEMP_FAILURE_RETRY(close(interrupt_fds_[0]));
   VOID_TEMP_FAILURE_RETRY(close(interrupt_fds_[1]));

runtime/bin/socket.cc

@@ -127,57 +127,42 @@ Dart_Handle ListeningSocketRegistry::CreateBindListen(Dart_Handle socket_object,
 }
 
 
-bool ListeningSocketRegistry::CloseOneSafe(OSSocket* os_socket) {
+bool ListeningSocketRegistry::CloseSafe(intptr_t socketfd) {
   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.
-  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;
-  }
-
-  ASSERT(os_socket->ref_count == 0);
-  delete os_socket;
-  return true;
-}
 
+  SocketsIterator it = sockets_by_fd_.find(socketfd);
+  if (it != sockets_by_fd_.end()) {
+    OSSocket *os_socket = it->second;
 
-void ListeningSocketRegistry::CloseAllSafe() {
-  MutexLocker ml(mutex_);
-  SocketsIterator it = sockets_by_fd_.begin();
-  while (it != sockets_by_fd_.end()) {
-    CloseOneSafe(it->second);
-    it++;
-  }
-}
+    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;
+      }
 
-bool ListeningSocketRegistry::CloseSafe(intptr_t socketfd) {
-  ASSERT(!mutex_->TryLock());
-  SocketsIterator it = sockets_by_fd_.find(socketfd);
-  if (it != sockets_by_fd_.end()) {
-    return CloseOneSafe(it->second);
+      delete os_socket;
+      return true;
+    }
+    return false;
   } else {
     // It should be impossible for the event handler to close something that
     // hasn't been created before.

runtime/bin/socket.h

@@ -402,7 +402,6 @@ class ListeningSocketRegistry {
   ListeningSocketRegistry() : mutex_(new Mutex()) {}
 
   ~ListeningSocketRegistry() {
-    CloseAllSafe();
     delete mutex_;
     mutex_ = NULL;
   }
@@ -438,10 +437,6 @@ class ListeningSocketRegistry {
     return NULL;
   }
 
-  bool CloseOneSafe(OSSocket* os_socket);
-  void CloseAllSafe();
-
-  // TODO(zra): Replace std::map with the HashMap in platform/hashmap.h.
   std::map<intptr_t, OSSocket*> sockets_by_port_;
   std::map<intptr_t, OSSocket*> sockets_by_fd_;
   Mutex *mutex_;

runtime/platform/hashmap.cc

@@ -15,7 +15,7 @@ HashMap::HashMap(MatchFun match, uint32_t initial_capacity) {
 
 
 HashMap::~HashMap() {
-  delete[] map_;
+  free(map_);
 }
 
 
@@ -106,19 +106,14 @@ void HashMap::Remove(void* key, uint32_t hash) {
 
   // Clear the candidate which will not break searching the hash table.
   candidate->key = NULL;
-  candidate->value = NULL;
   occupancy_--;
 }
 
 
-void HashMap::Clear(ClearFun clear) {
+void HashMap::Clear() {
   // Mark all entries as empty.
   const Entry* end = map_end();
   for (Entry* p = map_; p < end; p++) {
-    if ((clear != NULL) && (p->key != NULL)) {
-      clear(p->value);
-    }
-    p->value = NULL;
     p->key = NULL;
   }
   occupancy_ = 0;
@@ -164,14 +159,14 @@ HashMap::Entry* HashMap::Probe(void* key, uint32_t hash) {
 
 void HashMap::Initialize(uint32_t capacity) {
   ASSERT(dart::Utils::IsPowerOfTwo(capacity));
-  map_ = new Entry[capacity];
+  map_ = reinterpret_cast<Entry*>(malloc(capacity * sizeof(Entry)));
   if (map_ == NULL) {
     // TODO(sgjesse): Handle out of memory.
     FATAL("Cannot allocate memory for hashmap");
     return;
   }
   capacity_ = capacity;
-  occupancy_ = 0;
+  Clear();
 }
 
 
@@ -191,7 +186,7 @@ void HashMap::Resize() {
   }
 
   // Delete old map.
-  delete[] map;
+  free(map);
 }
 
 }  // namespace dart

runtime/platform/hashmap.h

@@ -13,8 +13,6 @@ class HashMap {
  public:
   typedef bool (*MatchFun) (void* key1, void* key2);
 
-  typedef void (*ClearFun) (void* value);
-
   static bool SamePointerValue(void* key1, void* key2) {
     return key1 == key2;
   }
@@ -50,7 +48,6 @@ class HashMap {
   // Some clients may not need to use the value slot
   // (e.g. implementers of sets, where the key is the value).
   struct Entry {
-    Entry() : key(NULL), value(NULL), hash(0) {}
     void* key;
     void* value;
     uint32_t hash;  // The full hash value for key.
@@ -66,12 +63,8 @@ class HashMap {
   // Removes the entry with matching key.
   void Remove(void* key, uint32_t hash);
 
-  // Empties the hash map (occupancy() == 0), and calls the function 'clear' on
-  // each of the values if given.
-  void Clear(ClearFun clear = NULL);
-
-  // The number of entries stored in the table.
-  intptr_t size() const { return occupancy_; }
+  // Empties the hash map (occupancy() == 0).
+  void Clear();
 
   // The capacity of the table. The implementation
   // makes sure that occupancy is at most 80% of