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
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 2 additions & 4 deletions crates/backend/src/codegen.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1042,11 +1042,9 @@ impl ToTokens for ast::ImportStatic {
fn init() -> #ty {
panic!("cannot access imported statics on non-wasm targets")
}
static mut _VAL: ::wasm_bindgen::__rt::core::cell::UnsafeCell<Option<#ty>> =
::wasm_bindgen::__rt::core::cell::UnsafeCell::new(None);
thread_local!(static _VAL: #ty = init(););
::wasm_bindgen::JsStatic {
__inner: unsafe { &_VAL },
__init: init,
__inner: &_VAL,
}
};
}).to_tokens(into);
Expand Down
2 changes: 1 addition & 1 deletion src/closure.rs
Original file line number Diff line number Diff line change
Expand Up @@ -214,7 +214,7 @@ impl<T> Closure<T>
};

Closure {
js: ManuallyDrop::new(JsValue { idx }),
js: ManuallyDrop::new(JsValue::_new(idx)),
data: ManuallyDrop::new(data),
}
}
Expand Down
4 changes: 2 additions & 2 deletions src/convert/impls.rs
Original file line number Diff line number Diff line change
Expand Up @@ -337,7 +337,7 @@ impl FromWasmAbi for JsValue {

#[inline]
unsafe fn from_abi(js: u32, _extra: &mut Stack) -> JsValue {
JsValue { idx: js }
JsValue::_new(js)
}
}

Expand All @@ -356,7 +356,7 @@ impl RefFromWasmAbi for JsValue {

#[inline]
unsafe fn ref_from_abi(js: u32, _extra: &mut Stack) -> Self::Anchor {
ManuallyDrop::new(JsValue { idx: js })
ManuallyDrop::new(JsValue::_new(js))
}
}

Expand Down
90 changes: 41 additions & 49 deletions src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -16,8 +16,8 @@ extern crate serde_json;

extern crate wasm_bindgen_macro;

