diff --git a/src/global-handles.cc b/src/global-handles.cc index f868974b07..e51c4aadf1 100644 --- a/src/global-handles.cc +++ b/src/global-handles.cc @@ -144,8 +144,8 @@ class GlobalHandles::Node : public Malloced { // Returns the callback for this weak handle. WeakReferenceCallback callback() { return callback_; } - void PostGarbageCollectionProcessing() { - if (state_ != Node::PENDING) return; + bool PostGarbageCollectionProcessing() { + if (state_ != Node::PENDING) return false; LOG(HandleEvent("GlobalHandle::Processing", handle().location())); void* par = parameter(); state_ = NEAR_DEATH; @@ -153,18 +153,19 @@ class GlobalHandles::Node : public Malloced { // The callback function is resolved as late as possible to preserve old // behavior. WeakReferenceCallback func = callback(); - if (func != NULL) { - v8::Persistent object = ToApi(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); - } + if (func == NULL) return false; + + v8::Persistent object = ToApi(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); } + return true; } // Place the handle address first to avoid offset computation. @@ -275,15 +276,26 @@ void GlobalHandles::IdentifyWeakHandles(WeakSlotCallback f) { } +int post_gc_processing_count = 0; + void GlobalHandles::PostGarbageCollectionProcessing() { // Process weak global handle callbacks. This must be done after the // GC is completely done, because the callbacks may invoke arbitrary // API functions. // At the same time deallocate all DESTROYED nodes ASSERT(Heap::gc_state() == Heap::NOT_IN_GC); + const int initial_post_gc_processing_count = ++post_gc_processing_count; Node** p = &head_; while (*p != NULL) { - (*p)->PostGarbageCollectionProcessing(); + if ((*p)->PostGarbageCollectionProcessing()) { + if (initial_post_gc_processing_count != post_gc_processing_count) { + // Weak callback triggered another GC and another round of + // PostGarbageCollection processing. The current node might + // have been deleted in that round, so we need to bail out (or + // restart the processing). + break; + } + } if ((*p)->state_ == Node::DESTROYED) { // Delete the link. Node* node = *p; diff --git a/src/heap.cc b/src/heap.cc index e778c9625c..ad25f93efb 100644 --- a/src/heap.cc +++ b/src/heap.cc @@ -487,7 +487,10 @@ void Heap::PerformGarbageCollection(AllocationSpace space, void Heap::PostGarbageCollectionProcessing() { // Process weak handles post gc. - GlobalHandles::PostGarbageCollectionProcessing(); + { + DisableAssertNoAllocation allow_allocation; + GlobalHandles::PostGarbageCollectionProcessing(); + } // Update flat string readers. FlatStringReader::PostGarbageCollectionProcessing(); } diff --git a/src/heap.h b/src/heap.h index a9d44c66bc..ac6f5be511 100644 --- a/src/heap.h +++ b/src/heap.h @@ -1388,6 +1388,20 @@ class AssertNoAllocation { bool old_state_; }; +class DisableAssertNoAllocation { + public: + DisableAssertNoAllocation() { + old_state_ = Heap::allow_allocation(true); + } + + ~DisableAssertNoAllocation() { + Heap::allow_allocation(old_state_); + } + + private: + bool old_state_; +}; + #else // ndef DEBUG class AssertNoAllocation { @@ -1396,6 +1410,12 @@ class AssertNoAllocation { ~AssertNoAllocation() { } }; +class DisableAssertNoAllocation { + public: + DisableAssertNoAllocation() { } + ~DisableAssertNoAllocation() { } +}; + #endif #ifdef ENABLE_LOGGING_AND_PROFILING diff --git a/test/cctest/test-api.cc b/test/cctest/test-api.cc index 9b69f6c688..7b7e1a3be6 100644 --- a/test/cctest/test-api.cc +++ b/test/cctest/test-api.cc @@ -6246,6 +6246,29 @@ THREADED_TEST(NewPersistentHandleFromWeakCallback) { } +v8::Persistent to_be_disposed; + +void DisposeAndForceGcCallback(v8::Persistent handle, void*) { + to_be_disposed.Dispose(); + i::Heap::CollectAllGarbage(); +} + + +THREADED_TEST(DoNotUseDeletedNodesInSecondLevelGc) { + LocalContext context; + + v8::Persistent handle1, handle2; + { + v8::HandleScope scope; + handle1 = v8::Persistent::New(v8::Object::New()); + handle2 = v8::Persistent::New(v8::Object::New()); + } + handle1.MakeWeak(NULL, DisposeAndForceGcCallback); + to_be_disposed = handle2; + i::Heap::CollectAllGarbage(); +} + + THREADED_TEST(CheckForCrossContextObjectLiterals) { v8::V8::Initialize();