Skip to content

Commit

Permalink
src,worker: runtime error on loop creation failure
Browse files Browse the repository at this point in the history
Instead of hard asserting throw a runtime error,
that is more consumable.
Fixes: nodejs#31614
  • Loading branch information
HarshithaKP committed Feb 17, 2020
1 parent 4c746a6 commit eee4378
Show file tree
Hide file tree
Showing 5 changed files with 48 additions and 15 deletions.
5 changes: 5 additions & 0 deletions doc/api/errors.md
Original file line number Diff line number Diff line change
Expand Up @@ -2047,6 +2047,11 @@ meaning of the error depends on the specific function.

The WASI instance has already started.

<a id="ERR_WORKER_INIT_FAILED"></a>
### `ERR_WORKER_INIT_FAILED`

The `Worker` initialization failed.

<a id="ERR_WORKER_INVALID_EXEC_ARGV"></a>
### `ERR_WORKER_INVALID_EXEC_ARGV`

Expand Down
1 change: 1 addition & 0 deletions lib/internal/errors.js
Original file line number Diff line number Diff line change
Expand Up @@ -1358,6 +1358,7 @@ E('ERR_VM_MODULE_NOT_MODULE',
'Provided module is not an instance of Module', Error);
E('ERR_VM_MODULE_STATUS', 'Module status %s', Error);
E('ERR_WASI_ALREADY_STARTED', 'WASI instance has already started', Error);
E('ERR_WORKER_INIT_FAILED', 'Worker initialization failure: %s', Error);
E('ERR_WORKER_INVALID_EXEC_ARGV', (errors, msg = 'invalid execArgv flags') =>
`Initiated Worker with ${msg}: ${errors.join(', ')}`,
Error);
Expand Down
13 changes: 9 additions & 4 deletions lib/internal/worker.js
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,8 @@ const {
ERR_WORKER_UNSUPPORTED_EXTENSION,
ERR_WORKER_INVALID_EXEC_ARGV,
ERR_INVALID_ARG_TYPE,
// eslint-disable-next-line no-unused-vars
ERR_WORKER_INIT_FAILED,
} = errorCodes;
const { validateString } = require('internal/validators');
const { getOptionValue } = require('internal/options');
Expand Down Expand Up @@ -136,7 +138,9 @@ class Worker extends EventEmitter {
throw new ERR_WORKER_INVALID_EXEC_ARGV(
this[kHandle].invalidNodeOptions, 'invalid NODE_OPTIONS env variable');
}
this[kHandle].onexit = (code, customErr) => this[kOnExit](code, customErr);
this[kHandle].onexit = (code, customErr, customErrReason) => {
this[kOnExit](code, customErr, customErrReason);
};
this[kPort] = this[kHandle].messagePort;
this[kPort].on('message', (data) => this[kOnMessage](data));
this[kPort].start();
Expand Down Expand Up @@ -181,14 +185,15 @@ class Worker extends EventEmitter {
this[kHandle].startThread();
}

[kOnExit](code, customErr) {
[kOnExit](code, customErr, customErrReason) {
debug(`[${threadId}] hears end event for Worker ${this.threadId}`);
drainMessagePort(this[kPublicPort]);
drainMessagePort(this[kPort]);
this[kDispose]();
if (customErr) {
debug(`[${threadId}] failing with custom error ${customErr}`);
this.emit('error', new errorCodes[customErr]());
debug(`[${threadId}] failing with custom error ${customErr} \
and with reason {customErrReason}`);
this.emit('error', new errorCodes[customErr](customErrReason));
}
this.emit('exit', code);
this.removeAllListeners();
Expand Down
40 changes: 30 additions & 10 deletions src/node_worker.cc
Original file line number Diff line number Diff line change
Expand Up @@ -134,7 +134,17 @@ class WorkerThreadData {
public:
explicit WorkerThreadData(Worker* w)
: w_(w) {
CHECK_EQ(uv_loop_init(&loop_), 0);
int ret = uv_loop_init(&loop_);
if (ret != 0) {
char err_buf[128];
uv_err_name_r(ret, err_buf, sizeof(err_buf));
std::string error_str = SPrintF("ERR_WORKER_INIT_FAILED: %s", err_buf);
w->custom_error_ = "ERR_WORKER_INIT_FAILED";
w->custom_error_str_ = err_buf;
w->loop_init_failed_ = true;
w->stopped_ = true;
return;
}

std::shared_ptr<ArrayBufferAllocator> allocator =
ArrayBufferAllocator::Create();
Expand All @@ -147,6 +157,8 @@ class WorkerThreadData {
Isolate* isolate = Isolate::Allocate();
if (isolate == nullptr) {
w->custom_error_ = "ERR_WORKER_OUT_OF_MEMORY";
w->custom_error_str_ = "Failed to create new Isolate";
w->stopped_ = true;
return;
}

Expand Down Expand Up @@ -204,11 +216,14 @@ class WorkerThreadData {
isolate->Dispose();

// Wait until the platform has cleaned up all relevant resources.
while (!platform_finished)
while (!platform_finished) {
CHECK(!w_->loop_init_failed_);
uv_run(&loop_, UV_RUN_ONCE);
}
}
if (!w_->loop_init_failed_) {
CheckedUvLoopClose(&loop_);
}

CheckedUvLoopClose(&loop_);
}

private:
Expand All @@ -223,6 +238,7 @@ size_t Worker::NearHeapLimit(void* data, size_t current_heap_limit,
size_t initial_heap_limit) {
Worker* worker = static_cast<Worker*>(data);
worker->custom_error_ = "ERR_WORKER_OUT_OF_MEMORY";
worker->custom_error_str_ = "JS heap Out of Memory";
worker->Exit(1);
// Give the current GC some extra leeway to let it finish rather than
// crash hard. We are not going to perform further allocations anyway.
Expand All @@ -242,6 +258,7 @@ void Worker::Run() {

WorkerThreadData data(this);
if (isolate_ == nullptr) return;
CHECK(!data.w_->loop_init_failed_);

Debug(this, "Starting worker with id %llu", thread_id_);
{
Expand Down Expand Up @@ -287,9 +304,8 @@ void Worker::Run() {
TryCatch try_catch(isolate_);
context = NewContext(isolate_);
if (context.IsEmpty()) {
// TODO(addaleax): Inform the target about the actual underlying
// failure.
custom_error_ = "ERR_WORKER_OUT_OF_MEMORY";
custom_error_str_ = "Failed to create new Context";
return;
}
}
Expand Down Expand Up @@ -417,10 +433,14 @@ void Worker::JoinThread() {
Undefined(env()->isolate())).Check();

Local<Value> args[] = {
Integer::New(env()->isolate(), exit_code_),
custom_error_ != nullptr ?
OneByteString(env()->isolate(), custom_error_).As<Value>() :
Null(env()->isolate()).As<Value>(),
Integer::New(env()->isolate(), exit_code_),
!custom_error_.empty()
? OneByteString(env()->isolate(), custom_error_.c_str()).As<Value>()
: Null(env()->isolate()).As<Value>(),
!custom_error_str_.empty()
? OneByteString(env()->isolate(), custom_error_str_.c_str())
.As<Value>()
: Null(env()->isolate()).As<Value>(),
};

MakeCallback(env()->onexit_string(), arraysize(args), args);
Expand Down
4 changes: 3 additions & 1 deletion src/node_worker.h
Original file line number Diff line number Diff line change
Expand Up @@ -84,7 +84,9 @@ class Worker : public AsyncWrap {
mutable Mutex mutex_;

bool thread_joined_ = true;
const char* custom_error_ = nullptr;
std::string custom_error_;
std::string custom_error_str_;
bool loop_init_failed_ = false;
int exit_code_ = 0;
uint64_t thread_id_ = -1;
uintptr_t stack_base_ = 0;
Expand Down

0 comments on commit eee4378

Please sign in to comment.