Do not allow GlobalHandles::Create to reuse destoryed nodes (ones from free list)

while performing GlobalHandles::PostGarbageCollectionProcessing as those might be already deleted (in C++ sense).

Review URL: http://codereview.chromium.org/173060

git-svn-id: http://v8.googlecode.com/svn/branches/bleeding_edge@2724 ce2b1a6d-e550-0410-aec6-3dcde31c8c00

diff --git a/src/global-handles.cc b/src/global-handles.cc
index ed4e262e628856e5a5d16ffd47d0431afaac267c..f868974b077df12c2312508aa0b8273a28c0dd98 100644 (file)
--- a/src/global-handles.cc
+++ b/src/global-handles.cc
@@ -156,6 +156,10 @@ class GlobalHandles::Node : public Malloced {
     if (func != NULL) {
       v8::Persistent<v8::Object> object = ToApi<v8::Object>(handle());
       {
+        // Forbid reuse of destroyed nodes as they might be already deallocated.
+        // It's fine though to reuse nodes that were destroyed in weak callback
+        // as those cannot be deallocated until we are back from the callback.
+        set_first_free(NULL);
         // Leaving V8.
         VMState state(EXTERNAL);
         func(object, par);

diff --git a/test/cctest/test-api.cc b/test/cctest/test-api.cc
index e1caba3c836b318baae697b592dd66f8f1bf10ce..48ee6e5aa226daa7b1248d1321ba41f4b508e7e5 100644 (file)
--- a/test/cctest/test-api.cc
+++ b/test/cctest/test-api.cc
@@ -6217,6 +6217,35 @@ TEST(DontLeakGlobalObjects) {
 }
 
 
+v8::Persistent<v8::Object> some_object;
+v8::Persistent<v8::Object> bad_handle;
+
+void NewPersistentHandleCallback(v8::Persistent<v8::Value>, void*) {
+  v8::HandleScope scope;
+  bad_handle = v8::Persistent<v8::Object>::New(some_object);
+}
+
+
+THREADED_TEST(NewPersistentHandleFromWeakCallback) {
+  LocalContext context;
+
+  v8::Persistent<v8::Object> handle1, handle2;
+  {
+    v8::HandleScope scope;
+    some_object = v8::Persistent<v8::Object>::New(v8::Object::New());
+    handle1 = v8::Persistent<v8::Object>::New(v8::Object::New());
+    handle2 = v8::Persistent<v8::Object>::New(v8::Object::New());
+  }
+  // Note: order is implementation dependent alas: currently
+  // global handle nodes are processed by PostGarbageCollectionProcessing
+  // in reverse allocation order, so if second allocated handle is deleted,
+  // weak callback of the first handle would be able to 'reallocate' it.
+  handle1.MakeWeak(NULL, NewPersistentHandleCallback);
+  handle2.Dispose();
+  i::Heap::CollectAllGarbage();
+}
+
+
 THREADED_TEST(CheckForCrossContextObjectLiterals) {
   v8::V8::Initialize();