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.

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

Worker threads: the process keeps living on exception #22736

Closed
wilk opened this issue Sep 6, 2018 · 0 comments
Closed

Worker threads: the process keeps living on exception #22736

wilk opened this issue Sep 6, 2018 · 0 comments
Labels
worker Issues and PRs related to Worker support.

Comments

@wilk
Copy link

wilk commented Sep 6, 2018

  • Version: v10.9.0
  • Platform: Darwin 17.7.0 Darwin Kernel Version 17.7.0
  • Subsystem:

The problem is related to Node.js worker threads: https://nodejs.org/api/worker_threads.html
The worker thread is spawned as illustrated in the following example:

// main.js
const {Worker} = require('worker_threads')

try {
  new Worker('./worker.js', {
    workerData: {fn: () => {}}
  })
} catch (err) {
  console.log(err)
}

And then this is the actual worker:

// worker.js
const {parentPort, workerData} = require('worker_threads')

console.log(workerData)

parentPort.postMessage('ok')

Now, worker threads do not support elements such as functions and classes in worker data and an exception is thrown when main.js is launched:

$ node --experimental-worker main.js

The output is this:

$ node --experimental-worker main.js 
DataCloneError: () => {} could not be cloned.
    at new Worker (internal/worker.js:269:17)
    at Object.<anonymous> (/Users/wilk/Projects/mine/task/main.js:22:3)
    at Module._compile (internal/modules/cjs/loader.js:689:30)
    at Object.Module._extensions..js (internal/modules/cjs/loader.js:700:10)
    at Module.load (internal/modules/cjs/loader.js:599:32)
    at tryModuleLoad (internal/modules/cjs/loader.js:538:12)
    at Function.Module._load (internal/modules/cjs/loader.js:530:3)
    at Function.Module.runMain (internal/modules/cjs/loader.js:742:12)
    at startup (internal/bootstrap/node.js:266:19)
    at bootstrapNodeJSCore (internal/bootstrap/node.js:596:3)

Unfortunately, the process keeps alive instead of dying.
However, if I prepend the v8 serialization before the worker instantiation, then it terminates as expected:

// main.js
const {Worker} = require('worker_threads')
const v8 = require('v8')

try {
  v8.serialize({fn: () => {}})
  new Worker('./worker.js', {
    workerData: {fn: () => {}}
  })
} catch (err) {
  console.log(err)
}

I think the issue is with v8 serialization's error handling inside the Worker's constructor.

@addaleax addaleax added the worker Issues and PRs related to Worker support. label Sep 9, 2018
addaleax added a commit to addaleax/node that referenced this issue Sep 9, 2018
- Initialize `thread_exit_async_` only once the thread has been
  started. This is done since it is only triggered from the
  thread when it is exiting.
- Move the final `uv_run` to the `Worker` destructor.
  This makes sure that it is always run, regardless of whether
  the thread is actually started or not.
- Always dispose the `Isolate` before cleaning up the libuv event
  loop. This now matches the reverse order of initialization.

Fixes: nodejs#22736
targos pushed a commit that referenced this issue Sep 15, 2018
- Initialize `thread_exit_async_` only once the thread has been
  started. This is done since it is only triggered from the
  thread when it is exiting.
- Move the final `uv_run` to the `Worker` destructor.
  This makes sure that it is always run, regardless of whether
  the thread is actually started or not.
- Always dispose the `Isolate` before cleaning up the libuv event
  loop. This now matches the reverse order of initialization.

Fixes: #22736
PR-URL: #22773
Reviewed-By: James M Snell <[email protected]>
targos pushed a commit that referenced this issue Sep 19, 2018
- Initialize `thread_exit_async_` only once the thread has been
  started. This is done since it is only triggered from the
  thread when it is exiting.
- Move the final `uv_run` to the `Worker` destructor.
  This makes sure that it is always run, regardless of whether
  the thread is actually started or not.
- Always dispose the `Isolate` before cleaning up the libuv event
  loop. This now matches the reverse order of initialization.

Fixes: #22736
PR-URL: #22773
Reviewed-By: James M Snell <[email protected]>
targos pushed a commit that referenced this issue Sep 20, 2018
- Initialize `thread_exit_async_` only once the thread has been
  started. This is done since it is only triggered from the
  thread when it is exiting.
- Move the final `uv_run` to the `Worker` destructor.
  This makes sure that it is always run, regardless of whether
  the thread is actually started or not.
- Always dispose the `Isolate` before cleaning up the libuv event
  loop. This now matches the reverse order of initialization.

Fixes: #22736
PR-URL: #22773
Reviewed-By: James M Snell <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
worker Issues and PRs related to Worker support.
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants