From 161efc42bf12682616ce4fba99d854935f9148f6 Mon Sep 17 00:00:00 2001 From: yangguo Date: Mon, 9 Feb 2015 01:04:45 -0800 Subject: [PATCH] Correctly clean up natives sources on tear down. R=ulan@chromium.org Review URL: https://codereview.chromium.org/911543002 Cr-Commit-Position: refs/heads/master@{#26516} --- src/bootstrapper.cc | 66 +++++++++++------------------------------------------ src/bootstrapper.h | 13 ++--------- src/serialize.cc | 4 ++-- 3 files changed, 17 insertions(+), 66 deletions(-) diff --git a/src/bootstrapper.cc b/src/bootstrapper.cc index abe6dcc..63223fb 100644 --- a/src/bootstrapper.cc +++ b/src/bootstrapper.cc @@ -20,28 +20,10 @@ namespace v8 { namespace internal { -NativesExternalStringResource::NativesExternalStringResource( - Bootstrapper* bootstrapper, - const char* source, - size_t length) - : data_(source), length_(length) { - if (bootstrapper->delete_these_non_arrays_on_tear_down_ == NULL) { - bootstrapper->delete_these_non_arrays_on_tear_down_ = new List(2); - } - // The resources are small objects and we only make a fixed number of - // them, but let's clean them up on exit for neatness. - bootstrapper->delete_these_non_arrays_on_tear_down_-> - Add(reinterpret_cast(this)); -} - - Bootstrapper::Bootstrapper(Isolate* isolate) : isolate_(isolate), nesting_(0), - extensions_cache_(Script::TYPE_EXTENSION), - delete_these_non_arrays_on_tear_down_(NULL), - delete_these_arrays_on_tear_down_(NULL) { -} + extensions_cache_(Script::TYPE_EXTENSION) {} Handle Bootstrapper::NativesSourceLookup(int index) { @@ -51,9 +33,7 @@ Handle Bootstrapper::NativesSourceLookup(int index) { // We can use external strings for the natives. Vector source = Natives::GetScriptSource(index); NativesExternalStringResource* resource = - new NativesExternalStringResource(this, - source.start(), - source.length()); + new NativesExternalStringResource(source.start(), source.length()); // We do not expect this to throw an exception. Change this if it does. Handle source_code = isolate_->factory() ->NewExternalStringFromOneByte(resource) @@ -114,39 +94,19 @@ void Bootstrapper::TearDownExtensions() { } -char* Bootstrapper::AllocateAutoDeletedArray(int bytes) { - char* memory = new char[bytes]; - if (memory != NULL) { - if (delete_these_arrays_on_tear_down_ == NULL) { - delete_these_arrays_on_tear_down_ = new List(2); - } - delete_these_arrays_on_tear_down_->Add(memory); - } - return memory; -} - - void Bootstrapper::TearDown() { - if (delete_these_non_arrays_on_tear_down_ != NULL) { - int len = delete_these_non_arrays_on_tear_down_->length(); - DCHECK(len < 1000); // Don't use this mechanism for unbounded allocations. - for (int i = 0; i < len; i++) { - delete delete_these_non_arrays_on_tear_down_->at(i); - delete_these_non_arrays_on_tear_down_->at(i) = NULL; - } - delete delete_these_non_arrays_on_tear_down_; - delete_these_non_arrays_on_tear_down_ = NULL; - } - - if (delete_these_arrays_on_tear_down_ != NULL) { - int len = delete_these_arrays_on_tear_down_->length(); - DCHECK(len < 1000); // Don't use this mechanism for unbounded allocations. - for (int i = 0; i < len; i++) { - delete[] delete_these_arrays_on_tear_down_->at(i); - delete_these_arrays_on_tear_down_->at(i) = NULL; + Object* natives_source_cache = isolate_->heap()->natives_source_cache(); + if (natives_source_cache->IsFixedArray()) { + FixedArray* natives_source_array = FixedArray::cast(natives_source_cache); + for (int i = 0; i < Natives::GetBuiltinsCount(); i++) { + Object* natives_source = natives_source_array->get(i); + if (!natives_source->IsUndefined()) { + const NativesExternalStringResource* resource = + reinterpret_cast( + ExternalOneByteString::cast(natives_source)->resource()); + delete resource; + } } - delete delete_these_arrays_on_tear_down_; - delete_these_arrays_on_tear_down_ = NULL; } extensions_cache_.Initialize(isolate_, false); // Yes, symmetrical diff --git a/src/bootstrapper.h b/src/bootstrapper.h index 9d4f270..4bf74b3 100644 --- a/src/bootstrapper.h +++ b/src/bootstrapper.h @@ -98,10 +98,6 @@ class Bootstrapper FINAL { char* RestoreState(char* from); void FreeThreadResources(); - // This will allocate a char array that is deleted when V8 is shut down. - // It should only be used for strictly finite allocations. - char* AllocateAutoDeletedArray(int bytes); - // Used for new context creation. bool InstallExtensions(Handle native_context, v8::ExtensionConfiguration* extensions); @@ -113,10 +109,6 @@ class Bootstrapper FINAL { typedef int NestingCounterType; NestingCounterType nesting_; SourceCodeCache extensions_cache_; - // This is for delete, not delete[]. - List* delete_these_non_arrays_on_tear_down_; - // This is for delete[] - List* delete_these_arrays_on_tear_down_; friend class BootstrapperActive; friend class Isolate; @@ -155,9 +147,8 @@ class BootstrapperActive FINAL BASE_EMBEDDED { class NativesExternalStringResource FINAL : public v8::String::ExternalOneByteStringResource { public: - NativesExternalStringResource(Bootstrapper* bootstrapper, - const char* source, - size_t length); + NativesExternalStringResource(const char* source, size_t length) + : data_(source), length_(length) {} const char* data() const OVERRIDE { return data_; } size_t length() const OVERRIDE { return length_; } diff --git a/src/serialize.cc b/src/serialize.cc index 1423b23..673c01f 100644 --- a/src/serialize.cc +++ b/src/serialize.cc @@ -1295,11 +1295,11 @@ void Deserializer::ReadData(Object** current, Object** limit, int source_space, } case kNativesStringResource: { + DCHECK(!isolate_->heap()->deserialization_complete()); int index = source_.Get(); Vector source_vector = Natives::GetScriptSource(index); NativesExternalStringResource* resource = - new NativesExternalStringResource(isolate->bootstrapper(), - source_vector.start(), + new NativesExternalStringResource(source_vector.start(), source_vector.length()); Object* resource_obj = reinterpret_cast(resource); UnalignedCopy(current++, &resource_obj); -- 2.7.4