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

feat: implement worker timeout #78

Merged
merged 3 commits into from
Jul 16, 2024

Conversation

chrisdickinson
Copy link
Contributor

This applies the timeout to both the initialization of the plugin and execution of plugin methods. It turns out that Node was hanging because we were leaving MessagePort instances alive; so everywhere we instantiate a worker we race the createWorker call against a timer. If the timer wins, we terminate the worker. This appears to work, even with Wasm running in a tight loop!

Additionally: accept manifest options in snakeCase and accept memory settings via plugin options.

Fixes #74.

This applies the timeout to both the initialization of the plugin and execution
of plugin methods. We have to be a little careful about leaving `MessagePort`
instances alive in Node.

Additionally: accept manifest options in snakeCase and accept memory settings
via plugin options.

Fixes #74.
It looks like the promise returned by `worker.terminate()` doesn't
ref() properly in Bun. We can work around this by keeping a timer
alive while we terminate the worker process.

However, that reveals that Bun appears not to be able to handle
halting Wasm execution in a thread. So -- for now, at least --
disable timeouts in Bun.
Copy link
Collaborator

@mhmd-azeez mhmd-azeez left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Excellent job! I'd love to see a loom of how you went about debugging the issue and finding the root cause

Copy link
Contributor

@zshipko zshipko left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Wow, this is really great work! Excited to have suppport for cancellation in the js-sdk 🎉

@chrisdickinson chrisdickinson merged commit 02af79a into main Jul 16, 2024
4 checks passed
@chrisdickinson chrisdickinson deleted the chris/20240715-implement-timeout branch July 16, 2024 17:19
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.

Support timeout in manifest
3 participants