Fix Initialize & Dispose for external snapshot. Make sure...

Fix Initialize & Dispose for external snapshot. Make sure v8::V8::(Initialize|Dispose) can be called in any order.

This is a follow-on to crrev.com/960883003, which fixed a memory leak in this code, but uncovered another, more subtle bug:

Previously, the code expected you would v8::V8::Initialize once, and v8::V8::Dispose once. The first bug was that in this case the holder_ variable would point to deallocated memory. The second bug was that once the snapshot was disposed, there was no way to get it back on a future Initialize. These are uncovered by the InitializeAndDisposeMultiple test case.

The fix is to keep memory to the raw snapshot and to then cleanly build & destroy the tables in Initialize & Dispose. Since sometimes setNativesBlob is called just after Initialize, that situation must be handled, too.

BUG=

Review URL: https://codereview.chromium.org/974943003

Cr-Commit-Position: refs/heads/master@{#26978}

src/api.cc

@@ -5366,6 +5366,9 @@ void v8::V8::ShutdownPlatform() {
 
 bool v8::V8::Initialize() {
   i::V8::Initialize();
+#ifdef V8_USE_EXTERNAL_STARTUP_DATA
+  i::ReadNatives();
+#endif
   return true;
 }
 

src/natives-external.cc

@@ -132,9 +132,10 @@ class NativesHolder {
     DCHECK(store);
     holder_ = store;
   }
+  static bool empty() { return holder_ == NULL; }
   static void Dispose() {
-    DCHECK(holder_);
     delete holder_;
+    holder_ = NULL;
   }
 
  private:
@@ -145,19 +146,36 @@ template<NativeType type>
 NativesStore* NativesHolder<type>::holder_ = NULL;
 
 
+// The natives blob. Memory is owned by caller.
+static StartupData* natives_blob_ = NULL;
+
+
+/**
+ * Read the Natives blob, as previously set by SetNativesFromFile.
+ */
+void ReadNatives() {
+  if (natives_blob_ && NativesHolder<CORE>::empty()) {
+    SnapshotByteSource bytes(natives_blob_->data, natives_blob_->raw_size);
+    NativesHolder<CORE>::set(NativesStore::MakeFromScriptsSource(&bytes));
+    NativesHolder<EXPERIMENTAL>::set(
+        NativesStore::MakeFromScriptsSource(&bytes));
+    DCHECK(!bytes.HasMore());
+  }
+}
+
+
 /**
- * Read the Natives (library sources) blob, as generated by js2c + the build
+ * Set the Natives (library sources) blob, as generated by js2c + the build
  * system.
  */
 void SetNativesFromFile(StartupData* natives_blob) {
+  DCHECK(!natives_blob_);
   DCHECK(natives_blob);
   DCHECK(natives_blob->data);
   DCHECK(natives_blob->raw_size > 0);
 
-  SnapshotByteSource bytes(natives_blob->data, natives_blob->raw_size);
-  NativesHolder<CORE>::set(NativesStore::MakeFromScriptsSource(&bytes));
-  NativesHolder<EXPERIMENTAL>::set(NativesStore::MakeFromScriptsSource(&bytes));
-  DCHECK(!bytes.HasMore());
+  natives_blob_ = natives_blob;
+  ReadNatives();
 }
 
 

src/natives.h

@@ -40,6 +40,7 @@ typedef NativesCollection<EXPERIMENTAL> ExperimentalNatives;
 #ifdef V8_USE_EXTERNAL_STARTUP_DATA
 // Used for reading the natives at runtime. Implementation in natives-empty.cc
 void SetNativesFromFile(StartupData* natives_blob);
+void ReadNatives();
 void DisposeNatives();
 #endif
 

src/snapshot-empty.cc

@@ -19,6 +19,7 @@ namespace internal {
 // below. This happens when compiling the mksnapshot utility.
 void SetNativesFromFile(StartupData* data) { CHECK(false); }
 void SetSnapshotFromFile(StartupData* data) { CHECK(false); }
+void ReadNatives() {}
 void DisposeNatives() {}
 #endif  // V8_USE_EXTERNAL_STARTUP_DATA