two pass phantom collection

src/global-handles.cc

Index: src/global-handles.cc
diff --git a/src/global-handles.cc b/src/global-handles.cc
index 0554c53bb5deca3d5458199a29793ced681fdb1b..52c8c9ea20a92b7eff9fd5f3a5dd001b9e187b02 100644
--- a/src/global-handles.cc
+++ b/src/global-handles.cc
@@ -264,8 +264,6 @@ class GlobalHandles::Node {
     if (weak_callback_ != NULL) {
       if (weakness_type() == NORMAL_WEAK) return;
 
-      v8::Isolate* api_isolate = reinterpret_cast<v8::Isolate*>(isolate);
-
       DCHECK(weakness_type() == PHANTOM_WEAK ||
              weakness_type() == PHANTOM_WEAK_2_INTERNAL_FIELDS);
 
@@ -286,16 +284,15 @@ class GlobalHandles::Node {
         }
       }
 
-      // Zap with harmless value.
-      *location() = Smi::FromInt(0);
+      // Zap with something dangerous.
+      *location() = reinterpret_cast<Object*>(0x1);

[Erik Corry, 2015/03/26 10:07:11]

I would prefer some zap value that you can grep for when you find it in the
debugger.  How about something phantom-themed:

0x6057ca11 // Gost-call

[dcarney, 2015/03/26 10:26:56]

Done.

       typedef v8::WeakCallbackInfo<void> Data;
 
-      Data data(api_isolate, parameter(), internal_field0, internal_field1);
       Data::Callback callback =
           reinterpret_cast<Data::Callback>(weak_callback_);
 
-      pending_phantom_callbacks->Add(
-          PendingPhantomCallback(this, data, callback));
+      pending_phantom_callbacks->Add(PendingPhantomCallback(
+          this, callback, parameter(), internal_field0, internal_field1));
       DCHECK(IsInUse());
       set_state(NEAR_DEATH);
     }
@@ -838,17 +835,50 @@ void GlobalHandles::UpdateListOfNewSpaceNodes() {
 
 int GlobalHandles::DispatchPendingPhantomCallbacks() {
   int freed_nodes = 0;
+  {
+    // The initial pass callbacks must simply clear the nodes.
+    for (auto i = pending_phantom_callbacks_.begin();
+         i != pending_phantom_callbacks_.end(); ++i) {
+      auto callback = i;
+      // Skip callbacks that have already been processed once.
+      if (callback->node() == nullptr) continue;
+      callback->Invoke(isolate());
+      freed_nodes++;
+    }
+  }
+  // The second pass empties the list.
   while (pending_phantom_callbacks_.length() != 0) {
-    PendingPhantomCallback callback = pending_phantom_callbacks_.RemoveLast();
-    DCHECK(callback.node()->IsInUse());
-    callback.invoke();
-    DCHECK(!callback.node()->IsInUse());
-    freed_nodes++;
+    auto callback = pending_phantom_callbacks_.RemoveLast();
+    DCHECK(callback.node() == nullptr);
+    // No second pass callback required.
+    if (callback.callback() == nullptr) continue;
+    // Fire second pass callback.
+    callback.Invoke(isolate());
   }
   return freed_nodes;
 }
 
 
+void GlobalHandles::PendingPhantomCallback::Invoke(Isolate* isolate) {
+  Data::Callback* callback_addr = nullptr;
+  if (node_ != nullptr) {
+    // Initialize for first pass callback.
+    DCHECK(node_->state() == Node::NEAR_DEATH);
+    callback_addr = &callback_;
+  }
+  Data data(reinterpret_cast<v8::Isolate*>(isolate), parameter_,
+            internal_fields_, callback_addr);
+  Data::Callback callback = callback_;
+  callback_ = nullptr;
+  callback(data);
+  if (node_ != nullptr) {
+    // Transition to second pass state.
+    DCHECK(node_->state() == Node::FREE);
+    node_ = nullptr;
+  }
+}
+
+
 int GlobalHandles::PostGarbageCollectionProcessing(GarbageCollector collector) {
   // Process weak global handle callbacks. This must be done after the
   // GC is completely done, because the callbacks may invoke arbitrary
@@ -879,14 +909,6 @@ int GlobalHandles::PostGarbageCollectionProcessing(GarbageCollector collector) {
 }
 
 
-void GlobalHandles::PendingPhantomCallback::invoke() {
-  if (node_->state() == Node::FREE) return;
-  DCHECK(node_->state() == Node::NEAR_DEATH);
-  callback_(data_);
-  if (node_->state() != Node::FREE) node_->Release();
-}
-
-
 void GlobalHandles::IterateStrongRoots(ObjectVisitor* v) {
   for (NodeIterator it(this); !it.done(); it.Advance()) {
     if (it.node()->IsStrongRetainer()) {