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

git-svn-id: http://v8.googlecode.com/svn/branches/bleeding_edge@7063 ce2b1a6d-e550-0410-aec6-3dcde31c8c00
This commit is contained in:
cira@chromium.org 2011-03-04 17:22:03 +00:00
parent 76a6c73960
commit 0f6709330c
2 changed files with 32 additions and 6 deletions

View File

@ -46,6 +46,23 @@ icu::BreakIterator* BreakIterator::UnpackBreakIterator(
return NULL; 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 BreakIterator::DeleteBreakIterator(v8::Persistent<v8::Value> object,
void* param) { void* param) {
v8::Persistent<v8::Object> persistent_object = v8::Persistent<v8::Object> persistent_object =
@ -57,6 +74,9 @@ void BreakIterator::DeleteBreakIterator(v8::Persistent<v8::Value> object,
// pointing to a break iterator. // pointing to a break iterator.
delete UnpackBreakIterator(persistent_object); delete UnpackBreakIterator(persistent_object);
delete static_cast<UnicodeString*>(
persistent_object->GetPointerFromInternalField(1));
// Then dispose of the persistent handle to JS object. // Then dispose of the persistent handle to JS object.
persistent_object.Dispose(); persistent_object.Dispose();
} }
@ -81,11 +101,7 @@ v8::Handle<v8::Value> BreakIterator::BreakIteratorAdoptText(
return ThrowUnexpectedObjectError(); return ThrowUnexpectedObjectError();
} }
v8::String::Value text_value(args[0]); break_iterator->setText(*ResetAdoptedText(args.Holder(), args[0]));
UnicodeString text(
reinterpret_cast<const UChar*>(*text_value), text_value.length());
break_iterator->setText(text);
return v8::Undefined(); return v8::Undefined();
} }
@ -192,7 +208,9 @@ v8::Handle<v8::Value> BreakIterator::JSBreakIterator(
// Define internal field count on instance template. // Define internal field count on instance template.
v8::Local<v8::ObjectTemplate> object_template = v8::Local<v8::ObjectTemplate> object_template =
raw_template->InstanceTemplate(); 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. // Define all of the prototype methods on prototype template.
v8::Local<v8::ObjectTemplate> proto = raw_template->PrototypeTemplate(); v8::Local<v8::ObjectTemplate> proto = raw_template->PrototypeTemplate();
@ -219,6 +237,8 @@ v8::Handle<v8::Value> BreakIterator::JSBreakIterator(
// Set break iterator as internal field of the resulting JS object. // Set break iterator as internal field of the resulting JS object.
wrapper->SetPointerInInternalField(0, break_iterator); 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. // Make object handle weak so we can delete iterator once GC kicks in.
wrapper.MakeWeak(NULL, DeleteBreakIterator); wrapper.MakeWeak(NULL, DeleteBreakIterator);

View File

@ -34,6 +34,7 @@
namespace U_ICU_NAMESPACE { namespace U_ICU_NAMESPACE {
class BreakIterator; class BreakIterator;
class UnicodeString;
} }
namespace v8 { namespace v8 {
@ -48,6 +49,11 @@ class BreakIterator {
// Unpacks break iterator object from corresponding JavaScript object. // Unpacks break iterator object from corresponding JavaScript object.
static icu::BreakIterator* UnpackBreakIterator(v8::Handle<v8::Object> obj); 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 // Release memory we allocated for the BreakIterator once the JS object that
// holds the pointer gets garbage collected. // holds the pointer gets garbage collected.
static void DeleteBreakIterator(v8::Persistent<v8::Value> object, static void DeleteBreakIterator(v8::Persistent<v8::Value> object,