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 throws exception while loading npm lib #21783

Closed
danrevah opened this issue Jul 12, 2018 · 10 comments
Closed

worker threads throws exception while loading npm lib #21783

danrevah opened this issue Jul 12, 2018 · 10 comments
Labels
addons Issues and PRs related to native addons. worker Issues and PRs related to Worker support.

Comments

@danrevah
Copy link

danrevah commented Jul 12, 2018

the following code fails if I'm trying to run it, seems the problem comes from the marked line below.

const {
  Worker, isMainThread, parentPort, workerData
} = require('worker_threads');

const Kafka = require('node-rdkafka'); // <-- Problemetic line..

if (isMainThread)
{
  console.log('main thread');
  const worker = new Worker(__filename, {
    workerData: 1
  });

  worker.on('message', (args) => console.log('got message from worker', args));
  worker.on('error', (args) => console.log('got error from worker', args));
  worker.on('exit', (code) => {
    if (code !== 0)
      console.log(`Worker stopped with exit code ${code}`);
  });

} else {
  const script = workerData;
  parentPort.postMessage(2);
}

'node-rdkafka' is a external library I'm using, and I was tempted to try and see if I could improve performance with the new worker threads.

while a worker is loaded, there's an error thrown saying 'Module did not self-register'.

this lib does work perfectly without the worker thread.

got error from worker Error: Module did not self-register.
    at Object.Module._extensions..node (internal/modules/cjs/loader.js:718:18)
    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 Module.require (internal/modules/cjs/loader.js:637:17)
    at require (internal/modules/cjs/helpers.js:20:18)
    at bindings (/Users/danrevah/dev/git/tmp-librdkafka/node_modules/bindings/bindings.js:81:44)
    at Object.<anonymous> (/Users/danrevah/dev/git/tmp-librdkafka/node_modules/node-rdkafka/librdkafka.js:10:32)
    at Module._compile (internal/modules/cjs/loader.js:689:30)
  • Version: 10.6.0
  • Platform: Darwin Dans-MacBook-Pro.local 17.5.0 Darwin Kernel Version 17.5.0: Fri Apr 13 19:32:32 PDT 2018; root:xnu-4570.51.2~1/RELEASE_X86_64 x86_64

Angular 4-6+ Pipes - https://github.com/danrevah/ngx-pipes

@danrevah danrevah changed the title Bug with worker threads while loading external lib Bug with worker threads while loading npm lib Jul 12, 2018
@danrevah danrevah changed the title Bug with worker threads while loading npm lib worker threads throws exception while loading npm lib Jul 12, 2018
@devsnek devsnek added the worker Issues and PRs related to Worker support. label Jul 12, 2018
@devsnek
Copy link
Member

devsnek commented Jul 12, 2018

probably related to #21611?

@danrevah
Copy link
Author

Seems like it can be related, is this PR going to be accepted or there's going to be a different approach on solving this?

@addaleax
Copy link
Member

@danrevah I’m hoping to eventually require addons to explicitly opt in into supporting workers. Maybe that’s better through a warning than disabling it completely, though.

@alexcastillo
Copy link

Hi, any updates? I was under the impression worker threads supported npm libs.

@addaleax
Copy link
Member

addaleax commented Sep 2, 2018

@alexcastillo It does, but native add-ons are a special case and you’ll need to contact the add-on author about this.

@alexcastillo
Copy link

Thanks for the clarification, @addaleax!

@mattolson
Copy link

mattolson commented Oct 10, 2018

I'm running into this as well when using experimental workers that try to load the lzo package. If this is something that addon authors need to handle, is there documentation or an example somewhere on how to do that? Also, is this issue a duplicate of #21481?

@fathyb
Copy link

fathyb commented Oct 14, 2018

@mattolson

is there documentation or an example somewhere on how to do that?

It looks like making your module context-aware fixes the issue, see the docs#Context-aware addons.
For me, it meant changing this:

NODE_MODULE(NODE_GYP_MODULE_NAME, Init)

Into this:

NODE_MODULE_INIT() {
    Init(exports);
}

But that's because I don't touch the global context, if you do you'll need to use the context macro argument of NODE_MODULE_INIT.

@mattolson
Copy link

@fathyb Thanks for the pointer. That helped us update node-lzo to work with worker threads in schroffl/node-lzo#11

@Ethan-Arrowood
Copy link
Contributor

Another affected module is mmap-io. i'm trying to fix it now but it is proving difficult.

@addaleax addaleax added the addons Issues and PRs related to native addons. label Feb 17, 2019
addaleax added a commit that referenced this issue Feb 25, 2019
Allow loading add-ons from multiple Node.js instances if they are
declared context-aware; in particular, this applies to N-API addons.

Also, plug a memory leak that occurred when registering N-API addons.

Refs: #23319

PR-URL: #26175
Fixes: #21481
Fixes: #21783
Fixes: #25662
Fixes: #20239
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Gireesh Punathil <gpunathi@in.ibm.com>
Reviewed-By: Richard Lau <riclau@uk.ibm.com>
rvagg pushed a commit that referenced this issue Feb 28, 2019
Allow loading add-ons from multiple Node.js instances if they are
declared context-aware; in particular, this applies to N-API addons.

Also, plug a memory leak that occurred when registering N-API addons.

Refs: #23319

PR-URL: #26175
Fixes: #21481
Fixes: #21783
Fixes: #25662
Fixes: #20239
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Gireesh Punathil <gpunathi@in.ibm.com>
Reviewed-By: Richard Lau <riclau@uk.ibm.com>
rm-hull pushed a commit to hthetiot/node-snowball that referenced this issue Aug 14, 2020
Vikasg7 added a commit to Vikasg7/node-multi-hashing that referenced this issue Jun 24, 2021
S4N0I added a commit to S4N0I/annoy-node that referenced this issue Apr 27, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
addons Issues and PRs related to native addons. worker Issues and PRs related to Worker support.
Projects
None yet
Development

Successfully merging a pull request may close this issue.

7 participants