[wasm] Managed<T> ensures T's lifetime does not leak past Isolate's

src/isolate.cc

 // Copyright 2012 the V8 project authors. All rights reserved.
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
 #include "src/isolate.h"
 
 #include <stdlib.h>
 
 #include <fstream>  // NOLINT(readability/streams)
 #include <sstream>
@@ skipping 2004 matching lines @@
 }
 
 
 Isolate::ThreadDataTable::~ThreadDataTable() {
   // TODO(svenpanne) The assertion below would fire if an embedder does not
   // cleanly dispose all Isolates before disposing v8, so we are conservative
   // and leave it out for now.
   // DCHECK_NULL(list_);
 }
 
+void Isolate::ManagedLifeline::CallDeleter() {
+  DCHECK_NOT_NULL(value_);
+  // deleter_ may be null in test scenarios.
+  if (deleter_ != nullptr) {
+    deleter_(value_);
+  }
+}
+
+Isolate::ManagedLifeline::~ManagedLifeline() {
+  // when we delete the Isolate, the destructor will be called for the
+  // managed_list_root_;
+  if (prev_ == nullptr) return;
+  DCHECK_NOT_NULL(value_);
+
+  prev_->next_ = next_;
+  if (next_ != nullptr) next_->prev_ = prev_;
+  // Used in test - where the memory is allocated by the test.
+  CallDeleter();
+}
+
+void Isolate::ManagedLifeline::Clear() {

[titzer, 2017/02/08 23:32:47]

Something seems weird with this. Why are we using a both a destructor and an
explicit clear? Why not just remove the destructor and put it all into clear?

[Mircea Trofin, 2017/02/08 23:52:35]

This clears the whole list. The destructor deletes a node and may be invoked
either by finalizer or when the list is cleared at Teardown

[titzer, 2017/02/09 04:25:18]

Sure, but I don't think it's necessary to have a destructor do either of those
jobs. You can just have a struct that forms the linked list. In the call site in
the code below, you can just iterate over the whole list and delete() both the
managed object and the link in the list.

+  // Should be called from the head of the dlinked list. We don't have a reason
+  // to do otherwise.
+  DCHECK_NULL(prev_);
+
+  for (Isolate::ManagedLifeline* current = next_; current != nullptr;) {
+    Isolate::ManagedLifeline* next = current->next_;
+    // This also relinks the list, which is redundant in this case,
+    // but that's at most 2 assignments  per node.
+    delete current;
+    current = next;
+  }
+}
+
+Isolate::ManagedLifeline* Isolate::AddManagedObject(
+    void* value, Isolate::ManagedLifeline::Deleter deleter) {
+  Isolate::ManagedLifeline* ret = new Isolate::ManagedLifeline();
+  ret->value_ = value;
+  ret->deleter_ = deleter;
+  // Insert at head. We keep the head alive for the lifetime of the Isolate
+  // because otherwise we can't
+  // reset the head, should we delete it before the isolate expires
+  Isolate::ManagedLifeline* next = managed_lifelines_root_.next_;
+  managed_lifelines_root_.next_ = ret;
+  ret->prev_ = &managed_lifelines_root_;
+  ret->next_ = next;
+  if (next != nullptr) next->prev_ = ret;
+  return ret;
+}
 
 Isolate::PerIsolateThreadData::~PerIsolateThreadData() {
 #if defined(USE_SIMULATOR)
   delete simulator_;
 #endif
 }
 
 
 Isolate::PerIsolateThreadData*
     Isolate::ThreadDataTable::Lookup(Isolate* isolate,
@@ skipping 349 matching lines @@
 
   delete cpu_profiler_;
   cpu_profiler_ = NULL;
 
   code_event_dispatcher_.reset();
 
   delete root_index_map_;
   root_index_map_ = NULL;
 
   ClearSerializerData();
+  managed_lifelines_root_.Clear();
 }
 
 
 void Isolate::SetIsolateThreadLocals(Isolate* isolate,
                                      PerIsolateThreadData* data) {
   base::Thread::SetThreadLocal(isolate_key_, isolate);
   base::Thread::SetThreadLocal(per_isolate_thread_data_key_, data);
 }
 
 
@@ skipping 1244 matching lines @@
   // Then check whether this scope intercepts.
   if ((flag & intercept_mask_)) {
     intercepted_flags_ |= flag;
     return true;
   }
   return false;
 }
 
 }  // namespace internal
 }  // namespace v8