Revision: 7063
Author: [email protected]
Date: Fri Mar 4 09:22:03 2011
Log: Fix memory corruption with AdoptText method.
Icu setText method keeps pointer to text, it doesn't copy it so we have to
keep text around for the lifetime of the break iterator object,
or next setText operation.
Review URL: http://codereview.chromium.org/6609038
http://code.google.com/p/v8/source/detail?r=7063
Modified:
/branches/bleeding_edge/src/extensions/experimental/break-iterator.cc
/branches/bleeding_edge/src/extensions/experimental/break-iterator.h
=======================================
--- /branches/bleeding_edge/src/extensions/experimental/break-iterator.cc
Thu Mar 3 09:32:18 2011
+++ /branches/bleeding_edge/src/extensions/experimental/break-iterator.cc
Fri Mar 4 09:22:03 2011
@@ -45,6 +45,23 @@
return NULL;
}
+
+UnicodeString* BreakIterator::ResetAdoptedText(
+ v8::Handle<v8::Object> obj, v8::Handle<v8::Value> value) {
+ // Get the previous value from the internal field.
+ UnicodeString* text = static_cast<UnicodeString*>(
+ obj->GetPointerFromInternalField(1));
+ delete text;
+
+ // Assign new value to the internal pointer.
+ v8::String::Value text_value(value);
+ text = new UnicodeString(
+ reinterpret_cast<const UChar*>(*text_value), text_value.length());
+ obj->SetPointerInInternalField(1, text);
+
+ // Return new unicode string pointer.
+ return text;
+}
void BreakIterator::DeleteBreakIterator(v8::Persistent<v8::Value> object,
void* param) {
@@ -57,6 +74,9 @@
// pointing to a break iterator.
delete UnpackBreakIterator(persistent_object);
+ delete static_cast<UnicodeString*>(
+ persistent_object->GetPointerFromInternalField(1));
+
// Then dispose of the persistent handle to JS object.
persistent_object.Dispose();
}
@@ -81,11 +101,7 @@
return ThrowUnexpectedObjectError();
}
- v8::String::Value text_value(args[0]);
- UnicodeString text(
- reinterpret_cast<const UChar*>(*text_value), text_value.length());
-
- break_iterator->setText(text);
+ break_iterator->setText(*ResetAdoptedText(args.Holder(), args[0]));
return v8::Undefined();
}
@@ -192,7 +208,9 @@
// Define internal field count on instance template.
v8::Local<v8::ObjectTemplate> object_template =
raw_template->InstanceTemplate();
- object_template->SetInternalFieldCount(1);
+
+ // Set aside internal fields for icu break iterator and adopted text.
+ object_template->SetInternalFieldCount(2);
// Define all of the prototype methods on prototype template.
v8::Local<v8::ObjectTemplate> proto =
raw_template->PrototypeTemplate();
@@ -219,6 +237,8 @@
// Set break iterator as internal field of the resulting JS object.
wrapper->SetPointerInInternalField(0, break_iterator);
+ // Make sure that the pointer to adopted text is NULL.
+ wrapper->SetPointerInInternalField(1, NULL);
// Make object handle weak so we can delete iterator once GC kicks in.
wrapper.MakeWeak(NULL, DeleteBreakIterator);
=======================================
--- /branches/bleeding_edge/src/extensions/experimental/break-iterator.h
Thu Mar 3 09:32:18 2011
+++ /branches/bleeding_edge/src/extensions/experimental/break-iterator.h
Fri Mar 4 09:22:03 2011
@@ -34,6 +34,7 @@
namespace U_ICU_NAMESPACE {
class BreakIterator;
+class UnicodeString;
}
namespace v8 {
@@ -48,6 +49,11 @@
// Unpacks break iterator object from corresponding JavaScript object.
static icu::BreakIterator* UnpackBreakIterator(v8::Handle<v8::Object>
obj);
+ // Deletes the old value and sets the adopted text in
+ // corresponding JavaScript object.
+ static UnicodeString* ResetAdoptedText(v8::Handle<v8::Object> obj,
+ v8::Handle<v8::Value> text_value);
+
// Release memory we allocated for the BreakIterator once the JS object
that
// holds the pointer gets garbage collected.
static void DeleteBreakIterator(v8::Persistent<v8::Value> object,
--
v8-dev mailing list
[email protected]
http://groups.google.com/group/v8-dev
