Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

worker_threads.terminate results in no response on v12.x #38208

Closed
zyscoder opened this issue Apr 12, 2021 · 1 comment
Closed

worker_threads.terminate results in no response on v12.x #38208

zyscoder opened this issue Apr 12, 2021 · 1 comment
Labels
confirmed-bug Issues with confirmed bugs. worker Issues and PRs related to Worker support.

Comments

@zyscoder
Copy link

What steps will reproduce the bug?

Setup a node instance,

» node

and run the following javascript code.

worker_threads = require('worker_threads');newWorker = new worker_threads.Worker('str',{eval:true,resourceLimits:{maxYoungGenerationSizeMb:0,maxOldGenerationSizeMb:0}});
arr = [];
for (let i = 0; i < 15000000; i ++) {
    arr.push("num." + i);
}
newWorker.terminate();

Then the node instance stuck and has no response even with "Ctl-D".

How often does it reproduce? Is there a required condition?

This can be triggered following the steps above almost every time. If not, try again, please.

What is the expected behavior?

As the docs said, Stop all JavaScript execution in the worker thread as soon as possible. Returns a Promise for the exit code that is fulfilled when the 'exit' event is emitted..

What do you see instead?

» node
Welcome to Node.js v12.21.0.
Type ".help" for more information.
> worker_threads = require('worker_threads');newWorker = new worker_threads.Worker('str',{eval:true,resourceLimits:{maxYoungGenerationSizeMb:0,maxOldGenerationSizeMb:0}});
Worker {
...
}
> arr = [];
[]
> for (let i = 0; i < 15000000; i ++) {
...     arr.push("num." + i);
... }
15000000
> newWorker.terminate();
^C^C^C^C

Additional information
@Trott
Copy link
Member

Trott commented Apr 12, 2021

On macOS, the test code aborts the process in Node.js 12.x but works as expected in Node.js 14.x and 15.x.

@nodejs/workers

@Trott Trott added worker Issues and PRs related to Worker support. v12.x confirmed-bug Issues with confirmed bugs. labels Apr 12, 2021
santigimeno added a commit to santigimeno/node that referenced this issue Apr 25, 2021
@santigimeno
worker: avoid potential deadlock on NearHeapLimit
b9cd8b9 
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: nodejs#38208
@santigimeno santigimeno mentioned this issue Apr 25, 2021
Closed
santigimeno added a commit to santigimeno/node that referenced this issue May 5, 2021
@santigimeno
worker: avoid potential deadlock on NearHeapLimit
d4b11cb 
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: nodejs#38208
santigimeno added a commit to santigimeno/node that referenced this issue May 7, 2021
@santigimeno
worker: avoid potential deadlock on NearHeapLimit
750d62b 
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: nodejs#38208
santigimeno added a commit to santigimeno/node that referenced this issue Aug 3, 2021
@santigimeno
worker: avoid potential deadlock on NearHeapLimit
6142adc 
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: nodejs#38208
santigimeno added a commit to santigimeno/node that referenced this issue Aug 4, 2021
@santigimeno
worker: avoid potential deadlock on NearHeapLimit
73ac434 
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: nodejs#38208
santigimeno added a commit to santigimeno/node that referenced this issue Sep 9, 2021
@santigimeno
worker: avoid potential deadlock on NearHeapLimit
28414cb 
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: nodejs#38208
BethGriggs pushed a commit that referenced this issue Sep 21, 2021
@santigimeno @BethGriggs
worker: avoid potential deadlock on NearHeapLimit
a562016 
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>
BethGriggs pushed a commit that referenced this issue Sep 21, 2021
@santigimeno @BethGriggs
worker: avoid potential deadlock on NearHeapLimit
5983568 
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>
richardlau pushed a commit that referenced this issue Nov 26, 2021
@santigimeno @richardlau
worker: avoid potential deadlock on NearHeapLimit
39583f7 
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>
richardlau pushed a commit that referenced this issue Dec 14, 2021
@santigimeno @richardlau
worker: avoid potential deadlock on NearHeapLimit
f50b9c1 
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>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
confirmed-bug Issues with confirmed bugs. worker Issues and PRs related to Worker support.
Projects
None yet
Milestone
No milestone
Development

Successfully merging a pull request may close this issue.

worker: avoid potential deadlock on NearHeapLimit santigimeno/node
2 participants
@Trott @zyscoder