From f2389eba99909b6fcdcf76d28a687208341e50f4 Mon Sep 17 00:00:00 2001 From: Harshitha KP Date: Mon, 3 Feb 2020 00:06:35 -0500 Subject: [PATCH] worker: emit runtime error on loop creation failure Instead of hard asserting throw a runtime error, that is more consumable. Fixes: https://github.com/nodejs/node/issues/31614 PR-URL: https://github.com/nodejs/node/pull/31621 Reviewed-By: Anna Henningsen --- doc/api/errors.md | 5 +++ lib/internal/errors.js | 5 ++- lib/internal/worker.js | 13 +++++-- src/node_worker.cc | 39 +++++++++++++++----- src/node_worker.h | 2 + test/parallel/test-worker-resource-limits.js | 3 +- 6 files changed, 50 insertions(+), 17 deletions(-) diff --git a/doc/api/errors.md b/doc/api/errors.md index d231cf28857dc0..09f43e5036729d 100644 --- a/doc/api/errors.md +++ b/doc/api/errors.md @@ -2066,6 +2066,11 @@ meaning of the error depends on the specific function. The WASI instance has already started. + +### `ERR_WORKER_INIT_FAILED` + +The `Worker` initialization failed. + ### `ERR_WORKER_INVALID_EXEC_ARGV` diff --git a/lib/internal/errors.js b/lib/internal/errors.js index 8c18eabb7a8d06..dcadbedb9e2f77 100644 --- a/lib/internal/errors.js +++ b/lib/internal/errors.js @@ -1381,12 +1381,13 @@ 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_NOT_RUNNING', 'Worker instance not running', 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"', diff --git a/lib/internal/worker.js b/lib/internal/worker.js index de626af1bef962..7fc45695851f8c 100644 --- a/lib/internal/worker.js +++ b/lib/internal/worker.js @@ -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'); @@ -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(); @@ -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(); diff --git a/src/node_worker.cc b/src/node_worker.cc index 2c984373308d91..a5dcec250e7a72 100644 --- a/src/node_worker.cc +++ b/src/node_worker.cc @@ -133,7 +133,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; + } std::shared_ptr allocator = ArrayBufferAllocator::Create(); @@ -146,6 +155,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; } @@ -204,11 +215,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: @@ -223,6 +237,7 @@ size_t Worker::NearHeapLimit(void* data, size_t current_heap_limit, size_t initial_heap_limit) { Worker* worker = static_cast(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. @@ -242,6 +257,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_); { @@ -287,9 +303,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; } } @@ -417,10 +432,14 @@ void Worker::JoinThread() { Undefined(env()->isolate())).Check(); Local args[] = { - Integer::New(env()->isolate(), exit_code_), - custom_error_ != nullptr ? - OneByteString(env()->isolate(), custom_error_).As() : - Null(env()->isolate()).As(), + Integer::New(env()->isolate(), exit_code_), + custom_error_ != nullptr + ? OneByteString(env()->isolate(), custom_error_).As() + : Null(env()->isolate()).As(), + !custom_error_str_.empty() + ? OneByteString(env()->isolate(), custom_error_str_.c_str()) + .As() + : Null(env()->isolate()).As(), }; MakeCallback(env()->onexit_string(), arraysize(args), args); diff --git a/src/node_worker.h b/src/node_worker.h index 0c6fd35c0ab032..dbd286109948da 100644 --- a/src/node_worker.h +++ b/src/node_worker.h @@ -85,6 +85,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; diff --git a/test/parallel/test-worker-resource-limits.js b/test/parallel/test-worker-resource-limits.js index 2d4ebbc0ce6011..9332a132694e78 100644 --- a/test/parallel/test-worker-resource-limits.js +++ b/test/parallel/test-worker-resource-limits.js @@ -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; }