-
Notifications
You must be signed in to change notification settings - Fork 3
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 thread support #6
Comments
Hi @willfrew, thanks for the thorough information, and I appreciate you trying to create a fix. Can you share a basic code snippet that leads to that error? That's not an error I've encountered before but I can also take a look once I can reproduce. |
Hi @mikeokner! So I tried to create a minimal repro that throws the same error I see above, however my first attempt yielded segfaults which may still be useful but obviously not quite the same thing. Repo here: https://github.com/willfrew/posix-mq-repro Run |
For completeness here, the code I have that I'm seeing segfaults with is: const { Worker, parentPort } = require('worker_threads');
if (parentPort) {
console.log('In worker');
const PosixMQ = require('posix-mq');
const mq = new PosixMQ();
try {
mq.open({
name: '/test-mq',
mode: '0700',
create: true,
exclusive: true,
});
mq.unlink();
} catch (error) {
console.error(error);
}
} else {
// main
const worker = new Worker(__filename);
} I was just doing a bit more testing and I see that if I push the require up such that is is required in both main and worker threads (i.e.): const { Worker, parentPort } = require('worker_threads');
const PosixMQ = require('posix-mq');
if (parentPort) {
console.log('In worker');
... The error I then see is instead:
Unfortunately I've been unable to reproduce the error I'm seeing in my main repo but am unable to share the code.. sorry! |
Will, I was able to reproduce the original issue. From what I can tell, the best solution to add suppoort for worker_threads is to switch from using NAN to Napi, which is a somewhat involved process. I found these issues to be helpful in understanding the situation:
I'll look more at getting this ported, but it could take a little while. Another approach that I validated works if you are able to use something other than const Worker = require("tiny-worker");
const worker = new Worker(function() {
self.onmessage = async function(event) {
const idx = event.data;
console.log(`${idx} - In worker. Creating posix mq...`);
try {
const PosixMQ = require('posix-mq');
const mq = new PosixMQ();
mq.open({
name: '/test-pmq',
mode: '0777',
create: true,
maxmsgs: 10,
msgsize: 8
});
let wbuf = Buffer.alloc(1);
wbuf[0] = Math.floor(Math.random() * 93) + 33;
console.log(`${idx} - writing `+ wbuf[0] +" ('"+ String.fromCharCode(wbuf[0]) +"')...");
mq.push(wbuf);
console.log(`${idx} - Done writing.`);
// sleep for a bit to validate parallel execution
await new Promise(r => setTimeout(r, Math.random()*5*1000));
console.log(`${idx} - Reading from queue...`);
let rbuf = Buffer.alloc(mq.msgsize);
let n = mq.shift(rbuf);
console.log(`${idx} - got msg: '`+ rbuf.toString('utf8', 0, n) + `' (${n} bytes)`);
//mq.unlink(); // don't unlink or it will remove the handle for other workers
mq.close();
console.log(`${idx} - Queue opened & closed successfully. Worker done.`);
// Send update back to main
postMessage(`${idx} - Message - done with all operations.`);
} catch (error) {
console.error(`${idx} - Worker failed: `, error);
}
}
});
// Run 4 workers
worker.postMessage(1);
worker.postMessage(2);
worker.postMessage(3);
worker.postMessage(4);
// Handle any response from workers
worker.onmessage = function(event) {
console.log("Message received: " + event.data);
}; Running this looks like
|
Hey @mikeokner, just wanted to update that I ended up switching to another ipc transport to work around this - so no urgent need for this from me! Definitely understanding switching to n-api is non-trivial. FWIW My use-case prevented me from using process-based concurrency (and therefore |
Hi! I'm trying to use this module in a worker thread but am hitting the following error:
Tested with Node v18 & v16.
The above stacktrace was from v18, but with v16 it's almost identical.
I did some surface research and saw some description of what would be needed to make this a 'context-aware addon' here: https://nodejs.org/api/addons.html#context-aware-addons
And additionally in the Nan repo this discussion led me to the
NAN_MODULE_WORKER_ENABLED
macro.I'm going to try and pull the repo and see if I can make it work, but I'm a little bit in over my head with C++ and if significant changes are needed I'll probably not be able to do it :'D Any help / tips would be amazing!
The text was updated successfully, but these errors were encountered: