VisitPersistentHandle should not call visitDOMWrapper

Source/bindings/core/v8/V8GCController.cpp

Index: Source/bindings/core/v8/V8GCController.cpp
diff --git a/Source/bindings/core/v8/V8GCController.cpp b/Source/bindings/core/v8/V8GCController.cpp
index 3c496ae2a398335ee891c1ab67ba5a5fc48d13cd..b11aadd5fef77481cefc90201e637f4d6595b92f 100644
--- a/Source/bindings/core/v8/V8GCController.cpp
+++ b/Source/bindings/core/v8/V8GCController.cpp
@@ -255,15 +255,16 @@ public:
         if (classId != WrapperTypeInfo::NodeClassId && classId != WrapperTypeInfo::ObjectClassId)
             return;
 
-        v8::Local<v8::Object> wrapper = v8::Local<v8::Object>::New(m_isolate, v8::Persistent<v8::Object>::Cast(*value));
-        ASSERT(V8DOMWrapper::hasInternalFieldsSet(wrapper));
-
         if (value->IsIndependent())
             return;
 
-        const WrapperTypeInfo* type = toWrapperTypeInfo(wrapper);
+        v8::Persistent<v8::Object>& persistentWrapper = v8::Persistent<v8::Object>::Cast(*value);
+        v8::Local<v8::Object> wrapper = v8::Local<v8::Object>::New(m_isolate, persistentWrapper);
+        ASSERT(V8DOMWrapper::hasInternalFieldsSet(wrapper));
 
-        ActiveDOMObject* activeDOMObject = type->toActiveDOMObject(wrapper);
+        const WrapperTypeInfo* wrapperTypeInfo = toWrapperTypeInfo(wrapper);
+
+        ActiveDOMObject* activeDOMObject = wrapperTypeInfo->toActiveDOMObject(wrapper);
         if (activeDOMObject && activeDOMObject->hasPendingActivity()) {
             m_isolate->SetObjectGroupId(*value, liveRootId());
             ++m_domObjectsWithPendingActivity;
@@ -273,13 +274,13 @@ public:
             ASSERT(V8Node::hasInstance(wrapper, m_isolate));
             Node* node = V8Node::toImpl(wrapper);
             if (node->hasEventListeners())
-                addReferencesForNodeWithEventListeners(m_isolate, node, v8::Persistent<v8::Object>::Cast(*value));
+                addReferencesForNodeWithEventListeners(m_isolate, node, persistentWrapper);
             Node* root = V8GCController::opaqueRootForGC(m_isolate, node);
             m_isolate->SetObjectGroupId(*value, v8::UniqueId(reinterpret_cast<intptr_t>(root)));
             if (m_constructRetainedObjectInfos)
                 m_groupsWhichNeedRetainerInfo.append(root);
         } else if (classId == WrapperTypeInfo::ObjectClassId) {
-            type->visitDOMWrapper(m_isolate, toScriptWrappable(wrapper), v8::Persistent<v8::Object>::Cast(*value));
+            m_wrappersToBeVisited.append(value);
         } else {
             ASSERT_NOT_REACHED();
         }
@@ -287,6 +288,14 @@ public:
 
     void notifyFinished()
     {
+        for (auto& value : m_wrappersToBeVisited) {
+            ASSERT(!value->IsIndependent());

[haraken, 2015/06/20 09:29:49]

This CL is not correct. The access to |value| causes stack-use-after-free,
because v8::Persistent<v8::Value>* is pointing to an on-stack pointer. How can I
avoid the issue?

One way to avoid this would be to copy the persistent handles into
m_wrappersToBeVisited. However persistent handles are not copyable...

Jochen: Any idea?

[jochen (gone - plz use gerrit), 2015/06/22 08:45:49]

what about unwrapping the handle and collecting ScriptWrappable* ?

+            v8::Persistent<v8::Object>& persistentWrapper = v8::Persistent<v8::Object>::Cast(*value);
+            v8::Local<v8::Object> wrapper = v8::Local<v8::Object>::New(m_isolate, persistentWrapper);
+            const WrapperTypeInfo* wrapperTypeInfo = toWrapperTypeInfo(wrapper);
+            wrapperTypeInfo->visitDOMWrapper(m_isolate, toScriptWrappable(wrapper), persistentWrapper);

[jochen (gone - plz use gerrit), 2015/06/22 08:45:49]

can this create new Persistent<> handles that need to be in the right object
group?

+        }
+
         if (!m_constructRetainedObjectInfos)
             return;
         std::sort(m_groupsWhichNeedRetainerInfo.begin(), m_groupsWhichNeedRetainerInfo.end());
@@ -319,6 +328,7 @@ private:
 
     v8::Isolate* m_isolate;
     WillBePersistentHeapVector<RawPtrWillBeMember<Node>> m_groupsWhichNeedRetainerInfo;
+    Vector<v8::Persistent<v8::Value>*> m_wrappersToBeVisited;
     int m_domObjectsWithPendingActivity;
     bool m_liveRootGroupIdSet;
     bool m_constructRetainedObjectInfos;