Commit a87db3de authored by binji's avatar binji Committed by Commit bot

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

When the main thread terminates, it forcibly terminates all Worker threads.
When this happens, the threads objects were only half-created; they had a
JavaScript Worker object, but not a C++ worker object.

This CL fixes that bug, as well as some other fixes:
* Signatures on Worker methods
* Use SetAlignedPointerFromInternalField instead of using an External.
* Remove state_ from Worker. Simplify to atomic bool running_.

BUG=chromium:511880
R=jarin@chromium.org
LOG=n

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

Cr-Commit-Position: refs/heads/master@{#29911}
parent 597da503
......@@ -105,6 +105,13 @@ class MockArrayBufferAllocator : public v8::ArrayBuffer::Allocator {
v8::Platform* g_platform = NULL;
static Local<Value> Throw(Isolate* isolate, const char* message) {
return isolate->ThrowException(
String::NewFromUtf8(isolate, message, NewStringType::kNormal)
.ToLocalChecked());
}
#ifndef V8_SHARED
bool FindInObjectList(Local<Object> object, const Shell::ObjectList& list) {
for (int i = 0; i < list.length(); ++i) {
......@@ -114,19 +121,28 @@ bool FindInObjectList(Local<Object> object, const Shell::ObjectList& list) {
}
return false;
}
#endif // !V8_SHARED
} // namespace
Worker* GetWorkerFromInternalField(Isolate* isolate, Local<Object> object) {
if (object->InternalFieldCount() != 1) {
Throw(isolate, "this is not a Worker");
return NULL;
}
Worker* worker =
static_cast<Worker*>(object->GetAlignedPointerFromInternalField(0));
if (worker == NULL) {
Throw(isolate, "Worker is defunct because main thread is terminating");
return NULL;
}
static Local<Value> Throw(Isolate* isolate, const char* message) {
return isolate->ThrowException(
String::NewFromUtf8(isolate, message, NewStringType::kNormal)
.ToLocalChecked());
return worker;
}
#endif // !V8_SHARED
} // namespace
class PerIsolateData {
public:
......@@ -686,10 +702,15 @@ void Shell::WorkerNew(const v8::FunctionCallbackInfo<v8::Value>& args) {
{
base::LockGuard<base::Mutex> lock_guard(workers_mutex_.Pointer());
// Initialize 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]);
......@@ -697,7 +718,7 @@ void Shell::WorkerNew(const v8::FunctionCallbackInfo<v8::Value>& args) {
Throw(args.GetIsolate(), "Can't get worker script");
return;
}
worker->StartExecuteInThread(isolate, *script);
worker->StartExecuteInThread(*script);
}
}
......@@ -706,24 +727,17 @@ 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()) {
Throw(isolate, "this is not a Worker");
Worker* worker = GetWorkerFromInternalField(isolate, args.Holder());
if (!worker) {
return;
}
Worker* worker =
static_cast<Worker*>(Local<External>::Cast(this_value)->Value());
Local<Value> message = args[0];
ObjectList to_transfer;
if (args.Length() >= 2) {
......@@ -762,18 +776,11 @@ void Shell::WorkerPostMessage(const v8::FunctionCallbackInfo<v8::Value>& args) {
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()) {
Throw(isolate, "this is not a Worker");
Worker* worker = GetWorkerFromInternalField(isolate, args.Holder());
if (!worker) {
return;
}
Worker* worker =
static_cast<Worker*>(Local<External>::Cast(this_value)->Value());
SerializationData* data = worker->GetMessage();
if (data) {
int offset = 0;
......@@ -789,17 +796,11 @@ void Shell::WorkerGetMessage(const v8::FunctionCallbackInfo<v8::Value>& args) {
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()) {
Throw(isolate, "this is not a Worker");
Worker* worker = GetWorkerFromInternalField(isolate, args.Holder());
if (!worker) {
return;
}
Worker* worker =
static_cast<Worker*>(Local<External>::Cast(this_value)->Value());
worker->Terminate();
}
#endif // !V8_SHARED
......@@ -1134,18 +1135,26 @@ Local<ObjectTemplate> Shell::CreateGlobalTemplate(Isolate* isolate) {
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)
......@@ -1644,8 +1653,7 @@ Worker::Worker()
out_semaphore_(0),
thread_(NULL),
script_(NULL),
state_(IDLE),
join_called_(false) {}
running_(false) {}
Worker::~Worker() {
......@@ -1658,15 +1666,11 @@ Worker::~Worker() {
}
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();
}
void Worker::StartExecuteInThread(const char* script) {
running_ = true;
script_ = i::StrDup(script);
thread_ = new WorkerThread(this);
thread_->Start();
}
......@@ -1681,7 +1685,7 @@ SerializationData* Worker::GetMessage() {
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 (!base::NoBarrier_Load(&running_)) break;
out_semaphore_.Wait();
}
return data;
......@@ -1689,23 +1693,16 @@ SerializationData* Worker::GetMessage() {
void Worker::Terminate() {
if (base::NoBarrier_CompareAndSwap(&state_, RUNNING, TERMINATED) == RUNNING) {
// Post NULL to wake the Worker thread message loop, and tell it to stop
// running.
PostMessage(NULL);
}
base::NoBarrier_Store(&running_, false);
// 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();
}
......
......@@ -221,19 +221,25 @@ class Worker {
Worker();
~Worker();
void StartExecuteInThread(Isolate* isolate, const char* script);
// Run the given script on this Worker. This function should only be called
// once, and should only be called by the thread that created the Worker.
void StartExecuteInThread(const char* script);
// Post a message to the worker's incoming message queue. The worker will
// take ownership of the SerializationData.
// This function should only be called by the thread that created the Worker.
void PostMessage(SerializationData* data);
// Synchronously retrieve messages from the worker's outgoing message queue.
// If there is no message in the queue, block until a message is available.
// If there are no messages in the queue and the worker is no longer running,
// return nullptr.
// This function should only be called by the thread that created the Worker.
SerializationData* GetMessage();
// Terminate the worker's event loop. Messages from the worker that have been
// queued can still be read via GetMessage().
// This function can be called by any thread.
void Terminate();
// Terminate and join the thread.
// This function can be called by any thread.
void WaitForThread();
private:
......@@ -249,12 +255,6 @@ class Worker {
Worker* worker_;
};
enum State {
IDLE, // The worker thread hasn't been started yet
RUNNING, // The worker thread is running and hasn't been terminated
TERMINATED // The worker thread has been terminated and will soon finish
};
void ExecuteInThread();
static void PostMessageOut(const v8::FunctionCallbackInfo<v8::Value>& args);
......@@ -264,8 +264,7 @@ class Worker {
SerializationDataQueue out_queue_;
base::Thread* thread_;
char* script_;
base::Atomic32 state_;
base::Atomic32 join_called_;
base::Atomic32 running_;
};
#endif // !V8_SHARED
......
// Copyright 2015 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.
if (this.Worker) {
var __v_8 =
`var __v_9 = new Worker('postMessage(42)');
onmessage = function(parentMsg) {
__v_9.postMessage(parentMsg);
};`;
var __v_9 = new Worker(__v_8);
__v_9.postMessage(9);
}
Markdown is supported
0% or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment