From 0f6709330c5bf51492aedd99a8729817bf62ced4 Mon Sep 17 00:00:00 2001 From: "cira@chromium.org" Date: Fri, 4 Mar 2011 17:22:03 +0000 Subject: [PATCH] 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 --- src/extensions/experimental/break-iterator.cc | 32 ++++++++++++++++++++++----- src/extensions/experimental/break-iterator.h | 6 +++++ 2 files changed, 32 insertions(+), 6 deletions(-) diff --git a/src/extensions/experimental/break-iterator.cc b/src/extensions/experimental/break-iterator.cc index ea21867..6f574d4 100644 --- a/src/extensions/experimental/break-iterator.cc +++ b/src/extensions/experimental/break-iterator.cc @@ -46,6 +46,23 @@ icu::BreakIterator* BreakIterator::UnpackBreakIterator( return NULL; } +UnicodeString* BreakIterator::ResetAdoptedText( + v8::Handle obj, v8::Handle value) { + // Get the previous value from the internal field. + UnicodeString* text = static_cast( + obj->GetPointerFromInternalField(1)); + delete text; + + // Assign new value to the internal pointer. + v8::String::Value text_value(value); + text = new UnicodeString( + reinterpret_cast(*text_value), text_value.length()); + obj->SetPointerInInternalField(1, text); + + // Return new unicode string pointer. + return text; +} + void BreakIterator::DeleteBreakIterator(v8::Persistent object, void* param) { v8::Persistent persistent_object = @@ -57,6 +74,9 @@ void BreakIterator::DeleteBreakIterator(v8::Persistent object, // pointing to a break iterator. delete UnpackBreakIterator(persistent_object); + delete static_cast( + persistent_object->GetPointerFromInternalField(1)); + // Then dispose of the persistent handle to JS object. persistent_object.Dispose(); } @@ -81,11 +101,7 @@ v8::Handle BreakIterator::BreakIteratorAdoptText( return ThrowUnexpectedObjectError(); } - v8::String::Value text_value(args[0]); - UnicodeString text( - reinterpret_cast(*text_value), text_value.length()); - - break_iterator->setText(text); + break_iterator->setText(*ResetAdoptedText(args.Holder(), args[0])); return v8::Undefined(); } @@ -192,7 +208,9 @@ v8::Handle BreakIterator::JSBreakIterator( // Define internal field count on instance template. v8::Local 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 proto = raw_template->PrototypeTemplate(); @@ -219,6 +237,8 @@ v8::Handle BreakIterator::JSBreakIterator( // 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); diff --git a/src/extensions/experimental/break-iterator.h b/src/extensions/experimental/break-iterator.h index 318b153..473bc89 100644 --- a/src/extensions/experimental/break-iterator.h +++ b/src/extensions/experimental/break-iterator.h @@ -34,6 +34,7 @@ namespace U_ICU_NAMESPACE { class BreakIterator; +class UnicodeString; } namespace v8 { @@ -48,6 +49,11 @@ class BreakIterator { // Unpacks break iterator object from corresponding JavaScript object. static icu::BreakIterator* UnpackBreakIterator(v8::Handle obj); + // Deletes the old value and sets the adopted text in + // corresponding JavaScript object. + static UnicodeString* ResetAdoptedText(v8::Handle obj, + v8::Handle 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 object, -- 2.7.4