Skip to content

Navigation Menu

Sign in
Appearance settings

Search code, repositories, users, issues, pull requests...

Provide feedback

We read every piece of feedback, and take your input very seriously.

Saved searches

Use saved searches to filter your results more quickly

Appearance settings

Commit 18ecaeb

Browse filesBrowse 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 <gpunathi@in.ibm.com> Reviewed-By: Colin Ihrig <cjihrig@gmail.com> Reviewed-By: James M Snell <jasnell@gmail.com>
1 parent 1066341 commit 18ecaeb
Copy full SHA for 18ecaeb

File tree

Expand file treeCollapse file tree

2 files changed

+22
-15
lines changed
Open diff view settings
Filter options
Expand file treeCollapse file tree

2 files changed

+22
-15
lines changed
Open diff view settings
Collapse file

‎src/node_worker.cc‎

Copy file name to clipboardExpand all lines: src/node_worker.cc
+17-13Lines changed: 17 additions & 13 deletions
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_);
Collapse file

‎src/node_worker.h‎

Copy file name to clipboardExpand all lines: src/node_worker.h
+5-2Lines changed: 5 additions & 2 deletions
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 (0)
Morty Proxy This is a proxified and sanitized view of the page, visit original site.