h0x91b / redis-fast-driver

78 stars 13 forks source link

Error: 'Module did not self-register' using worker_threads in v12 #26

Open shuckc opened 5 years ago

shuckc commented 5 years ago

If redis-fast-driver is requireed by both the 'main thread' and also a worker thread, the 2nd attempt fails and triggers an Error in the cjs loader.

Reproducing code - worker.js:

/* eslint-disable no-console */
const Redis = require('redis-fast-driver');
const {Worker, isMainThread} = require('worker_threads');

if (isMainThread) {
  console.log('Main Thread');
  const worker = new Worker(__filename);
  worker.on('message', console.log);
} else {
  console.log(`in worker ${__filename}`);
}
console.log('first pass done');

Output:

$ node worker.js 
Main Thread
first pass done

events.js:173
      throw er; // Unhandled 'error' event
      ^
Error: Module did not self-register.
    at Object.Module._extensions..node (internal/modules/cjs/loader.js:800:18)
    at Module.load (internal/modules/cjs/loader.js:628:32)
    at Function.Module._load (internal/modules/cjs/loader.js:555:12)
    at Module.require (internal/modules/cjs/loader.js:666:19)
    at require (internal/modules/cjs/helpers.js:16:16)
    at Object.<anonymous> (/Users/shuckc/project1/node_modules/redis-fast-driver/index.js:2:15)
    at Module._compile (internal/modules/cjs/loader.js:759:30)
    at Object.Module._extensions..js (internal/modules/cjs/loader.js:770:10)
    at Module.load (internal/modules/cjs/loader.js:628:32)
    at Function.Module._load (internal/modules/cjs/loader.js:555:12)
Emitted 'error' event at:
    at Worker.[kOnErrorMessage] (internal/worker.js:161:10)
    at Worker.[kOnMessage] (internal/worker.js:171:37)
    at MessagePort.<anonymous> (internal/worker.js:104:57)
    at MessagePort.emit (events.js:196:13)
    at MessagePort.onmessage (internal/worker/io.js:68:8)

It seems to me there might be some additional work on the C++ side to have a module support being required twice. This is an issue even if the worker does not invoke the client on the codepath, just requires it.

shuckc commented 5 years ago

Seems relevant: Another module with same issue: https://github.com/schroffl/node-lzo/issues/10 Reported fix: https://github.com/nodejs/node/issues/21783#issuecomment-429637117

Node docs for context-aware-addons: https://github.com/nodejs/node/blob/master/doc/api/addons.md#context-aware-addons

h0x91b commented 5 years ago

Thanks, I'll try to fix it

h0x91b commented 5 years ago

Sadly but, not enough to just use NODE_MODULE_INIT (BTW this was hard to do)

Seems like I must migrate from NaN to N-API via node-addon-api to correctly support workers, this will take little bit time and may not be stable.

Will update here.

FlorianMaak commented 4 years ago

Any Updates on this?

h0x91b commented 4 years ago

No, need to completely rewrite C++ part of the module. So actually it will be a new driver. Sadly but I don't have enough free time now to rewrite and test it.