Hi Kent!
Here is the diff against 565cca4b737f28572230d88a42052ce5014294b9
you've requested. I had to do some changes. For example your version
of V8 doesn't even have that Foreign::foreign_address()
function that caused the segfault I wrote about earlier.
So I've changed it back to Foreign::address().
I also ran the tests, all of them passed.
Best regards,
Gabor Ballabas
----------------------------------------------------------------
This message was sent using IMP, the Internet Messaging Program.
diff --git a/include/v8.h b/include/v8.h
index 3ef4dd6..ac5a374 100644
--- a/include/v8.h
+++ b/include/v8.h
@@ -1613,8 +1613,8 @@ class Object : public Value {
friend class v8::internal::Heap;
};
- V8EXPORT void SetExternalResource(ExternalResource *);
- V8EXPORT ExternalResource *GetExternalResource();
+ V8EXPORT void SetExternalResource(ExternalResource* resource);
+ V8EXPORT ExternalResource* GetExternalResource();
// Testers for local properties.
V8EXPORT bool HasOwnProperty(Handle<String> key);
diff --git a/src/api.cc b/src/api.cc
index 7d54252..5a8bb78 100644
--- a/src/api.cc
+++ b/src/api.cc
@@ -1436,8 +1436,7 @@ void ObjectTemplate::SetInternalFieldCount(int value) {
}
-bool ObjectTemplate::HasExternalResource()
-{
+bool ObjectTemplate::HasExternalResource() {
if (IsDeadCheck(Utils::OpenHandle(this)->GetIsolate(),
"v8::ObjectTemplate::HasExternalResource()")) {
return 0;
@@ -1446,8 +1445,7 @@ bool ObjectTemplate::HasExternalResource()
}
-void ObjectTemplate::SetHasExternalResource(bool value)
-{
+void ObjectTemplate::SetHasExternalResource(bool value) {
i::Isolate* isolate = Utils::OpenHandle(this)->GetIsolate();
if (IsDeadCheck(isolate, "v8::ObjectTemplate::SetHasExternalResource()")) {
return;
@@ -1459,7 +1457,8 @@ void ObjectTemplate::SetHasExternalResource(bool value)
if (value) {
Utils::OpenHandle(this)->set_has_external_resource(i::Smi::FromInt(1));
} else {
- Utils::OpenHandle(this)->set_has_external_resource(Utils::OpenHandle(this)->GetHeap()->undefined_value());
+ Utils::OpenHandle(this)->set_has_external_resource(
+ Utils::OpenHandle(this)->GetHeap()->undefined_value());
}
}
@@ -4057,14 +4056,15 @@ void v8::Object::SetPointerInInternalField(int index, void* value) {
}
-void v8::Object::SetExternalResource(v8::Object::ExternalResource *resource) {
+void v8::Object::SetExternalResource(v8::Object::ExternalResource* resource) {
i::Isolate* isolate = Utils::OpenHandle(this)->GetIsolate();
ENTER_V8(isolate);
i::Handle<i::JSObject> obj = Utils::OpenHandle(this);
if (CanBeEncodedAsSmi(resource)) {
obj->SetExternalResourceObject(EncodeAsSmi(resource));
} else {
- obj->SetExternalResourceObject(*isolate->factory()->NewForeign(static_cast<i::Address>((void *)resource)));
+ obj->SetExternalResourceObject(*isolate->factory()->NewForeign(
+ static_cast<i::Address>(reinterpret_cast<void*>(resource))));
}
if (!obj->IsSymbol()) {
isolate->heap()->external_string_table()->AddObject(*obj);
@@ -4072,13 +4072,15 @@ void v8::Object::SetExternalResource(v8::Object::ExternalResource *resource) {
}
-v8::Object::ExternalResource *v8::Object::GetExternalResource() {
+v8::Object::ExternalResource* v8::Object::GetExternalResource() {
i::Handle<i::JSObject> obj = Utils::OpenHandle(this);
i::Object* value = obj->GetExternalResourceObject();
if (value->IsSmi()) {
- return reinterpret_cast<v8::Object::ExternalResource*>(i::Internals::GetExternalPointerFromSmi(value));
+ return reinterpret_cast<v8::Object::ExternalResource*>(
+ i::Internals::GetExternalPointerFromSmi(value));
} else if (value->IsForeign()) {
- return reinterpret_cast<v8::Object::ExternalResource*>(i::Foreign::cast(value)->address());
+ return reinterpret_cast<v8::Object::ExternalResource*>(
+ i::Foreign::cast(value)->address());
} else {
return NULL;
}
diff --git a/src/heap-inl.h b/src/heap-inl.h
index bca57cb..371ff5a 100644
--- a/src/heap-inl.h
+++ b/src/heap-inl.h
@@ -231,25 +231,28 @@ void Heap::FinalizeExternalString(HeapObject* string) {
reinterpret_cast<byte*>(string) +
ExternalString::kResourceOffset -
kHeapObjectTag);
-
+
// Dispose of the C++ object if it has not already been disposed.
if (*resource_addr != NULL) {
(*resource_addr)->Dispose();
+ *resource_addr = NULL;
}
-
- // Clear the resource pointer in the string.
- *resource_addr = NULL;
+
} else {
- JSObject *object = JSObject::cast(string);
- Object *value = object->GetExternalResourceObject();
+ JSObject* object = JSObject::cast(string);
+ Object* value = object->GetExternalResourceObject();
v8::Object::ExternalResource *resource = 0;
+
if (value->IsSmi()) {
- resource = reinterpret_cast<v8::Object::ExternalResource*>(Internals::GetExternalPointerFromSmi(value));
+ resource = reinterpret_cast<v8::Object::ExternalResource*>(
+ Internals::GetExternalPointerFromSmi(value));
} else if (value->IsForeign()) {
- resource = reinterpret_cast<v8::Object::ExternalResource*>(Foreign::cast(value)->address());
- }
+ resource = reinterpret_cast<v8::Object::ExternalResource*>(
+ Foreign::cast(value)->address());
+ }
+
if (resource) {
- resource->Dispose();
+ resource->Dispose();
}
}
}
diff --git a/src/heap.cc b/src/heap.cc
index 53a0f27..10c10fe 100644
--- a/src/heap.cc
+++ b/src/heap.cc
@@ -1099,8 +1099,9 @@ void Heap::Scavenge() {
}
-HeapObject* Heap::UpdateNewSpaceReferenceInExternalStringTableEntry(Heap* heap,
- Object** p) {
+HeapObject* Heap::UpdateNewSpaceReferenceInExternalStringTableEntry(
+ Heap* heap,
+ Object** p) {
MapWord first_word = HeapObject::cast(*p)->map_word();
if (!first_word.IsForwardingAddress()) {
@@ -1132,7 +1133,8 @@ void Heap::UpdateNewSpaceReferencesInExternalStringTable(
if (target == NULL) continue;
- ASSERT(target->IsExternalString() || target->map()->has_external_resource());
+ ASSERT(target->IsExternalString() ||
+ target->map()->has_external_resource());
if (InNewSpace(target)) {
// String is still in new space. Update the table entry.
@@ -6367,15 +6369,15 @@ void ExternalStringTable::CleanUp() {
void ExternalStringTable::TearDown() {
- for (int i = 0; i < new_space_strings_.length(); ++i) {
+ for (int i = 0; i < new_space_strings_.length(); ++i) {
if (new_space_strings_[i] == heap_->raw_unchecked_null_value()) continue;
- HeapObject *object = HeapObject::cast(new_space_strings_[i]);
+ HeapObject* object = HeapObject::cast(new_space_strings_[i]);
if (!object->IsExternalString())
heap_->FinalizeExternalString(object);
}
- for (int i = 0; i < old_space_strings_.length(); ++i) {
+ for (int i = 0; i < old_space_strings_.length(); ++i) {
if (old_space_strings_[i] == heap_->raw_unchecked_null_value()) continue;
- HeapObject *object = HeapObject::cast(old_space_strings_[i]);
+ HeapObject* object = HeapObject::cast(old_space_strings_[i]);
if (!object->IsExternalString())
heap_->FinalizeExternalString(object);
}
diff --git a/src/mark-compact.cc b/src/mark-compact.cc
index bf0aab8..ad3f386 100644
--- a/src/mark-compact.cc
+++ b/src/mark-compact.cc
@@ -1514,7 +1514,8 @@ class SymbolTableCleaner : public ObjectVisitor {
// Since no objects have yet been moved we can safely access the map of
// the object.
if (o->IsExternalString() ||
- (o->IsHeapObject() && HeapObject::cast(o)->map()->has_external_resource())) {
+ (o->IsHeapObject() &&
+ HeapObject::cast(o)->map()->has_external_resource())) {
heap_->FinalizeExternalString(HeapObject::cast(*p));
}
// Set the entry to null_value (as deleted).
diff --git a/src/objects-inl.h b/src/objects-inl.h
index 6a80c9c..eb97033 100644
--- a/src/objects-inl.h
+++ b/src/objects-inl.h
@@ -1343,7 +1343,8 @@ int JSObject::GetInternalFieldCount() {
// Make sure to adjust for the number of in-object properties. These
// properties do contribute to the size, but are not internal fields.
return ((Size() - GetHeaderSize()) >> kPointerSizeLog2) -
- map()->inobject_properties() - (map()->has_external_resource()?1:0);
+ map()->inobject_properties() -
+ (map()->has_external_resource() ? 1 : 0);
}
@@ -1362,18 +1363,7 @@ Object* JSObject::GetInternalField(int index) {
}
-void JSObject::SetInternalField(int index, Object* value) {
- ASSERT(index < GetInternalFieldCount() && index >= 0);
- // Internal objects do follow immediately after the header, whereas in-object
- // properties are at the end of the object. Therefore there is no need
- // to adjust the index here.
- int offset = GetHeaderSize() + (kPointerSize * index);
- WRITE_FIELD(this, offset, value);
- WRITE_BARRIER(GetHeap(), this, offset, value);
-}
-
-
-void JSObject::SetExternalResourceObject(Object *value) {
+void JSObject::SetExternalResourceObject(Object* value) {
ASSERT(map()->has_external_resource());
int offset = GetHeaderSize() + kPointerSize * GetInternalFieldCount();
WRITE_FIELD(this, offset, value);
@@ -1381,15 +1371,27 @@ void JSObject::SetExternalResourceObject(Object *value) {
}
-Object *JSObject::GetExternalResourceObject() {
+Object *JSObject::GetExternalResourceObject() {
if (map()->has_external_resource()) {
- return READ_FIELD(this, GetHeaderSize() + kPointerSize * GetInternalFieldCount());
+ return READ_FIELD(this, GetHeaderSize() +
+ kPointerSize * GetInternalFieldCount());
} else {
return GetHeap()->undefined_value();
}
}
+void JSObject::SetInternalField(int index, Object* value) {
+ ASSERT(index < GetInternalFieldCount() && index >= 0);
+ // Internal objects do follow immediately after the header, whereas in-object
+ // properties are at the end of the object. Therefore there is no need
+ // to adjust the index here.
+ int offset = GetHeaderSize() + (kPointerSize * index);
+ WRITE_FIELD(this, offset, value);
+ WRITE_BARRIER(GetHeap(), this, offset, value);
+}
+
+
// Access fast-case object properties at index. The use of these routines
// is needed to correctly distinguish between properties stored in-object and
// properties stored in the properties array.
@@ -2772,32 +2774,31 @@ bool Map::is_shared() {
return ((1 << kIsShared) & bit_field3()) != 0;
}
-void Map::set_has_external_resource(bool value) {
+void Map::set_named_interceptor_is_fallback(bool value)
+{
if (value) {
- set_bit_field(bit_field() | (1 << kHasExternalResource));
+ set_bit_field3(bit_field3() | (1 << kNamedInterceptorIsFallback));
} else {
- set_bit_field(bit_field() & ~(1 << kHasExternalResource));
+ set_bit_field3(bit_field3() & ~(1 << kNamedInterceptorIsFallback));
}
}
-bool Map::has_external_resource()
+bool Map::named_interceptor_is_fallback()
{
- return ((1 << kHasExternalResource) & bit_field()) != 0;
+ return ((1 << kNamedInterceptorIsFallback) & bit_field3()) != 0;
}
-
-void Map::set_named_interceptor_is_fallback(bool value)
-{
+
+void Map::set_has_external_resource(bool value) {
if (value) {
- set_bit_field3(bit_field3() | (1 << kNamedInterceptorIsFallback));
+ set_bit_field(bit_field() | (1 << kHasExternalResource));
} else {
- set_bit_field3(bit_field3() & ~(1 << kNamedInterceptorIsFallback));
+ set_bit_field(bit_field() & ~(1 << kHasExternalResource));
}
}
-bool Map::named_interceptor_is_fallback()
-{
- return ((1 << kNamedInterceptorIsFallback) & bit_field3()) != 0;
+bool Map::has_external_resource() {
+ return ((1 << kHasExternalResource) & bit_field()) != 0;
}
diff --git a/src/objects.h b/src/objects.h
index c38d461..9918a92 100644
--- a/src/objects.h
+++ b/src/objects.h
@@ -1760,8 +1760,9 @@ class JSObject: public JSReceiver {
inline Object* GetInternalField(int index);
inline void SetInternalField(int index, Object* value);
- inline void SetExternalResourceObject(Object *);
- inline Object *GetExternalResourceObject();
+ inline void SetExternalResourceObject(Object* value);
+ inline Object* GetExternalResourceObject();
+
// The following lookup functions skip interceptors.
void LocalLookupRealNamedProperty(String* name, LookupResult* result);
@@ -4246,15 +4247,15 @@ class Map: public HeapObject {
inline void set_is_access_check_needed(bool access_check_needed);
inline bool is_access_check_needed();
- // Whether the named interceptor is a fallback interceptor or not
- inline void set_named_interceptor_is_fallback(bool value);
- inline bool named_interceptor_is_fallback();
-
// Tells whether the instance has the space for an external resource
// object
inline void set_has_external_resource(bool value);
inline bool has_external_resource();
+ // Whether the named interceptor is a fallback interceptor or not
+ inline void set_named_interceptor_is_fallback(bool value);
+ inline bool named_interceptor_is_fallback();
+
// [prototype]: implicit prototype object.
DECL_ACCESSORS(prototype, Object)
@@ -4519,8 +4520,8 @@ class Map: public HeapObject {
// Bit positions for bit field 3
static const int kIsShared = 0;
- static const int kNamedInterceptorIsFallback = 1;
static const int kHasInstanceCallHandler = 2;
+ static const int kNamedInterceptorIsFallback = 1;
// Layout of the default cache. It holds alternating name and code objects.
static const int kCodeCacheEntrySize = 2;
@@ -7565,7 +7566,8 @@ class ObjectTemplateInfo: public TemplateInfo {
static const int kConstructorOffset = TemplateInfo::kHeaderSize;
static const int kInternalFieldCountOffset =
kConstructorOffset + kPointerSize;
- static const int kHasExternalResourceOffset = kInternalFieldCountOffset + kPointerSize;
+ static const int kHasExternalResourceOffset =
+ kInternalFieldCountOffset + kPointerSize;
static const int kSize = kHasExternalResourceOffset + kPointerSize;
};
diff --git a/test/cctest/test-api.cc b/test/cctest/test-api.cc
index 5081a64..3743b02 100644
--- a/test/cctest/test-api.cc
+++ b/test/cctest/test-api.cc
@@ -710,6 +710,179 @@ TEST(ExternalStringWithDisposeHandling) {
}
+class TestPoint {
+ public:
+ TestPoint(int x, int y)
+ : x_(x), y_(y) { }
+
+ int32_t x_, y_;
+};
+
+
+class TestPointResource : public v8::Object::ExternalResource {
+ public:
+ TestPointResource(TestPoint* tpoint, int* counter = NULL)
+ : tpoint_(tpoint), counter_(counter) {}
+
+ ~TestPointResource() {
+ delete tpoint_;
+ if (counter_ != NULL) ++*counter_;
+ }
+
+ TestPoint* tpoint_;
+ int* counter_;
+};
+
+
+static Handle<Value> GetTestPointX(Local<String>, const AccessorInfo& info) {
+ Local<Object> self = info.Holder();
+ TestPointResource* test_point_resource =
+ static_cast<TestPointResource*>(self->GetExternalResource());
+ CHECK(test_point_resource);
+ return v8::Int32::New(test_point_resource->tpoint_->x_);
+}
+
+
+static Handle<Value> GetTestPointY(Local<String>, const AccessorInfo& info) {
+ Local<Object> self = info.Holder();
+ TestPointResource* test_point_resource =
+ static_cast<TestPointResource*>(self->GetExternalResource());
+ CHECK(test_point_resource);
+ return v8::Int32::New(test_point_resource->tpoint_->y_);
+}
+
+
+THREADED_TEST(ExternalResource) {
+ int dispose_count = 0;
+ TestPoint* test_point = new TestPoint(3, 4);
+ TestPointResource* test_point_resource =
+ new TestPointResource(test_point, &dispose_count);
+ {
+ v8::HandleScope scope;
+ LocalContext env;
+ Local<ObjectTemplate> resource_template = ObjectTemplate::New();
+ resource_template->SetHasExternalResource(true);
+ resource_template->SetAccessor(String::New("x"), GetTestPointX);
+ resource_template->SetAccessor(String::New("y"), GetTestPointY);
+ CHECK(resource_template->HasExternalResource());
+ Local<Object> resource_obj = resource_template->NewInstance();
+ resource_obj->SetExternalResource(test_point_resource);
+ CHECK_EQ(test_point_resource,
+ static_cast<TestPointResource*>(resource_obj->GetExternalResource()));
+ // There was some problem with InternalFieldCount() in an erlier version.
+ CHECK_EQ(0, resource_obj->InternalFieldCount());
+ CHECK_EQ(test_point->x_, resource_obj->Get(String::New("x"))->Int32Value());
+ CHECK_EQ(test_point->y_, resource_obj->Get(String::New("y"))->Int32Value());
+ HEAP->CollectAllGarbage(i::Heap::kNoGCFlags);
+ CHECK_EQ(0, dispose_count);
+ }
+ HEAP->CollectAllAvailableGarbage();
+ CHECK_EQ(1, dispose_count);
+}
+
+
+THREADED_TEST(ScavengeExternalResource) {
+ int dispose_count = 0;
+ TestPoint* test_point = new TestPoint(3, 4);
+ TestPointResource* test_point_resource =
+ new TestPointResource(test_point, &dispose_count);
+ bool in_new_space;
+ {
+ v8::HandleScope scope;
+ LocalContext env;
+ Local<ObjectTemplate> resource_template = ObjectTemplate::New();
+ resource_template->SetHasExternalResource(true);
+ resource_template->SetAccessor(String::New("x"), GetTestPointX);
+ resource_template->SetAccessor(String::New("y"), GetTestPointY);
+ CHECK(resource_template->HasExternalResource());
+ Local<Object> resource_obj = resource_template->NewInstance();
+ resource_obj->SetExternalResource(test_point_resource);
+ i::Handle<i::JSObject> iobject = v8::Utils::OpenHandle(*resource_obj);
+ HEAP->CollectGarbage(i::NEW_SPACE);
+ in_new_space = HEAP->InNewSpace(*iobject);
+ CHECK(in_new_space || HEAP->old_data_space()->Contains(*iobject));
+ CHECK_EQ(0, dispose_count);
+ }
+ HEAP->CollectGarbage(in_new_space ? i::NEW_SPACE : i::OLD_DATA_SPACE);
+ CHECK_EQ(1, dispose_count);
+}
+
+
+class TestPointResourceWithDisposeControl: public TestPointResource {
+ public:
+ // Only used by non-threaded tests, so it can use static fields.
+ static int dispose_calls;
+ static int dispose_count;
+
+ TestPointResourceWithDisposeControl(TestPoint* tpoint, bool dispose)
+ : TestPointResource(tpoint, &dispose_count),
+ dispose_(dispose) { }
+
+ void Dispose() {
+ ++dispose_calls;
+ if (dispose_) delete this;
+ }
+
+ private:
+ bool dispose_;
+};
+
+
+int TestPointResourceWithDisposeControl::dispose_count = 0;
+int TestPointResourceWithDisposeControl::dispose_calls = 0;
+
+
+TEST(ExternalResourceWithDisposeHandling) {
+ TestPoint* test_point_one = new TestPoint(3, 4);
+
+ // Use a stack allocated external resource allocated object.
+ TestPointResourceWithDisposeControl::dispose_count = 0;
+ TestPointResourceWithDisposeControl::dispose_calls = 0;
+ TestPointResourceWithDisposeControl res_stack(test_point_one, false);
+ {
+ v8::HandleScope scope;
+ LocalContext env;
+ Local<ObjectTemplate> resource_template = ObjectTemplate::New();
+ resource_template->SetHasExternalResource(true);
+ resource_template->SetAccessor(String::New("x"), GetTestPointX);
+ resource_template->SetAccessor(String::New("y"), GetTestPointY);
+ CHECK(resource_template->HasExternalResource());
+ Local<Object> resource_obj = resource_template->NewInstance();
+ resource_obj->SetExternalResource(&res_stack);
+
+ HEAP->CollectAllAvailableGarbage();
+ CHECK_EQ(0, TestPointResourceWithDisposeControl::dispose_count);
+ }
+ HEAP->CollectAllAvailableGarbage();
+ CHECK_EQ(1, TestPointResourceWithDisposeControl::dispose_calls);
+ CHECK_EQ(0, TestPointResourceWithDisposeControl::dispose_count);
+
+ // Use a heap allocated external resource allocated object.
+ TestPointResourceWithDisposeControl::dispose_count = 0;
+ TestPointResourceWithDisposeControl::dispose_calls = 0;
+ TestPoint* test_point_two = new TestPoint(5, 6);
+ TestPointResource* res_heap =
+ new TestPointResourceWithDisposeControl(test_point_two, true);
+ {
+ v8::HandleScope scope;
+ LocalContext env;
+ Local<ObjectTemplate> resource_template = ObjectTemplate::New();
+ resource_template->SetHasExternalResource(true);
+ resource_template->SetAccessor(String::New("x"), GetTestPointX);
+ resource_template->SetAccessor(String::New("y"), GetTestPointY);
+ CHECK(resource_template->HasExternalResource());
+ Local<Object> resource_obj = resource_template->NewInstance();
+ resource_obj->SetExternalResource(res_heap);
+
+ HEAP->CollectAllAvailableGarbage();
+ CHECK_EQ(0, TestPointResourceWithDisposeControl::dispose_count);
+ }
+ HEAP->CollectAllAvailableGarbage();
+ CHECK_EQ(1, TestPointResourceWithDisposeControl::dispose_calls);
+ CHECK_EQ(1, TestPointResourceWithDisposeControl::dispose_count);
+}
+
+
THREADED_TEST(StringConcat) {
{
v8::HandleScope scope;
_______________________________________________
Qt-script mailing list
[email protected]
http://lists.qt.nokia.com/mailman/listinfo/qt-script
