Skip to content

Commit 18ecaeb

Browse files
committed
worker: unify custom error creation
Mostly, this introduces a pattern that makes sure that if a custom error is reported, `stopped_` will be set to `true` correctly in every cast, which was previously missing for the `NewContext().IsEmpty()` case (which led to a hard crash from the `Worker` destructor). This also leaves TODO comments for a few cases in which `ERR_WORKER_OUT_OF_MEMORY` was not used in accordance with the documentation for that error code (or according to its intention). Fixing that is semver-major. Backport-PR-URL: #35241 PR-URL: #33084 Reviewed-By: Gireesh Punathil <[email protected]> Reviewed-By: Colin Ihrig <[email protected]> Reviewed-By: James M Snell <[email protected]>
1 parent 1066341 commit 18ecaeb

File tree

2 files changed

+22
-15
lines changed

2 files changed

+22
-15
lines changed

src/node_worker.cc

+17-13
Original file line numberDiff line numberDiff line change
@@ -136,9 +136,7 @@ class WorkerThreadData {
136136
if (ret != 0) {
137137
char err_buf[128];
138138
uv_err_name_r(ret, err_buf, sizeof(err_buf));
139-
w->custom_error_ = "ERR_WORKER_INIT_FAILED";
140-
w->custom_error_str_ = err_buf;
141-
w->stopped_ = true;
139+
w->Exit(1, "ERR_WORKER_INIT_FAILED", err_buf);
142140
return;
143141
}
144142
loop_init_failed_ = false;
@@ -152,9 +150,9 @@ class WorkerThreadData {
152150

153151
Isolate* isolate = Isolate::Allocate();
154152
if (isolate == nullptr) {
155-
w->custom_error_ = "ERR_WORKER_OUT_OF_MEMORY";
156-
w->custom_error_str_ = "Failed to create new Isolate";
157-
w->stopped_ = true;
153+
// TODO(addaleax): This should be ERR_WORKER_INIT_FAILED,
154+
// ERR_WORKER_OUT_OF_MEMORY is for reaching the per-Worker heap limit.
155+
w->Exit(1, "ERR_WORKER_OUT_OF_MEMORY", "Failed to create new Isolate");
158156
return;
159157
}
160158

@@ -237,9 +235,7 @@ class WorkerThreadData {
237235
size_t Worker::NearHeapLimit(void* data, size_t current_heap_limit,
238236
size_t initial_heap_limit) {
239237
Worker* worker = static_cast<Worker*>(data);
240-
worker->custom_error_ = "ERR_WORKER_OUT_OF_MEMORY";
241-
worker->custom_error_str_ = "JS heap out of memory";
242-
worker->Exit(1);
238+
worker->Exit(1, "ERR_WORKER_OUT_OF_MEMORY", "JS heap out of memory");
243239
// Give the current GC some extra leeway to let it finish rather than
244240
// crash hard. We are not going to perform further allocations anyway.
245241
constexpr size_t kExtraHeapAllowance = 16 * 1024 * 1024;
@@ -301,8 +297,9 @@ void Worker::Run() {
301297
TryCatch try_catch(isolate_);
302298
context = NewContext(isolate_);
303299
if (context.IsEmpty()) {
304-
custom_error_ = "ERR_WORKER_OUT_OF_MEMORY";
305-
custom_error_str_ = "Failed to create new Context";
300+
// TODO(addaleax): This should be ERR_WORKER_INIT_FAILED,
301+
// ERR_WORKER_OUT_OF_MEMORY is for reaching the per-Worker heap limit.
302+
Exit(1, "ERR_WORKER_OUT_OF_MEMORY", "Failed to create new Context");
306303
return;
307304
}
308305
}
@@ -685,9 +682,16 @@ Local<Float64Array> Worker::GetResourceLimits(Isolate* isolate) const {
685682
return Float64Array::New(ab, 0, kTotalResourceLimitCount);
686683
}
687684

688-
void Worker::Exit(int code) {
685+
void Worker::Exit(int code, const char* error_code, const char* error_message) {
689686
Mutex::ScopedLock lock(mutex_);
690-
Debug(this, "Worker %llu called Exit(%d)", thread_id_.id, code);
687+
Debug(this, "Worker %llu called Exit(%d, %s, %s)",
688+
thread_id_.id, code, error_code, error_message);
689+
690+
if (error_code != nullptr) {
691+
custom_error_ = error_code;
692+
custom_error_str_ = error_message;
693+
}
694+
691695
if (env_ != nullptr) {
692696
exit_code_ = code;
693697
Stop(env_);

src/node_worker.h

+5-2
Original file line numberDiff line numberDiff line change
@@ -34,8 +34,11 @@ class Worker : public AsyncWrap {
3434
void Run();
3535

3636
// Forcibly exit the thread with a specified exit code. This may be called
37-
// from any thread.
38-
void Exit(int code);
37+
// from any thread. `error_code` and `error_message` can be used to create
38+
// a custom `'error'` event before emitting `'exit'`.
39+
void Exit(int code,
40+
const char* error_code = nullptr,
41+
const char* error_message = nullptr);
3942

4043
// Wait for the worker thread to stop (in a blocking manner).
4144
void JoinThread();

0 commit comments

Comments
 (0)