Add a check that weak object handle is not in NEAR_DEATH state after weak callback invocation.

If object enters NEAR_DEATH state, it must be explicitly cleared and/or disposed, otherwise
it would retain JS object forever.  Note as well that parameter is reset to NULL on first
invocation so weak handle callback would be in hard situation.

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

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

include/v8.h

@@ -137,6 +137,9 @@ class Top;
 /**
  * A weak reference callback function.
  *
+ * This callback should either explicitly invoke Dispose on |object| if
+ * V8 wrapper is not needed anymore, or 'revive' it by invocation of MakeWeak.
+ *
  * \param object the weak global object to be reclaimed by the garbage collector
  * \param parameter the value passed in when making the weak global object
  */

src/global-handles.cc

@@ -151,13 +151,14 @@ class GlobalHandles::Node : public Malloced {
   bool PostGarbageCollectionProcessing() {
     if (state_ != Node::PENDING) return false;
     LOG(HandleEvent("GlobalHandle::Processing", handle().location()));
+    WeakReferenceCallback func = callback();
+    if (func == NULL) {
+      Destroy();
+      return false;
+    }
     void* par = parameter();
     state_ = NEAR_DEATH;
     set_parameter(NULL);
-    // The callback function is resolved as late as possible to preserve old
-    // behavior.
-    WeakReferenceCallback func = callback();
-    if (func == NULL) return false;
 
     v8::Persistent<v8::Object> object = ToApi<v8::Object>(handle());
     {
@@ -178,6 +179,9 @@ class GlobalHandles::Node : public Malloced {
       VMState state(EXTERNAL);
       func(object, par);
     }
+    // Absense of explicit cleanup or revival of weak handle
+    // in most of the cases would lead to memory leak.
+    ASSERT(state_ != NEAR_DEATH);
     return true;
   }
 

src/profile-generator.cc

@@ -74,6 +74,7 @@ void TokenEnumerator::TokenRemovedCallback(v8::Persistent<v8::Value> handle,
                                            void* parameter) {
   reinterpret_cast<TokenEnumerator*>(parameter)->TokenRemoved(
       Utils::OpenHandle(*handle).location());
+  handle.Dispose();
 }
 
 

test/cctest/test-api.cc

@@ -8015,9 +8015,10 @@ TEST(DontLeakGlobalObjects) {
 v8::Persistent<v8::Object> some_object;
 v8::Persistent<v8::Object> bad_handle;
 
-void NewPersistentHandleCallback(v8::Persistent<v8::Value>, void*) {
+void NewPersistentHandleCallback(v8::Persistent<v8::Value> handle, void*) {
   v8::HandleScope scope;
   bad_handle = v8::Persistent<v8::Object>::New(some_object);
+  handle.Dispose();
 }
 
 
@@ -8046,6 +8047,7 @@ v8::Persistent<v8::Object> to_be_disposed;
 void DisposeAndForceGcCallback(v8::Persistent<v8::Value> handle, void*) {
   to_be_disposed.Dispose();
   i::Heap::CollectAllGarbage(false);
+  handle.Dispose();
 }
 
 
@@ -8070,6 +8072,7 @@ void DisposingCallback(v8::Persistent<v8::Value> handle, void*) {
 void HandleCreatingCallback(v8::Persistent<v8::Value> handle, void*) {
   v8::HandleScope scope;
   v8::Persistent<v8::Object>::New(v8::Object::New());
+  handle.Dispose();
 }
 
 

test/cctest/test-heap.cc

@@ -322,8 +322,8 @@ static bool WeakPointerCleared = false;
 
 static void TestWeakGlobalHandleCallback(v8::Persistent<v8::Value> handle,
                                          void* id) {
-  USE(handle);
   if (1234 == reinterpret_cast<intptr_t>(id)) WeakPointerCleared = true;
+  handle.Dispose();
 }
 
 
@@ -398,17 +398,8 @@ TEST(WeakGlobalHandlesMark) {
 
   CHECK(WeakPointerCleared);
   CHECK(!GlobalHandles::IsNearDeath(h1.location()));
-  CHECK(GlobalHandles::IsNearDeath(h2.location()));
 
   GlobalHandles::Destroy(h1.location());
-  GlobalHandles::Destroy(h2.location());
-}
-
-static void TestDeleteWeakGlobalHandleCallback(
-    v8::Persistent<v8::Value> handle,
-    void* id) {
-  if (1234 == reinterpret_cast<intptr_t>(id)) WeakPointerCleared = true;
-  handle.Dispose();
 }
 
 TEST(DeleteWeakGlobalHandle) {
@@ -427,7 +418,7 @@ TEST(DeleteWeakGlobalHandle) {
 
   GlobalHandles::MakeWeak(h.location(),
                           reinterpret_cast<void*>(1234),
-                          &TestDeleteWeakGlobalHandleCallback);
+                          &TestWeakGlobalHandleCallback);
 
   // Scanvenge does not recognize weak reference.
   Heap::PerformScavenge();

test/cctest/test-mark-compact.cc

@@ -273,6 +273,7 @@ TEST(GCCallback) {
 static int NumberOfWeakCalls = 0;
 static void WeakPointerCallback(v8::Persistent<v8::Value> handle, void* id) {
   NumberOfWeakCalls++;
+  handle.Dispose();
 }
 
 TEST(ObjectGroups) {