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

Implement a threadsafe version of wasm-bindgen-futures #1379

Closed
alexcrichton opened this issue Mar 21, 2019 · 4 comments · Fixed by #1514
Closed

Implement a threadsafe version of wasm-bindgen-futures #1379

alexcrichton opened this issue Mar 21, 2019 · 4 comments · Fixed by #1514
Labels
assigned This issue has been assigned to a contributor sprint

Comments

@alexcrichton
Copy link
Contributor

Currently the wasm-bindgen-futures crate provides a number of unsafe implementations to implement a runtime to execute futures within. These unsafe blocks, however, are predicated on the application being single threaded. The crate isn't safe in a threaded environment!

We should implement a threadsafe version of wasm-bindgen-futures, gated on the target_feature = "atomics" feature of WebAssembly!

@alexcrichton
Copy link
Contributor Author

For those interested in getting their feet wet with the multithreading story of Rust and WebAssembly, this is definitely a meaty project to take on!

@ibaryshnikov
Copy link
Member

So the task is to replace Cell and RefCell inside a Package struct? Would you mind if I try it?

@fitzgen fitzgen added the assigned This issue has been assigned to a contributor label Mar 21, 2019
@fitzgen
Copy link
Member

fitzgen commented Mar 21, 2019

@ibaryshnikov all yours!

I think that should do the trick, ignoring potential dead locks. I don't remember if there is unsafe code that needs to be updated too...

@alexcrichton
Copy link
Contributor Author

To solve this issue the unsafe code from crates/futures/src/lib.rs basically needs to be removed, notably these two unsafe impl blocks. Those are safe in a single-threaded scenario, but with multithreaded wasm those are legitimately unsafe.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
assigned This issue has been assigned to a contributor sprint
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants