Reviewers: Mads Ager, Description: 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).
Please review this at http://codereview.chromium.org/173060 SVN Base: http://v8.googlecode.com/svn/branches/bleeding_edge/ Affected files: M src/global-handles.cc M test/cctest/test-api.cc Index: src/global-handles.cc =================================================================== --- src/global-handles.cc (revision 2717) +++ src/global-handles.cc (working copy) @@ -156,6 +156,10 @@ 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); Index: test/cctest/test-api.cc =================================================================== --- test/cctest/test-api.cc (revision 2717) +++ test/cctest/test-api.cc (working copy) @@ -6217,6 +6217,35 @@ } +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(); --~--~---------~--~----~------------~-------~--~----~ v8-dev mailing list [email protected] http://groups.google.com/group/v8-dev -~----------~----~----~----~------~----~------~--~---
