Author: [email protected]
Date: Fri May 8 05:39:37 2009
New Revision: 1903
Modified:
branches/bleeding_edge/src/objects.cc
branches/bleeding_edge/src/objects.h
branches/bleeding_edge/src/runtime.cc
branches/bleeding_edge/test/cctest/test-api.cc
Log:
Fix intermittent crashes caused by unexpected GCs in
HasLocalProperty (bug introduced in r1882 et al.)
Review URL: http://codereview.chromium.org/115106
Modified: branches/bleeding_edge/src/objects.cc
==============================================================================
--- branches/bleeding_edge/src/objects.cc (original)
+++ branches/bleeding_edge/src/objects.cc Fri May 8 05:39:37 2009
@@ -5161,7 +5161,8 @@
}
// Only attempt to find the hidden properties in the local object and not
- // in the prototype chain.
+ // in the prototype chain. Note that HasLocalProperty() can cause a GC
in
+ // the general case, but in this case we know it won't hit an
interceptor.
if (!this->HasLocalProperty(key)) {
// Hidden properties object not found. Allocate a new hidden properties
// object if requested. Otherwise return the undefined value.
Modified: branches/bleeding_edge/src/objects.h
==============================================================================
--- branches/bleeding_edge/src/objects.h (original)
+++ branches/bleeding_edge/src/objects.h Fri May 8 05:39:37 2009
@@ -1262,6 +1262,7 @@
return GetPropertyAttribute(name) != ABSENT;
}
+ // Can cause a GC if it hits an interceptor.
bool HasLocalProperty(String* name) {
return GetLocalPropertyAttribute(name) != ABSENT;
}
Modified: branches/bleeding_edge/src/runtime.cc
==============================================================================
--- branches/bleeding_edge/src/runtime.cc (original)
+++ branches/bleeding_edge/src/runtime.cc Fri May 8 05:39:37 2009
@@ -2832,19 +2832,37 @@
}
-static Object* HasLocalPropertyImplementation(Object* obj, String* key) {
+static Object* HasLocalPropertyImplementation(Handle<JSObject> object,
+ Handle<String> key) {
+ if (object->HasLocalProperty(*key)) return Heap::true_value();
+ // Handle hidden prototypes. If there's a hidden prototype above this
thing
+ // then we have to check it for properties, because they are supposed to
+ // look like they are on this object.
+ Handle<Object> proto(object->GetPrototype());
+ if (proto->IsJSObject() &&
+ Handle<JSObject>::cast(proto)->map()->is_hidden_prototype()) {
+ return HasLocalPropertyImplementation(Handle<JSObject>::cast(proto),
key);
+ }
+ return Heap::false_value();
+}
+
+
+static Object* Runtime_HasLocalProperty(Arguments args) {
+ NoHandleAllocation ha;
+ ASSERT(args.length() == 2);
+ CONVERT_CHECKED(String, key, args[1]);
+
+ Object* obj = args[0];
// Only JS objects can have properties.
if (obj->IsJSObject()) {
JSObject* object = JSObject::cast(obj);
- if (object->HasLocalProperty(key)) return Heap::true_value();
- // Handle hidden prototypes. If there's a hidden prototype above this
thing
- // then we have to check it for properties, because they are supposed
to
- // look like they are on this object.
- Object* proto = object->GetPrototype();
- if (proto->IsJSObject() &&
- JSObject::cast(proto)->map()->is_hidden_prototype()) {
- return HasLocalPropertyImplementation(object->GetPrototype(), key);
- }
+ // Fast case - no interceptors.
+ if (object->HasRealNamedProperty(key)) return Heap::true_value();
+ // Slow case. Either it's not there or we have an interceptor. We
should
+ // have handles for this kind of deal.
+ HandleScope scope;
+ return HasLocalPropertyImplementation(Handle<JSObject>(object),
+ Handle<String>(key));
} else if (obj->IsString()) {
// Well, there is one exception: Handle [] on strings.
uint32_t index;
@@ -2855,15 +2873,6 @@
}
}
return Heap::false_value();
-}
-
-
-static Object* Runtime_HasLocalProperty(Arguments args) {
- NoHandleAllocation ha;
- ASSERT(args.length() == 2);
- CONVERT_CHECKED(String, key, args[1]);
-
- return HasLocalPropertyImplementation(args[0], key);
}
Modified: branches/bleeding_edge/test/cctest/test-api.cc
==============================================================================
--- branches/bleeding_edge/test/cctest/test-api.cc (original)
+++ branches/bleeding_edge/test/cctest/test-api.cc Fri May 8 05:39:37 2009
@@ -4739,6 +4739,44 @@
}
+static v8::Handle<Value> InterceptorHasOwnPropertyGetterGC(
+ Local<String> name,
+ const AccessorInfo& info) {
+ ApiTestFuzzer::Fuzz();
+ i::Heap::CollectAllGarbage();
+ return v8::Handle<Value>();
+}
+
+
+THREADED_TEST(InterceptorHasOwnPropertyCausingGC) {
+ v8::HandleScope scope;
+ LocalContext context;
+ Local<v8::FunctionTemplate> fun_templ = v8::FunctionTemplate::New();
+ Local<v8::ObjectTemplate> instance_templ = fun_templ->InstanceTemplate();
+
instance_templ->SetNamedPropertyHandler(InterceptorHasOwnPropertyGetterGC);
+ Local<Function> function = fun_templ->GetFunction();
+ context->Global()->Set(v8_str("constructor"), function);
+ // Let's first make some stuff so we can be sure to get a good GC.
+ CompileRun(
+ "function makestr(size) {"
+ " switch (size) {"
+ " case 1: return 'f';"
+ " case 2: return 'fo';"
+ " case 3: return 'foo';"
+ " }"
+ " return makestr(size >> 1) + makestr((size + 1) >> 1);"
+ "}"
+ "var x = makestr(12345);"
+ "x = makestr(31415);"
+ "x = makestr(23456);");
+ v8::Handle<Value> value = CompileRun(
+ "var o = new constructor();"
+ "o.__proto__ = new String(x);"
+ "o.hasOwnProperty('ostehaps');");
+ CHECK_EQ(false, value->BooleanValue());
+}
+
+
static v8::Handle<Value> InterceptorLoadICGetter(Local<String> name,
const AccessorInfo& info)
{
ApiTestFuzzer::Fuzz();
--~--~---------~--~----~------------~-------~--~----~
v8-dev mailing list
[email protected]
http://groups.google.com/group/v8-dev
-~----------~----~----~----~------~----~------~--~---
