Use locker when creating snapshot if necessary.

src/api.cc

Index: src/api.cc
diff --git a/src/api.cc b/src/api.cc
index 38d108d785f3c43e4031a43d4b2aefadedccb2cf..f7a236275aa9fc663b2eb2136789446f472a9bc9 100644
--- a/src/api.cc
+++ b/src/api.cc
@@ -322,51 +322,61 @@ bool RunExtraCode(Isolate* isolate, const char* utf8_source) {
 }
 
 
-StartupData V8::CreateSnapshotDataBlob(const char* custom_source) {
-  i::Isolate* internal_isolate = new i::Isolate(true);
-  Isolate* isolate = reinterpret_cast<Isolate*>(internal_isolate);
-  StartupData result = {NULL, 0};
+StartupData CreateSnapshotWithinIsolate(Isolate* isolate,
+                                        const char* custom_source) {
+  i::Isolate* i_isolate = reinterpret_cast<i::Isolate*>(isolate);
+  Isolate::Scope isolate_scope(isolate);
+  i_isolate->Init(NULL);
+  Persistent<Context> context;
+  i::Snapshot::Metadata metadata;
   {
-    Isolate::Scope isolate_scope(isolate);
-    internal_isolate->Init(NULL);
-    Persistent<Context> context;
-    i::Snapshot::Metadata metadata;
-    {
-      HandleScope handle_scope(isolate);
-      Handle<Context> new_context = Context::New(isolate);
-      context.Reset(isolate, new_context);
-      if (custom_source != NULL) {
-        metadata.set_embeds_script(true);
-        Context::Scope context_scope(new_context);
-        if (!RunExtraCode(isolate, custom_source)) context.Reset();
-      }
+    HandleScope handle_scope(isolate);
+    Handle<Context> new_context = Context::New(isolate);
+    context.Reset(isolate, new_context);
+    if (custom_source != NULL) {
+      metadata.set_embeds_script(true);
+      Context::Scope context_scope(new_context);
+      if (!RunExtraCode(isolate, custom_source)) context.Reset();
     }
-    if (!context.IsEmpty()) {
-      // Make sure all builtin scripts are cached.
-      {
-        HandleScope scope(isolate);
-        for (int i = 0; i < i::Natives::GetBuiltinsCount(); i++) {
-          internal_isolate->bootstrapper()->NativesSourceLookup(i);
-        }
-      }
-      // If we don't do this then we end up with a stray root pointing at the
-      // context even after we have disposed of the context.
-      internal_isolate->heap()->CollectAllAvailableGarbage("mksnapshot");
-      i::Object* raw_context = *v8::Utils::OpenPersistent(context);
-      context.Reset();
-
-      i::SnapshotByteSink snapshot_sink;
-      i::StartupSerializer ser(internal_isolate, &snapshot_sink);
-      ser.SerializeStrongReferences();
-
-      i::SnapshotByteSink context_sink;
-      i::PartialSerializer context_ser(internal_isolate, &ser, &context_sink);
-      context_ser.Serialize(&raw_context);
-      ser.SerializeWeakReferences();
-
-      result = i::Snapshot::CreateSnapshotBlob(ser, context_ser, metadata);
+  }
+
+  if (context.IsEmpty()) return {NULL, 0};
+
+  // Make sure all builtin scripts are cached.
+  {
+    HandleScope scope(isolate);
+    for (int i = 0; i < i::Natives::GetBuiltinsCount(); i++) {
+      i_isolate->bootstrapper()->NativesSourceLookup(i);
     }
   }
+  // If we don't do this then we end up with a stray root pointing at the
+  // context even after we have disposed of the context.
+  i_isolate->heap()->CollectAllAvailableGarbage("CreateSnapshotDataBlob");
+  i::Object* raw_context = *v8::Utils::OpenPersistent(context);
+  context.Reset();
+
+  i::SnapshotByteSink snapshot_sink;
+  i::StartupSerializer ser(i_isolate, &snapshot_sink);
+  ser.SerializeStrongReferences();
+
+  i::SnapshotByteSink context_sink;
+  i::PartialSerializer context_ser(i_isolate, &ser, &context_sink);
+  context_ser.Serialize(&raw_context);
+  ser.SerializeWeakReferences();
+
+  return i::Snapshot::CreateSnapshotBlob(ser, context_ser, metadata);
+}
+
+
+StartupData V8::CreateSnapshotDataBlob(const char* custom_source) {
+  Isolate* isolate = reinterpret_cast<Isolate*>(new i::Isolate(true));
+  StartupData result = {NULL, 0};
+  if (Locker::IsActive()) {
+    Locker lock(isolate);
+    result = CreateSnapshotWithinIsolate(isolate, custom_source);
+  } else {
+    result = CreateSnapshotWithinIsolate(isolate, custom_source);

[vogelheim, 2015/03/03 13:18:29]

I'm confused by this. Admittedly I don't understand how the locker is supposed
to work and I'm mostly pattern-matching the code review, so quite possibly the
confusion is on my part...

I take it the Locker is meant to be used be the embedder. For all I can see,
this is the only case in the API where the API instantiates a Locker itself.
Also, I can't find the pattern of Locker-ing things if Locker::IsActive
elsewhere. Why is this the right thing to do here? Is is because we also create
our own Isolate here?

+  }
   isolate->Dispose();
   return result;
 }