[d8 Workers] Fix bug creating Worker during main thread termination

src/d8.cc

 // Copyright 2012 the V8 project authors. All rights reserved.
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
 
 // Defined when linking against shared lib on Windows.
 #if defined(USING_V8_SHARED) && !defined(V8_SHARED)
 #define V8_SHARED
 #endif
 
@@ skipping 713 matching lines @@
 void Shell::WorkerNew(const v8::FunctionCallbackInfo<v8::Value>& args) {
   Isolate* isolate = args.GetIsolate();
   HandleScope handle_scope(isolate);
   if (args.Length() < 1 || !args[0]->IsString()) {
     Throw(args.GetIsolate(), "1st argument must be string");
     return;
   }
 
   {
     base::LockGuard<base::Mutex> lock_guard(workers_mutex_.Pointer());
+    // Intialize the internal field to NULL; if we return early without
+    // creating a new Worker (because the main thread is terminating) we can
+    // early-out from the instance calls.
+    args.Holder()->SetAlignedPointerInInternalField(0, NULL);
+
     if (!allow_new_workers_) return;
 
     Worker* worker = new Worker;
-    args.This()->SetInternalField(0, External::New(isolate, worker));
+    args.Holder()->SetAlignedPointerInInternalField(0, worker);
     workers_.Add(worker);
 
     String::Utf8Value script(args[0]);
     if (!*script) {
       Throw(args.GetIsolate(), "Can't get worker script");
       return;
     }
     worker->StartExecuteInThread(isolate, *script);
   }
 }
 
 
 void Shell::WorkerPostMessage(const v8::FunctionCallbackInfo<v8::Value>& args) {
   Isolate* isolate = args.GetIsolate();
   HandleScope handle_scope(isolate);
   Local<Context> context = isolate->GetCurrentContext();
-  Local<Value> this_value;
 
   if (args.Length() < 1) {
     Throw(isolate, "Invalid argument");
     return;
   }
 
-  if (args.This()->InternalFieldCount() > 0) {
-    this_value = args.This()->GetInternalField(0);
-  }
-  if (this_value.IsEmpty()) {
+  if (args.Holder()->InternalFieldCount() <= 0) {
     Throw(isolate, "this is not a Worker");
     return;
   }
 
-  Worker* worker =
-      static_cast<Worker*>(Local<External>::Cast(this_value)->Value());
+  Worker* worker = static_cast<Worker*>(
+      args.Holder()->GetAlignedPointerFromInternalField(0));
+  if (worker == NULL) {
+    Throw(isolate, "Worker is defunct because main thread is terminating");
+    return;
+  }
 
   Local<Value> message = args[0];
   ObjectList to_transfer;
   if (args.Length() >= 2) {
     if (!args[1]->IsArray()) {
       Throw(isolate, "Transfer list must be an Array");
       return;
     }
 
     Local<Array> transfer = Local<Array>::Cast(args[1]);
@@ skipping 19 matching lines @@
     worker->PostMessage(data);
   } else {
     delete data;
   }
 }
 
 
 void Shell::WorkerGetMessage(const v8::FunctionCallbackInfo<v8::Value>& args) {
   Isolate* isolate = args.GetIsolate();
   HandleScope handle_scope(isolate);
-  Local<Value> this_value;
-  if (args.This()->InternalFieldCount() > 0) {
-    this_value = args.This()->GetInternalField(0);
-  }
-  if (this_value.IsEmpty()) {
+  if (args.Holder()->InternalFieldCount() <= 0) {
     Throw(isolate, "this is not a Worker");
     return;
   }
 
-  Worker* worker =
-      static_cast<Worker*>(Local<External>::Cast(this_value)->Value());
+  Worker* worker = static_cast<Worker*>(
+      args.Holder()->GetAlignedPointerFromInternalField(0));
+  if (worker == NULL) {
+    Throw(isolate, "Worker is defunct because main thread is terminating");
+    return;
+  }
 
   SerializationData* data = worker->GetMessage();
   if (data) {
     int offset = 0;
     Local<Value> data_value;
     if (Shell::DeserializeValue(isolate, *data, &offset).ToLocal(&data_value)) {
       args.GetReturnValue().Set(data_value);
     }
     delete data;
   }
 }
 
 
 void Shell::WorkerTerminate(const v8::FunctionCallbackInfo<v8::Value>& args) {
   Isolate* isolate = args.GetIsolate();
   HandleScope handle_scope(isolate);
-  Local<Value> this_value;
-  if (args.This()->InternalFieldCount() > 0) {
-    this_value = args.This()->GetInternalField(0);
-  }
-  if (this_value.IsEmpty()) {
+  if (args.Holder()->InternalFieldCount() <= 0) {
     Throw(isolate, "this is not a Worker");
     return;
   }
 
-  Worker* worker =
-      static_cast<Worker*>(Local<External>::Cast(this_value)->Value());
+  Worker* worker = static_cast<Worker*>(
+      args.Holder()->GetAlignedPointerFromInternalField(0));
+  if (worker == NULL) {
+    Throw(isolate, "Worker is defunct because main thread is terminating");
+    return;
+  }
+
   worker->Terminate();
 }
 #endif  // !V8_SHARED
 
 
 void Shell::QuitOnce(v8::FunctionCallbackInfo<v8::Value>* args) {
   int exit_code = (*args)[0]
                       ->Int32Value(args->GetIsolate()->GetCurrentContext())
                       .FromMaybe(0);
 #ifndef V8_SHARED
@@ skipping 394 matching lines @@
       String::NewFromUtf8(isolate, "now", NewStringType::kNormal)
           .ToLocalChecked(),
       FunctionTemplate::New(isolate, PerformanceNow));
   global_template->Set(
       String::NewFromUtf8(isolate, "performance", NewStringType::kNormal)
           .ToLocalChecked(),
       performance_template);
 
   Local<FunctionTemplate> worker_fun_template =
       FunctionTemplate::New(isolate, WorkerNew);
+  Local<Signature> worker_signature =
+      Signature::New(isolate, worker_fun_template);
+  worker_fun_template->SetClassName(
+      String::NewFromUtf8(isolate, "Worker", NewStringType::kNormal)
+          .ToLocalChecked());
   worker_fun_template->PrototypeTemplate()->Set(
       String::NewFromUtf8(isolate, "terminate", NewStringType::kNormal)
           .ToLocalChecked(),
-      FunctionTemplate::New(isolate, WorkerTerminate));
+      FunctionTemplate::New(isolate, WorkerTerminate, Local<Value>(),
+                            worker_signature));
   worker_fun_template->PrototypeTemplate()->Set(
       String::NewFromUtf8(isolate, "postMessage", NewStringType::kNormal)
           .ToLocalChecked(),
-      FunctionTemplate::New(isolate, WorkerPostMessage));
+      FunctionTemplate::New(isolate, WorkerPostMessage, Local<Value>(),
+                            worker_signature));
   worker_fun_template->PrototypeTemplate()->Set(
       String::NewFromUtf8(isolate, "getMessage", NewStringType::kNormal)
           .ToLocalChecked(),
-      FunctionTemplate::New(isolate, WorkerGetMessage));
+      FunctionTemplate::New(isolate, WorkerGetMessage, Local<Value>(),
+                            worker_signature));
   worker_fun_template->InstanceTemplate()->SetInternalFieldCount(1);
   global_template->Set(
       String::NewFromUtf8(isolate, "Worker", NewStringType::kNormal)
           .ToLocalChecked(),
       worker_fun_template);
 #endif  // !V8_SHARED
 
   Local<ObjectTemplate> os_templ = ObjectTemplate::New(isolate);
   AddOSMethods(isolate, os_templ);
   global_template->Set(
@@ skipping 491 matching lines @@
   }
   data_.Clear();
 }
 
 
 Worker::Worker()
     : in_semaphore_(0),
       out_semaphore_(0),
       thread_(NULL),
       script_(NULL),
