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

@@ -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<char*>(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<char*>(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<String> Bootstrapper::NativesSourceLookup(int index) {
@@ -51,9 +33,7 @@ Handle<String> Bootstrapper::NativesSourceLookup(int index) {
     // We can use external strings for the natives.
     Vector<const char> 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<String> 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<char*>(2);
+void Bootstrapper::TearDown() {
+  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<const NativesExternalStringResource*>(
+                ExternalOneByteString::cast(natives_source)->resource());
+        delete resource;
       }
-    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;
-    }
-    delete delete_these_arrays_on_tear_down_;
-    delete_these_arrays_on_tear_down_ = NULL;
   }
 
   extensions_cache_.Initialize(isolate_, false);  // Yes, symmetrical

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<Context> 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<char*>* delete_these_non_arrays_on_tear_down_;
-  // This is for delete[]
-  List<char*>* 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_; }
 

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<const char> 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<Object*>(resource);
         UnalignedCopy(current++, &resource_obj);