collect phantom handle data before it gets overwritten

src/global-handles.cc

 // Copyright 2009 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/v8.h"
 
 #include "src/api.h"
 #include "src/global-handles.h"
 
 #include "src/vm-state-inl.h"
@@ skipping 151 matching lines @@
 
   bool IsNearDeath() const {
     // Check for PENDING to ensure correct answer when processing callbacks.
     return state() == PENDING || state() == NEAR_DEATH;
   }
 
   bool IsWeak() const { return state() == WEAK; }
 
   bool IsInUse() const { return state() != FREE; }
 
-  bool IsRetainer() const { return state() != FREE; }
+  bool IsRetainer() const {
+    return state() != FREE &&
+           !(state() == NEAR_DEATH && weakness_type() != NORMAL_WEAK);

[Hannes Payer (out of office), 2015/04/13 11:50:45]

please simplify this statement

[dcarney, 2015/04/13 12:27:05]

this was the clearest version i could think of. what did you have in mind?

+  }
 
   bool IsStrongRetainer() const { return state() == NORMAL; }
 
   bool IsWeakRetainer() const {
-    return state() == WEAK || state() == PENDING || state() == NEAR_DEATH;
+    return state() == WEAK || state() == PENDING ||
+           (state() == NEAR_DEATH && weakness_type() == NORMAL_WEAK);
   }
 
   void MarkPending() {
     DCHECK(state() == WEAK);
     set_state(PENDING);
   }
 
   // Independent flag accessors.
   void MarkIndependent() {
     DCHECK(IsInUse());
@@ skipping 65 matching lines @@
     DCHECK(IsInUse());
     void* p = parameter();
     set_state(NORMAL);
     set_parameter(NULL);
     return p;
   }
 
   void CollectPhantomCallbackData(
       Isolate* isolate,
       List<PendingPhantomCallback>* pending_phantom_callbacks) {
-    if (state() != PENDING) return;
-    if (weak_callback_ != NULL) {
-      if (weakness_type() == NORMAL_WEAK) return;
+    DCHECK(weakness_type() == PHANTOM_WEAK ||
+           weakness_type() == PHANTOM_WEAK_2_INTERNAL_FIELDS);
+    DCHECK(state() == PENDING);
 
-      DCHECK(weakness_type() == PHANTOM_WEAK ||
-             weakness_type() == PHANTOM_WEAK_2_INTERNAL_FIELDS);
+    void* internal_fields[v8::kInternalFieldsInWeakCallback] = {nullptr,
+                                                                nullptr};
+    if (weakness_type() != PHANTOM_WEAK && object()->IsJSObject()) {
+      auto jsobject = JSObject::cast(object());
+      int field_count = jsobject->GetInternalFieldCount();
+      for (int i = 0; i < v8::kInternalFieldsInWeakCallback; ++i) {
+        if (field_count == i) break;
+        auto field = jsobject->GetInternalField(i);
+        if (field->IsSmi()) internal_fields[i] = field;
+      }
+    }
 
-      void* internal_fields[v8::kInternalFieldsInWeakCallback] = {nullptr,
-                                                                  nullptr};
-      if (weakness_type() != PHANTOM_WEAK && object()->IsJSObject()) {
-        auto jsobject = JSObject::cast(object());
-        int field_count = jsobject->GetInternalFieldCount();
-        for (int i = 0; i < v8::kInternalFieldsInWeakCallback; ++i) {
-          if (field_count == i) break;
-          auto field = jsobject->GetInternalField(i);
-          if (field->IsSmi()) internal_fields[i] = field;
-        }
-      }
+    // Zap with something dangerous.
+    *location() = reinterpret_cast<Object*>(0x6057ca11);
 
-      // Zap with something dangerous.
-      *location() = reinterpret_cast<Object*>(0x6057ca11);
-
-      typedef v8::WeakCallbackInfo<void> Data;
-      auto callback = reinterpret_cast<Data::Callback>(weak_callback_);
-      pending_phantom_callbacks->Add(
-          PendingPhantomCallback(this, callback, parameter(), internal_fields));
-      DCHECK(IsInUse());
-      set_state(NEAR_DEATH);
-    }
+    typedef v8::WeakCallbackInfo<void> Data;
+    auto callback = reinterpret_cast<Data::Callback>(weak_callback_);
+    pending_phantom_callbacks->Add(
+        PendingPhantomCallback(this, callback, parameter(), internal_fields));
+    DCHECK(IsInUse());
+    set_state(NEAR_DEATH);
   }
 
   bool PostGarbageCollectionProcessing(Isolate* isolate) {
     // Handles only weak handles (not phantom) that are dying.
     if (state() != Node::PENDING) return false;
     if (weak_callback_ == NULL) {
       Release();
       return false;
     }
     set_state(NEAR_DEATH);
@@ skipping 253 matching lines @@
 typedef v8::WeakCallbackInfo<void>::Callback GenericCallback;
 
 
 void GlobalHandles::MakeWeak(Object** location, void* parameter,
                              GenericCallback phantom_callback,
                              v8::WeakCallbackType type) {
   Node::FromLocation(location)->MakeWeak(parameter, phantom_callback, type);
 }
 
 
-void GlobalHandles::CollectAllPhantomCallbackData() {
-  for (NodeIterator it(this); !it.done(); it.Advance()) {
-    Node* node = it.node();
-    node->CollectPhantomCallbackData(isolate(), &pending_phantom_callbacks_);
-  }
-}
-
-
-void GlobalHandles::CollectYoungPhantomCallbackData() {
-  for (int i = 0; i < new_space_nodes_.length(); ++i) {
-    Node* node = new_space_nodes_[i];
-    DCHECK(node->is_in_new_space_list());
-    node->CollectPhantomCallbackData(isolate(), &pending_phantom_callbacks_);
-  }
-}
-
-
 void* GlobalHandles::ClearWeakness(Object** location) {
   return Node::FromLocation(location)->ClearWeakness();
 }
 
 
 void GlobalHandles::MarkIndependent(Object** location) {
   Node::FromLocation(location)->MarkIndependent();
 }
 
 
@@ skipping 17 matching lines @@
 }
 
 
 void GlobalHandles::IterateWeakRoots(ObjectVisitor* v) {
   for (NodeIterator it(this); !it.done(); it.Advance()) {
     Node* node = it.node();
     if (node->IsWeakRetainer()) {
       // Weakness type can be normal or phantom, with or without internal
       // fields).  For normal weakness we mark through the handle so that the
       // object and things reachable from it are available to the callback.
-      //
-      // In the case of phantom with no internal fields, we can zap the object
-      // handle now and we won't need it, so we don't need to mark through it.
-      // In the internal fields case we will need the internal
-      // fields, so we can't zap the handle.
       if (node->state() == Node::PENDING) {
-        if (node->weakness_type() == PHANTOM_WEAK) {
-          *(node->location()) = Smi::FromInt(0);
-        } else if (node->weakness_type() == NORMAL_WEAK) {
+        if (node->weakness_type() == NORMAL_WEAK) {
           v->VisitPointer(node->location());
         } else {
-          DCHECK(node->weakness_type() == PHANTOM_WEAK_2_INTERNAL_FIELDS);
+          node->CollectPhantomCallbackData(isolate(),
+                                           &pending_phantom_callbacks_);
         }
       } else {
         // Node is not pending, so that means the object survived.  We still
         // need to visit the pointer in case the object moved, eg. because of
         // compaction.
         v->VisitPointer(node->location());
       }
     }
   }
 }
@@ skipping 32 matching lines @@
   }
 }
 
 
 void GlobalHandles::IterateNewSpaceWeakIndependentRoots(ObjectVisitor* v) {
   for (int i = 0; i < new_space_nodes_.length(); ++i) {
     Node* node = new_space_nodes_[i];
     DCHECK(node->is_in_new_space_list());
     if ((node->is_independent() || node->is_partially_dependent()) &&
         node->IsWeakRetainer()) {
-      if (node->weakness_type() == PHANTOM_WEAK) {
-        *(node->location()) = Smi::FromInt(0);
-      } else if (node->weakness_type() == NORMAL_WEAK) {
+      if (node->weakness_type() == NORMAL_WEAK) {
         v->VisitPointer(node->location());
-      } else {
-        DCHECK(node->weakness_type() == PHANTOM_WEAK_2_INTERNAL_FIELDS);
-        // For this case we only need to trace if it's alive: The tracing of
-        // something that is already alive is just to get the pointer updated
-        // to the new location of the object).
-        DCHECK(node->state() != Node::NEAR_DEATH);
-        if (node->state() != Node::PENDING) {
-          v->VisitPointer(node->location());
-        }
+      } else if (node->state() == Node::PENDING) {
+        node->CollectPhantomCallbackData(isolate(),
+                                         &pending_phantom_callbacks_);
       }
     }
   }
 }
 
 
 bool GlobalHandles::IterateObjectGroups(ObjectVisitor* v,
                                         WeakSlotCallbackWithHeap can_skip) {
   ComputeObjectGroupsAndImplicitReferences();
   int last = 0;
@@ skipping 553 matching lines @@
   DCHECK_EQ(isolate->heap()->the_hole_value(), blocks_[block][offset]);
   blocks_[block][offset] = object;
   if (isolate->heap()->InNewSpace(object)) {
     new_space_indices_.Add(size_);
   }
   *index = size_++;
 }
 
 
 } }  // namespace v8::internal