Revision: 5096
Author: [email protected]
Date: Mon Jul 19 06:26:25 2010
Log: 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
http://code.google.com/p/v8/source/detail?r=5096
Modified:
/branches/bleeding_edge/include/v8.h
/branches/bleeding_edge/src/global-handles.cc
/branches/bleeding_edge/src/profile-generator.cc
/branches/bleeding_edge/test/cctest/test-api.cc
/branches/bleeding_edge/test/cctest/test-heap.cc
/branches/bleeding_edge/test/cctest/test-mark-compact.cc
=======================================
--- /branches/bleeding_edge/include/v8.h Mon Jul 19 02:51:33 2010
+++ /branches/bleeding_edge/include/v8.h Mon Jul 19 06:26:25 2010
@@ -137,6 +137,9 @@
/**
* 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
*/
=======================================
--- /branches/bleeding_edge/src/global-handles.cc Wed Dec 9 06:32:45 2009
+++ /branches/bleeding_edge/src/global-handles.cc Mon Jul 19 06:26:25 2010
@@ -151,13 +151,14 @@
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 @@
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;
}
=======================================
--- /branches/bleeding_edge/src/profile-generator.cc Fri Jul 16 01:20:39
2010
+++ /branches/bleeding_edge/src/profile-generator.cc Mon Jul 19 06:26:25
2010
@@ -74,6 +74,7 @@
void* parameter) {
reinterpret_cast<TokenEnumerator*>(parameter)->TokenRemoved(
Utils::OpenHandle(*handle).location());
+ handle.Dispose();
}
=======================================
--- /branches/bleeding_edge/test/cctest/test-api.cc Tue Jul 13 04:38:30 2010
+++ /branches/bleeding_edge/test/cctest/test-api.cc Mon Jul 19 06:26:25 2010
@@ -8015,9 +8015,10 @@
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 @@
void DisposeAndForceGcCallback(v8::Persistent<v8::Value> handle, void*) {
to_be_disposed.Dispose();
i::Heap::CollectAllGarbage(false);
+ handle.Dispose();
}
@@ -8070,6 +8072,7 @@
void HandleCreatingCallback(v8::Persistent<v8::Value> handle, void*) {
v8::HandleScope scope;
v8::Persistent<v8::Object>::New(v8::Object::New());
+ handle.Dispose();
}
=======================================
--- /branches/bleeding_edge/test/cctest/test-heap.cc Tue Jul 13 06:06:33
2010
+++ /branches/bleeding_edge/test/cctest/test-heap.cc Mon Jul 19 06:26:25
2010
@@ -322,8 +322,8 @@
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 @@
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 @@
GlobalHandles::MakeWeak(h.location(),
reinterpret_cast<void*>(1234),
- &TestDeleteWeakGlobalHandleCallback);
+ &TestWeakGlobalHandleCallback);
// Scanvenge does not recognize weak reference.
Heap::PerformScavenge();
=======================================
--- /branches/bleeding_edge/test/cctest/test-mark-compact.cc Tue Jan 19
08:34:37 2010
+++ /branches/bleeding_edge/test/cctest/test-mark-compact.cc Mon Jul 19
06:26:25 2010
@@ -273,6 +273,7 @@
static int NumberOfWeakCalls = 0;
static void WeakPointerCallback(v8::Persistent<v8::Value> handle, void*
id) {
NumberOfWeakCalls++;
+ handle.Dispose();
}
TEST(ObjectGroups) {
--
v8-dev mailing list
[email protected]
http://groups.google.com/group/v8-dev