use core::cell::UnsafeCell;
use core::fmt;
use core::marker;
use core::mem;
use core::ops::{Deref, DerefMut};
use core::ptr;
Expand Down Expand Up @@ -64,6 +64,7 @@ if_std! {
/// but for now it may be slightly slow.
pub struct JsValue {
idx: u32,
_marker: marker::PhantomData<*mut u8>, // not at all threadsafe
}

const JSIDX_UNDEFINED: u32 = 0;
Expand All @@ -74,18 +75,36 @@ const JSIDX_RESERVED: u32 = 8;

impl JsValue {
/// The `null` JS value constant.
pub const NULL: JsValue = JsValue { idx: JSIDX_NULL };
pub const NULL: JsValue = JsValue {
idx: JSIDX_NULL,
_marker: marker::PhantomData,
};

/// The `undefined` JS value constant.
pub const UNDEFINED: JsValue = JsValue {
idx: JSIDX_UNDEFINED,
_marker: marker::PhantomData,
};


/// The `true` JS value constant.
pub const TRUE: JsValue = JsValue { idx: JSIDX_TRUE };
pub const TRUE: JsValue = JsValue {
idx: JSIDX_TRUE,
_marker: marker::PhantomData,
};

/// The `false` JS value constant.
pub const FALSE: JsValue = JsValue { idx: JSIDX_FALSE };
pub const FALSE: JsValue = JsValue {
idx: JSIDX_FALSE,
_marker: marker::PhantomData,
};

fn _new(idx: u32) -> JsValue {
JsValue {
idx,
_marker: marker::PhantomData,
}
}

/// Creates a new JS value which is a string.
///
Expand All @@ -94,9 +113,7 @@ impl JsValue {
#[inline]
pub fn from_str(s: &str) -> JsValue {
unsafe {
JsValue {
idx: __wbindgen_string_new(s.as_ptr(), s.len()),
}
JsValue::_new(__wbindgen_string_new(s.as_ptr(), s.len()))
}
}

Expand All @@ -107,9 +124,7 @@ impl JsValue {
#[inline]
pub fn from_f64(n: f64) -> JsValue {
unsafe {
JsValue {
idx: __wbindgen_number_new(n),
}
JsValue::_new(__wbindgen_number_new(n))
}
}

Expand Down Expand Up @@ -142,9 +157,7 @@ impl JsValue {
unsafe {
let ptr = description.map(|s| s.as_ptr()).unwrap_or(ptr::null());
let len = description.map(|s| s.len()).unwrap_or(0);
JsValue {
idx: __wbindgen_symbol_new(ptr, len),
}
JsValue::_new(__wbindgen_symbol_new(ptr, len))
}
}

Expand All @@ -170,9 +183,7 @@ impl JsValue {
{
let s = serde_json::to_string(t)?;
unsafe {
Ok(JsValue {
idx: __wbindgen_json_parse(s.as_ptr(), s.len()),
})
Ok(JsValue::_new(__wbindgen_json_parse(s.as_ptr(), s.len())))
}
}

Expand Down Expand Up @@ -486,7 +497,7 @@ impl Clone for JsValue {
fn clone(&self) -> JsValue {
unsafe {
let idx = __wbindgen_object_clone_ref(self.idx);
JsValue { idx }
JsValue::_new(idx)
}
}
}
Expand Down Expand Up @@ -552,43 +563,26 @@ impl Drop for JsValue {
///
/// This type implements `Deref` to the inner type so it's typically used as if
/// it were `&T`.
#[cfg(feature = "std")]
pub struct JsStatic<T: 'static> {
#[doc(hidden)]
pub __inner: &'static UnsafeCell<Option<T>>,
#[doc(hidden)]
pub __init: fn() -> T,
pub __inner: &'static std::thread::LocalKey<T>,
}

unsafe impl<T: Sync> Sync for JsStatic<T> {}
unsafe impl<T: Send> Send for JsStatic<T> {}

#[cfg(feature = "std")]
impl<T: FromWasmAbi + 'static> Deref for JsStatic<T> {
type Target = T;
fn deref(&self) -> &T {
// We know that our tls key is never overwritten after initialization,
// so it should be safe (on that axis at least) to hand out a reference
// that lives longer than the closure below.
//
// FIXME: this is not sound if we ever implement thread exit hooks on
// wasm, as the pointer will eventually be invalidated but you can get
// `&'static T` from this interface. We... probably need to deprecate
// and/or remove this interface nowadays.
unsafe {
// Ideally we want to use `get_or_insert_with` here but
// unfortunately that has subpar codegen for now.
//
// If we get past the `Some` branch here LLVM statically
// knows that we're `None`, but the after the call to the `__init`
// function LLVM can no longer know this because `__init` could
// recursively call this function again (aka if JS came back to Rust
// and Rust referenced this static).
//
// We know, however, that cannot happen. As a result we can
// conclude that even after the call to `__init` our `ptr` still
// points to `None` (and a debug assertion to this effect). Then
// using `ptr::write` should tell rustc to not run destuctors
// (as one isn't there) and this should tighten up codegen for
// `JsStatic` a bit as well.
let ptr = self.__inner.get();
if let Some(ref t) = *ptr {
return t;
}
let init = Some((self.__init)());
debug_assert!((*ptr).is_none());
ptr::write(ptr, init);
(*ptr).as_ref().unwrap()
self.__inner.with(|ptr| &*(ptr as *const T))
}
}
}
Expand Down Expand Up @@ -642,9 +636,7 @@ pub fn throw_val(s: JsValue) -> ! {
/// Returns a handle to this wasm instance's `WebAssembly.Memory`
pub fn memory() -> JsValue {
unsafe {
JsValue {
idx: __wbindgen_memory(),
}
JsValue::_new(__wbindgen_memory())
}
}

Expand Down