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

Add additional documentation to spawn_local #4391

Closed
wants to merge 1 commit into from

Conversation

drewcrawford
Copy link

This adds documentation to help troubleshoot why spawn_local might not work as expected.

see #2945 and chemicstry/wasm_thread#6

Copy link
Collaborator

@daxpedda daxpedda left a comment

Choose a reason for hiding this comment

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

Events, queueMicrotask, setTimeout and Scheduler.postTask() are also affected by this so the issue isn't specific to spawn_local(). I think there needs to be a general understanding that blocking in Wasm is much more often than not a bad idea compared to native.

So its probably better to add documentation to functions that block instead to all possible functions that are affected by blocking. In the case of wasm-bindgen its js_sys::Atomics::wait*. Additionally some top-level documentation somewhere would help make this information be conveyed much more generally.

Thank you for looking into this and going through the effort of a PR!

@daxpedda daxpedda added the waiting for author Waiting for author to respond label Jan 7, 2025
@drewcrawford
Copy link
Author

The issue being addressed here is not caused by blocking, so wider awareness of blocking does not address the concern.

Rather, the issue is caused by not blocking: worker threads that never block never have their tasks executed (or stop executing suddenly).

If this is not appropriate to document then the PR should be closed.

@daxpedda
Copy link
Collaborator

Ah, I misunderstood the original issue!

Indeed, the issue that you are talking about has nothing to do with spawn_local() or wasm_bindgen. The issue is that wasm_thread decided to close workers after the main task is finished, to more closely mimic the behavior of std::thread. So this is a documentation issue on wasm_threads side.

I thought that the issue was that blocking a worker prevents tasks spawned with spawn_local & co. to never be run. Which is indeed something related to how JS works and probably could use some documentation in wasm_bindgen. But as you said, this is unrelated to this PR.

With that said: closing this PR then.

@daxpedda daxpedda closed this Jan 17, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
waiting for author Waiting for author to respond
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants