Revision: 21126
Author: [email protected]
Date: Fri May 2 16:13:10 2014 UTC
Log: Don't leak contexts in Object.observe
The Object.observe API may construct internal structures as a result of API
calls. These structures can persist as long as an object that was once
observed persists. This patch ensures that these structures are created in
the correct context so as to avoid leaking contexts
[email protected], dcarney
BUG=
Review URL: https://codereview.chromium.org/263833007
http://code.google.com/p/v8/source/detail?r=21126
Modified:
/branches/bleeding_edge/src/bootstrapper.cc
/branches/bleeding_edge/src/contexts.h
/branches/bleeding_edge/src/object-observe.js
/branches/bleeding_edge/src/runtime.cc
/branches/bleeding_edge/src/runtime.h
/branches/bleeding_edge/test/cctest/test-object-observe.cc
=======================================
--- /branches/bleeding_edge/src/bootstrapper.cc Fri May 2 13:03:39 2014 UTC
+++ /branches/bleeding_edge/src/bootstrapper.cc Fri May 2 16:13:10 2014 UTC
@@ -1556,6 +1556,12 @@
observers_begin_perform_splice);
INSTALL_NATIVE(JSFunction, "EndPerformSplice",
observers_end_perform_splice);
+ INSTALL_NATIVE(JSFunction, "NativeObjectObserve",
+ native_object_observe);
+ INSTALL_NATIVE(JSFunction, "NativeObjectGetNotifier",
+ native_object_get_notifier);
+ INSTALL_NATIVE(JSFunction, "NativeObjectNotifierPerformChange",
+ native_object_notifier_perform_change);
}
=======================================
--- /branches/bleeding_edge/src/contexts.h Fri May 2 08:00:47 2014 UTC
+++ /branches/bleeding_edge/src/contexts.h Fri May 2 16:13:10 2014 UTC
@@ -174,6 +174,12 @@
observers_begin_perform_splice) \
V(OBSERVERS_END_SPLICE_INDEX, JSFunction, \
observers_end_perform_splice) \
+ V(NATIVE_OBJECT_OBSERVE_INDEX, JSFunction, \
+ native_object_observe) \
+ V(NATIVE_OBJECT_GET_NOTIFIER_INDEX, JSFunction, \
+ native_object_get_notifier) \
+ V(NATIVE_OBJECT_NOTIFIER_PERFORM_CHANGE, JSFunction, \
+ native_object_notifier_perform_change) \
V(SLOPPY_GENERATOR_FUNCTION_MAP_INDEX, Map,
sloppy_generator_function_map) \
V(STRICT_GENERATOR_FUNCTION_MAP_INDEX, Map,
strict_generator_function_map) \
V(GENERATOR_OBJECT_PROTOTYPE_MAP_INDEX, Map, \
@@ -342,6 +348,9 @@
OBSERVERS_ENQUEUE_SPLICE_INDEX,
OBSERVERS_BEGIN_SPLICE_INDEX,
OBSERVERS_END_SPLICE_INDEX,
+ NATIVE_OBJECT_OBSERVE_INDEX,
+ NATIVE_OBJECT_GET_NOTIFIER_INDEX,
+ NATIVE_OBJECT_NOTIFIER_PERFORM_CHANGE,
SLOPPY_GENERATOR_FUNCTION_MAP_INDEX,
STRICT_GENERATOR_FUNCTION_MAP_INDEX,
GENERATOR_OBJECT_PROTOTYPE_MAP_INDEX,
=======================================
--- /branches/bleeding_edge/src/object-observe.js Fri May 2 13:55:11 2014
UTC
+++ /branches/bleeding_edge/src/object-observe.js Fri May 2 16:13:10 2014
UTC
@@ -56,6 +56,7 @@
};
MapWrapper.prototype = {
+ __proto__: null,
get: function(key) {
return %WeakCollectionGet(this.map_, key);
},
@@ -364,6 +365,10 @@
if (!AcceptArgIsValid(acceptList))
throw MakeTypeError("observe_accept_invalid");
+ return %NativeObjectObserve(object, callback, acceptList);
+}
+
+function NativeObjectObserve(object, callback, acceptList) {
var objectInfo = ObjectInfoGetOrCreate(object);
ObjectInfoAddObserver(objectInfo, callback, acceptList);
return object;
@@ -527,7 +532,6 @@
throw MakeTypeError("called_on_non_object", ["performChange"]);
var objectInfo = ObjectInfoGetFromNotifier(this);
-
if (IS_UNDEFINED(objectInfo))
throw MakeTypeError("observe_notify_non_notifier");
if (!IS_STRING(changeType))
@@ -535,6 +539,10 @@
if (!IS_SPEC_FUNCTION(changeFn))
throw MakeTypeError("observe_perform_non_function");
+ return %NativeObjectNotifierPerformChange(objectInfo, changeType,
changeFn)
+}
+
+function NativeObjectNotifierPerformChange(objectInfo, changeType,
changeFn) {
ObjectInfoAddPerformingType(objectInfo, changeType);
var changeRecord;
@@ -558,6 +566,10 @@
if (!%ObjectWasCreatedInCurrentOrigin(object)) return null;
+ return %NativeObjectGetNotifier(object);
+}
+
+function NativeObjectGetNotifier(object) {
var objectInfo = ObjectInfoGetOrCreate(object);
return ObjectInfoGetNotifier(objectInfo);
}
=======================================
--- /branches/bleeding_edge/src/runtime.cc Fri May 2 13:55:11 2014 UTC
+++ /branches/bleeding_edge/src/runtime.cc Fri May 2 16:13:10 2014 UTC
@@ -14994,6 +14994,65 @@
return isolate->heap()->ToBoolean(
ContextsHaveSameOrigin(creation_context, isolate->native_context()));
}
+
+
+RUNTIME_FUNCTION(Runtime_NativeObjectObserve) {
+ HandleScope scope(isolate);
+ ASSERT(args.length() == 3);
+ CONVERT_ARG_HANDLE_CHECKED(JSObject, object, 0);
+ CONVERT_ARG_HANDLE_CHECKED(Object, callback, 1);
+ CONVERT_ARG_HANDLE_CHECKED(Object, accept, 2);
+
+ Handle<Context> context(object->GetCreationContext(), isolate);
+ Handle<JSFunction> function(context->native_object_observe(), isolate);
+ Handle<Object> call_args[] = { object, callback, accept };
+ Handle<Object> result;
+
+ ASSIGN_RETURN_FAILURE_ON_EXCEPTION(
+ isolate, result,
+ Execution::Call(isolate, function,
+ handle(context->object_function(), isolate), 3, call_args,
true));
+ return *result;
+}
+
+
+RUNTIME_FUNCTION(Runtime_NativeObjectGetNotifier) {
+ HandleScope scope(isolate);
+ ASSERT(args.length() == 1);
+ CONVERT_ARG_HANDLE_CHECKED(JSObject, object, 0);
+
+ Handle<Context> context(object->GetCreationContext(), isolate);
+ Handle<JSFunction> function(context->native_object_get_notifier(),
isolate);
+ Handle<Object> call_args[] = { object };
+ Handle<Object> result;
+
+ ASSIGN_RETURN_FAILURE_ON_EXCEPTION(
+ isolate, result,
+ Execution::Call(isolate, function,
+ handle(context->object_function(), isolate), 1, call_args,
true));
+ return *result;
+}
+
+
+RUNTIME_FUNCTION(Runtime_NativeObjectNotifierPerformChange) {
+ HandleScope scope(isolate);
+ ASSERT(args.length() == 3);
+ CONVERT_ARG_HANDLE_CHECKED(JSObject, object_info, 0);
+ CONVERT_ARG_HANDLE_CHECKED(Object, change_type, 1);
+ CONVERT_ARG_HANDLE_CHECKED(Object, change_fn, 2);
+
+ Handle<Context> context(object_info->GetCreationContext(), isolate);
+ Handle<JSFunction>
function(context->native_object_notifier_perform_change(),
+ isolate);
+ Handle<Object> call_args[] = { change_type, change_fn };
+ Handle<Object> result;
+
+ ASSIGN_RETURN_FAILURE_ON_EXCEPTION(
+ isolate, result,
+ Execution::Call(isolate, function,
isolate->factory()->undefined_value(),
+ 2, call_args, true));
+ return *result;
+}
static Object* ArrayConstructorCommon(Isolate* isolate,
=======================================
--- /branches/bleeding_edge/src/runtime.h Fri May 2 13:55:11 2014 UTC
+++ /branches/bleeding_edge/src/runtime.h Fri May 2 16:13:10 2014 UTC
@@ -313,6 +313,8 @@
F(ObservationWeakMapCreate, 0, 1) \
F(ObserverObjectAndRecordHaveSameOrigin, 3, 1) \
F(ObjectWasCreatedInCurrentOrigin, 1, 1) \
+ F(NativeObjectObserve, 3, 1) \
+ F(NativeObjectGetNotifier, 1, 1) \
\
/* Harmony typed arrays */ \
F(ArrayBufferInitialize, 2, 1)\
=======================================
--- /branches/bleeding_edge/test/cctest/test-object-observe.cc Fri May 2
13:55:11 2014 UTC
+++ /branches/bleeding_edge/test/cctest/test-object-observe.cc Fri May 2
16:13:10 2014 UTC
@@ -613,3 +613,95 @@
CHECK(CompileRun("Object.getNotifier(obj)")->IsObject());
}
}
+
+
+static int GetGlobalObjectsCount() {
+ CcTest::heap()->EnsureHeapIsIterable();
+ int count = 0;
+ i::HeapIterator it(CcTest::heap());
+ for (i::HeapObject* object = it.next(); object != NULL; object =
it.next())
+ if (object->IsJSGlobalObject()) count++;
+ return count;
+}
+
+
+static void CheckSurvivingGlobalObjectsCount(int expected) {
+ // We need to collect all garbage twice to be sure that everything
+ // has been collected. This is because inline caches are cleared in
+ // the first garbage collection but some of the maps have already
+ // been marked at that point. Therefore some of the maps are not
+ // collected until the second garbage collection.
+ CcTest::heap()->CollectAllGarbage(i::Heap::kNoGCFlags);
+ CcTest::heap()->CollectAllGarbage(i::Heap::kMakeHeapIterableMask);
+ int count = GetGlobalObjectsCount();
+#ifdef DEBUG
+ if (count != expected) CcTest::heap()->TracePathToGlobal();
+#endif
+ CHECK_EQ(expected, count);
+}
+
+
+TEST(DontLeakContextOnObserve) {
+ HandleScope scope(CcTest::isolate());
+ Handle<Value> foo = String::NewFromUtf8(CcTest::isolate(), "foo");
+ LocalContext context(CcTest::isolate());
+ context->SetSecurityToken(foo);
+ CompileRun("var obj = {};");
+ Handle<Value> object = CompileRun("obj");
+ {
+ HandleScope scope(CcTest::isolate());
+ LocalContext context2(CcTest::isolate());
+ context2->SetSecurityToken(foo);
+ context2->Global()->Set(String::NewFromUtf8(CcTest::isolate(), "obj"),
+ object);
+ CompileRun("function observer() {};"
+ "Object.observe(obj, observer, ['foo', 'bar', 'baz']);"
+ "Object.unobserve(obj, observer);");
+ }
+
+ v8::V8::ContextDisposedNotification();
+ CheckSurvivingGlobalObjectsCount(1);
+}
+
+
+TEST(DontLeakContextOnGetNotifier) {
+ HandleScope scope(CcTest::isolate());
+ Handle<Value> foo = String::NewFromUtf8(CcTest::isolate(), "foo");
+ LocalContext context(CcTest::isolate());
+ context->SetSecurityToken(foo);
+ CompileRun("var obj = {};");
+ Handle<Value> object = CompileRun("obj");
+ {
+ HandleScope scope(CcTest::isolate());
+ LocalContext context2(CcTest::isolate());
+ context2->SetSecurityToken(foo);
+ context2->Global()->Set(String::NewFromUtf8(CcTest::isolate(), "obj"),
+ object);
+ CompileRun("Object.getNotifier(obj);");
+ }
+
+ v8::V8::ContextDisposedNotification();
+ CheckSurvivingGlobalObjectsCount(1);
+}
+
+
+TEST(DontLeakContextOnNotifierPerformChange) {
+ HandleScope scope(CcTest::isolate());
+ Handle<Value> foo = String::NewFromUtf8(CcTest::isolate(), "foo");
+ LocalContext context(CcTest::isolate());
+ context->SetSecurityToken(foo);
+ CompileRun("var obj = {};");
+ Handle<Value> object = CompileRun("obj");
+ {
+ HandleScope scope(CcTest::isolate());
+ LocalContext context2(CcTest::isolate());
+ context2->SetSecurityToken(foo);
+ context2->Global()->Set(String::NewFromUtf8(CcTest::isolate(), "obj"),
+ object);
+ CompileRun("var n = Object.getNotifier(obj);"
+ "n.performChange('foo', function() {});");
+ }
+
+ v8::V8::ContextDisposedNotification();
+ CheckSurvivingGlobalObjectsCount(1);
+}
--
--
v8-dev mailing list
[email protected]
http://groups.google.com/group/v8-dev
---
You received this message because you are subscribed to the Google Groups "v8-dev" group.
To unsubscribe from this group and stop receiving emails from it, send an email
to [email protected].
For more options, visit https://groups.google.com/d/optout.