-      state_(IDLE),
-      join_called_(false) {}
+      state_(IDLE) {}
 
 
 Worker::~Worker() {
   delete thread_;
   thread_ = NULL;
   delete[] script_;
   script_ = NULL;
   in_queue_.Clear();
   out_queue_.Clear();
 }
 
 
 void Worker::StartExecuteInThread(Isolate* isolate, const char* script) {
-  if (base::NoBarrier_CompareAndSwap(&state_, IDLE, RUNNING) == IDLE) {
-    script_ = i::StrDup(script);
-    thread_ = new WorkerThread(this);
-    thread_->Start();
-  } else {
-    // Somehow the Worker was started twice.
-    UNREACHABLE();
-  }
+  DCHECK(state_ == IDLE);
+  state_ = RUNNING;
+
+  script_ = i::StrDup(script);
+  thread_ = new WorkerThread(this);
+  thread_->Start();
 }
 
 
 void Worker::PostMessage(SerializationData* data) {
   in_queue_.Enqueue(data);
   in_semaphore_.Signal();
 }
 
 
 SerializationData* Worker::GetMessage() {
   SerializationData* data = NULL;
   while (!out_queue_.Dequeue(&data)) {
     // If the worker is no longer running, and there are no messages in the
     // queue, don't expect any more messages from it.
-    if (base::NoBarrier_Load(&state_) != RUNNING) break;
+    if (state_ != RUNNING) break;
     out_semaphore_.Wait();
   }
   return data;
 }
 
 
 void Worker::Terminate() {
-  if (base::NoBarrier_CompareAndSwap(&state_, RUNNING, TERMINATED) == RUNNING) {
+  if (state_ == RUNNING) {
+    state_ = TERMINATED;

[Jarin, 2015/07/23 07:48:41]

What if we call quit() from one worker and worker.terminate() from another one?
Cannot there be a data race?

[binji, 2015/07/24 18:52:29]

Yes, you're right. I've removed the state_ (because it was mostly for
debugging), and replaced it with a simple atomic bool.

     // Post NULL to wake the Worker thread message loop, and tell it to stop
     // running.
     PostMessage(NULL);
   }
 }
 
 
 void Worker::WaitForThread() {
   Terminate();
-
-  if (base::NoBarrier_CompareAndSwap(&join_called_, false, true) == false) {
-    thread_->Join();
-  } else {
-    // Tried to call join twice.
-    UNREACHABLE();
-  }
+  thread_->Join();
 }
 
 
 void Worker::ExecuteInThread() {
   Isolate::CreateParams create_params;
   create_params.array_buffer_allocator = Shell::array_buffer_allocator;
   Isolate* isolate = Isolate::New(create_params);
   {
     Isolate::Scope iscope(isolate);
     {
@@ skipping 749 matching lines @@
 }
 
 }  // namespace v8
 
 
 #ifndef GOOGLE3
 int main(int argc, char* argv[]) {
   return v8::Shell::Main(argc, argv);
 }
 #endif