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

fix: allow for nesting workers #40

Merged
merged 1 commit into from
Jan 4, 2024

Conversation

eshaz
Copy link
Contributor

@eshaz eshaz commented Jun 2, 2023

This fixes an issue where it's not possible to use this library inside of a NodeJS worker thread, since the check below is not accounting for where the thread originated from. When using this library within a worker thread, the workerThread() code is executed, rather than the mainThread() code which creates the worker polyfill.

export default threads.isMainThread ? mainThread() : workerThread();

This change executes workerThread() only if the mod property is present from the workerData, indicating that it's being executed from a thread created within this library. If mod isn't present, this means that the code is executing within the context of some other worker thread that wasn't created from this library, and it needs to treat the current thread as the main thread by executing mainThread().

One enhancement might be to randomize the mod property name. This change assumes that a parent thread won't send a mod property in the workerData...

@chanmathew
Copy link

chanmathew commented Nov 25, 2023

Hi @eshaz - any update on merging this? Would this also address #42? This should also be updated in your other package https://github.com/eshaz/wasm-audio-decoders which is using this as a dependency.

@developit developit merged commit 683e5f4 into developit:master Jan 4, 2024
@developit
Copy link
Owner

developit commented Jan 4, 2024

Thanks for taking the time to articulate the logic here, and sorry for taking forever to merge!

This was released in 1.3.0.

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

Successfully merging this pull request may close these issues.

3 participants