Skip to content

Commit

Permalink
worker: emit 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: #31614

PR-URL: #31621
Reviewed-By: Anna Henningsen <anna@addaleax.net>
  • Loading branch information
HarshithaKP authored and codebytere committed Mar 17, 2020
1 parent b5b0dc9 commit 5af11df
Show file tree
Hide file tree
Showing 6 changed files with 50 additions and 17 deletions.
5 changes: 5 additions & 0 deletions doc/api/errors.md
Original file line number Diff line number Diff line change
Expand Up @@ -2007,6 +2007,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
5 changes: 3 additions & 2 deletions lib/internal/errors.js
Original file line number Diff line number Diff line change
Expand Up @@ -1344,11 +1344,12 @@ 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);
E('ERR_WORKER_OUT_OF_MEMORY', 'Worker terminated due to reaching memory limit',
Error);
E('ERR_WORKER_OUT_OF_MEMORY',
'Worker terminated due to reaching memory limit: %s', Error);
E('ERR_WORKER_PATH',
'The worker script filename must be an absolute path or a relative ' +
'path starting with \'./\' or \'../\'. Received "%s"',
Expand Down
13 changes: 9 additions & 4 deletions lib/internal/worker.js
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,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 @@ -135,7 +137,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 @@ -180,14 +184,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
39 changes: 29 additions & 10 deletions src/node_worker.cc
Original file line number Diff line number Diff line change
Expand Up @@ -138,7 +138,16 @@ 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));
w->custom_error_ = "ERR_WORKER_INIT_FAILED";
w->custom_error_str_ = err_buf;
w->loop_init_failed_ = true;
w->stopped_ = true;
return;
}

Isolate::CreateParams params;
SetIsolateCreateParamsForNode(&params);
Expand All @@ -149,6 +158,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 @@ -206,11 +217,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 @@ -225,6 +239,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 @@ -244,6 +259,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 @@ -289,9 +305,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 @@ -421,10 +436,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_ != nullptr
? OneByteString(env()->isolate(), custom_error_).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
2 changes: 2 additions & 0 deletions src/node_worker.h
Original file line number Diff line number Diff line change
Expand Up @@ -87,6 +87,8 @@ class Worker : public AsyncWrap {

bool thread_joined_ = true;
const char* custom_error_ = nullptr;
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
3 changes: 2 additions & 1 deletion test/parallel/test-worker-resource-limits.js
Original file line number Diff line number Diff line change
Expand Up @@ -25,7 +25,8 @@ if (!process.env.HAS_STARTED_WORKER) {
}));
w.on('error', common.expectsError({
code: 'ERR_WORKER_OUT_OF_MEMORY',
message: 'Worker terminated due to reaching memory limit'
message: 'Worker terminated due to reaching memory limit: ' +
'JS heap out of memory'
}));
return;
}
Expand Down

0 comments on commit 5af11df

Please sign in to comment.