Skip to content

Commit b9cd8b9

Browse files
committed
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
1 parent 8780537 commit b9cd8b9

File tree

3 files changed

+37
-5
lines changed

3 files changed

+37
-5
lines changed

src/node_worker.cc

Lines 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() {

src/node_worker.h

Lines 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

Lines 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_threads = require('worker_threads');
5+
6+
const opts = {
7+
eval: true,
8+
resourceLimits: {
9+
maxYoungGenerationSizeMb: 0,
10+
maxOldGenerationSizeMb: 0
11+
}
12+
};
13+
14+
const worker = new worker_threads.Worker('str', opts);
15+
const arr = [];
16+
for (let i = 0; i < 15000000; ++i) {
17+
arr.push('num.' + i);
18+
}
19+
20+
worker.on('error', common.mustCall((err) => {
21+
assert.strictEqual(err.code, 'ERR_WORKER_OUT_OF_MEMORY');
22+
}));

0 commit comments

Comments
 (0)