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 5983568

Browse filesBrowse files
santigimenoBethGriggs
authored andcommitted
worker: avoid potential deadlock on NearHeapLimit
It can happen that the `NearHeapLimit` callback is called while calling the `oninit()` function on `MessagePort` construction causing a deadlock when the `Worker::Exit()` method is called, as the `mutex_` was already held on the `CreateEnvMessagePort()` method. To fix it, just use the `mutex_` to protect the `child_port_data_` variable and avoid holding it when creating the `MessagePort`. Also, return early from `Worker::Run()` if the worker message port could not be created. Fixes: #38208 PR-URL: #38403 Reviewed-By: Anna Henningsen <anna@addaleax.net> Reviewed-By: James M Snell <jasnell@gmail.com> Reviewed-By: Colin Ihrig <cjihrig@gmail.com> Reviewed-By: Joyee Cheung <joyeec9h3@gmail.com> Reviewed-By: Rich Trott <rtrott@gmail.com> Reviewed-By: Juan José Arboleda <soyjuanarbol@gmail.com>
1 parent 5b5e272 commit 5983568
Copy full SHA for 5983568

File tree

Expand file treeCollapse file tree

3 files changed

+37
-5
lines changed
Open diff view settings
Filter options
Expand file treeCollapse file tree

3 files changed

+37
-5
lines changed
Open diff view settings
Collapse file

‎src/node_worker.cc‎

Copy file name to clipboardExpand all lines: src/node_worker.cc
+14-4Lines changed: 14 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -328,7 +328,10 @@ void Worker::Run() {
328328
Debug(this, "Created Environment for worker with id %llu", thread_id_.id);
329329
if (is_stopped()) return;
330330
{
331-
CreateEnvMessagePort(env_.get());
331+
if (!CreateEnvMessagePort(env_.get())) {
332+
return;
333+
}
334+
332335
Debug(this, "Created message port for worker %llu", thread_id_.id);
333336
if (LoadEnvironment(env_.get(), StartExecutionCallback{}).IsEmpty())
334337
return;
@@ -352,17 +355,24 @@ void Worker::Run() {
352355
Debug(this, "Worker %llu thread stops", thread_id_.id);
353356
}
354357

355-
void Worker::CreateEnvMessagePort(Environment* env) {
358+
bool Worker::CreateEnvMessagePort(Environment* env) {
356359
HandleScope handle_scope(isolate_);
357-
Mutex::ScopedLock lock(mutex_);
360+
std::unique_ptr<MessagePortData> data;
361+
{
362+
Mutex::ScopedLock lock(mutex_);
363+
data = std::move(child_port_data_);
364+
}
365+
358366
// Set up the message channel for receiving messages in the child.
359367
MessagePort* child_port = MessagePort::New(env,
360368
env->context(),
361-
std::move(child_port_data_));
369+
std::move(data));
362370
// MessagePort::New() may return nullptr if execution is terminated
363371
// within it.
364372
if (child_port != nullptr)
365373
env->set_message_port(child_port->object(isolate_));
374+
375+
return child_port;
366376
}
367377

368378
void Worker::JoinThread() {
Collapse file

‎src/node_worker.h‎

Copy file name to clipboardExpand all lines: src/node_worker.h
+1-1Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -70,7 +70,7 @@ class Worker : public AsyncWrap {
7070
static void LoopStartTime(const v8::FunctionCallbackInfo<v8::Value>& args);
7171

7272
private:
73-
void CreateEnvMessagePort(Environment* env);
73+
bool CreateEnvMessagePort(Environment* env);
7474
static size_t NearHeapLimit(void* data, size_t current_heap_limit,
7575
size_t initial_heap_limit);
7676

Collapse file
+22Lines changed: 22 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,22 @@
1+
'use strict';
2+
const common = require('../common');
3+
const assert = require('assert');
4+
const { Worker } = require('worker_threads');
5+
6+
// Do not use isMainThread so that this test itself can be run inside a Worker.
7+
if (!process.env.HAS_STARTED_WORKER) {
8+
process.env.HAS_STARTED_WORKER = 1;
9+
const opts = {
10+
resourceLimits: {
11+
maxYoungGenerationSizeMb: 0,
12+
maxOldGenerationSizeMb: 0
13+
}
14+
};
15+
16+
const worker = new Worker(__filename, opts);
17+
worker.on('error', common.mustCall((err) => {
18+
assert.strictEqual(err.code, 'ERR_WORKER_OUT_OF_MEMORY');
19+
}));
20+
} else {
21+
setInterval(() => {}, 1);
22+
}

0 commit comments

Comments
0 (0)
Morty Proxy This is a proxified and sanitized view of the page, visit original site.