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

Ensure that JsValue isn't considered Send #955

Merged
merged 1 commit into from
Oct 11, 2018

Conversation

alexcrichton
Copy link
Contributor

The JsValue type wraps a slab/heap of js objects which is managed by
the wasm-bindgen shim, and everything here is not actually able to cross
any thread boundaries. When wasm actually has threads, for example, each
thread will have to have its own slab of objects generated by
wasm-bindgen, and indices in one slab aren't valid in any other slabs.

This is technically a breaking change because JsValue was previously
Send and Sync, but I'm hoping that in practice this isn't actually a
breaking change because nothing in wasm can be using threads which in
theory shouldn't activate the Send and/or Sync bounds.

The `JsValue` type wraps a slab/heap of js objects which is managed by
the wasm-bindgen shim, and everything here is not actually able to cross
any thread boundaries. When wasm actually has threads, for example, each
thread will have to have its own slab of objects generated by
wasm-bindgen, and indices in one slab aren't valid in any other slabs.

This is technically a breaking change because `JsValue` was previously
`Send` and `Sync`, but I'm hoping that in practice this isn't actually a
breaking change because nothing in wasm can be using threads which in
theory shouldn't activate the `Send` and/or `Sync` bounds.
Copy link
Member

@fitzgen fitzgen left a comment

Choose a reason for hiding this comment

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

yep

@alexcrichton alexcrichton merged commit e03e404 into rustwasm:master Oct 11, 2018
@alexcrichton alexcrichton deleted the non-send branch October 11, 2018 00:52
@upsuper
Copy link

upsuper commented Jan 22, 2019

Well, it is a breaking change since it breaks the use of lazy_static :)

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