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

Support for worker_threads #59

Open
femanzo opened this issue Aug 12, 2019 · 1 comment
Open

Support for worker_threads #59

femanzo opened this issue Aug 12, 2019 · 1 comment

Comments

@femanzo
Copy link

femanzo commented Aug 12, 2019

Hi, I was getting an error (Module did not self-register) when while running this package inside a Worker thread, I have fixed it by changing:

NODE_MODULE(tulind, Init)

to

NODE_MODULE_INIT()
{
  Init(exports);
}

on the tulind.cpp file, then rebuilding with node-pre-gyp install --build-from-source

I'm still testing, but it seems to be working fine.
I didn't make a pull request because I don't want to mess up your code with my linters, but It would be nice to have this change on the main repo, in case anybody is having the same issue.

jeremyjs added a commit to jeremyjs/tulipnode that referenced this issue Oct 25, 2019
@jeremyjs
Copy link

jeremyjs commented Oct 28, 2019

After taking a look at the docs, it actually looks like there are a few other additions needed to properly support context awareness: https://nodejs.org/api/addons.html#addons_context_aware_addons and worker threads: https://nodejs.org/api/addons.html#addons_worker_support.

Tests would need to be added to ensure cleanup occurs and that no objects are accessed across threads. "...any global static data stored in the addon must be properly protected, and must not contain any persistent references to JavaScript objects."

I don't have the bandwidth to add this now, but I'd really like to use worker threads for my use case, so I might revisit this at a later time if no one else picks it up.

For now, it does seem like your patch allows the library to be used inside workers (I was able to use it successfully), but could possibly cause memory leaks or crashes due to cross-thread object usage.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants