Revision: 3318 Author: [email protected] Date: Tue Nov 17 05:50:07 2009 Log: Restore invariant (next of first deallocated must point to the head) before calling into weak callbacks.
Otherwise if callback allocates a new handle, it could orphan some global handles (with disastorous consequences if those global handles are cached). Review URL: http://codereview.chromium.org/395024 http://code.google.com/p/v8/source/detail?r=3318 Modified: /branches/bleeding_edge/src/global-handles.cc /branches/bleeding_edge/test/cctest/test-api.cc ======================================= --- /branches/bleeding_edge/src/global-handles.cc Thu Nov 5 07:12:36 2009 +++ /branches/bleeding_edge/src/global-handles.cc Tue Nov 17 05:50:07 2009 @@ -165,6 +165,9 @@ // 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); + if (first_deallocated()) { + first_deallocated()->set_next(head()); + } // Leaving V8. VMState state(EXTERNAL); func(object, par); @@ -270,6 +273,7 @@ // Next try deallocated list result = first_deallocated(); set_first_deallocated(result->next_free()); + ASSERT(result->next() == head()); set_head(result); } else { // Allocate a new node. ======================================= --- /branches/bleeding_edge/test/cctest/test-api.cc Thu Nov 12 08:34:52 2009 +++ /branches/bleeding_edge/test/cctest/test-api.cc Tue Nov 17 05:50:07 2009 @@ -6225,6 +6225,31 @@ to_be_disposed = handle2; i::Heap::CollectAllGarbage(false); } + +void DisposingCallback(v8::Persistent<v8::Value> handle, void*) { + handle.Dispose(); +} + +void HandleCreatingCallback(v8::Persistent<v8::Value> handle, void*) { + v8::HandleScope scope; + v8::Persistent<v8::Object>::New(v8::Object::New()); +} + + +THREADED_TEST(NoGlobalHandlesOrphaningDueToWeakCallback) { + LocalContext context; + + v8::Persistent<v8::Object> handle1, handle2, handle3; + { + v8::HandleScope scope; + handle3 = v8::Persistent<v8::Object>::New(v8::Object::New()); + handle2 = v8::Persistent<v8::Object>::New(v8::Object::New()); + handle1 = v8::Persistent<v8::Object>::New(v8::Object::New()); + } + handle2.MakeWeak(NULL, DisposingCallback); + handle3.MakeWeak(NULL, HandleCreatingCallback); + i::Heap::CollectAllGarbage(false); +} THREADED_TEST(CheckForCrossContextObjectLiterals) { --~--~---------~--~----~------------~-------~--~----~ v8-dev mailing list [email protected] http://groups.google.com/group/v8-dev -~----------~----~----~----~------~----~------~--~---
