servo: Merge #15118 - Use Heap instead of UnsafeCell in DOM reflectors (from jdm:reflector-barrier-crash); r=Ms2ger

The previous `Reflector` implementation did not use post barriers, so we could crash when storing nursery objects in a `Reflector` structure that were later moved out of the nursery.

- [X] `./mach build -d` does not report any errors
- [X] `./mach test-tidy` does not report any errors
- [X] These changes fix #15085
- [X] There are tests for these changes

Source-Repo: https://github.com/servo/servo
Source-Revision: 023a9c55ec84413b9c097c4963f5c3e1b4885eb9
This commit is contained in:
Josh Matthews 2017-01-24 14:11:50 -08:00
parent 0fbd723e7f
commit fe61c14b38
3 changed files with 18 additions and 26 deletions

View File

@ -7,9 +7,8 @@
use dom::bindings::conversions::DerivedFrom; use dom::bindings::conversions::DerivedFrom;
use dom::bindings::js::Root; use dom::bindings::js::Root;
use dom::globalscope::GlobalScope; use dom::globalscope::GlobalScope;
use js::jsapi::{HandleObject, JSContext, JSObject}; use js::jsapi::{HandleObject, JSContext, JSObject, Heap};
use std::cell::UnsafeCell; use std::default::Default;
use std::ptr;
/// Create the reflector for a new DOM object and yield ownership to the /// Create the reflector for a new DOM object and yield ownership to the
/// reflector. /// reflector.
@ -34,13 +33,13 @@ pub fn reflect_dom_object<T, U>(
// If you're renaming or moving this field, update the path in plugins::reflector as well // If you're renaming or moving this field, update the path in plugins::reflector as well
pub struct Reflector { pub struct Reflector {
#[ignore_heap_size_of = "defined and measured in rust-mozjs"] #[ignore_heap_size_of = "defined and measured in rust-mozjs"]
object: UnsafeCell<*mut JSObject>, object: Heap<*mut JSObject>,
} }
#[allow(unrooted_must_root)] #[allow(unrooted_must_root)]
impl PartialEq for Reflector { impl PartialEq for Reflector {
fn eq(&self, other: &Reflector) -> bool { fn eq(&self, other: &Reflector) -> bool {
unsafe { *self.object.get() == *other.object.get() } self.object.get() == other.object.get()
} }
} }
@ -48,30 +47,27 @@ impl Reflector {
/// Get the reflector. /// Get the reflector.
#[inline] #[inline]
pub fn get_jsobject(&self) -> HandleObject { pub fn get_jsobject(&self) -> HandleObject {
unsafe { HandleObject::from_marked_location(self.object.get()) } self.object.handle()
} }
/// Initialize the reflector. (May be called only once.) /// Initialize the reflector. (May be called only once.)
pub fn set_jsobject(&mut self, object: *mut JSObject) { pub fn set_jsobject(&mut self, object: *mut JSObject) {
unsafe { assert!(self.object.get().is_null());
let obj = self.object.get(); assert!(!object.is_null());
assert!((*obj).is_null()); self.object.set(object);
assert!(!object.is_null());
*obj = object;
}
} }
/// Return a pointer to the memory location at which the JS reflector /// Return a pointer to the memory location at which the JS reflector
/// object is stored. Used to root the reflector, as /// object is stored. Used to root the reflector, as
/// required by the JSAPI rooting APIs. /// required by the JSAPI rooting APIs.
pub fn rootable(&self) -> *mut *mut JSObject { pub fn rootable(&self) -> &Heap<*mut JSObject> {
self.object.get() &self.object
} }
/// Create an uninitialized `Reflector`. /// Create an uninitialized `Reflector`.
pub fn new() -> Reflector { pub fn new() -> Reflector {
Reflector { Reflector {
object: UnsafeCell::new(ptr::null_mut()), object: Heap::default(),
} }
} }
} }

View File

@ -20,7 +20,7 @@
//! calls `trace()` on the field. //! calls `trace()` on the field.
//! For example, for fields of type `JS<T>`, `JS<T>::trace()` calls //! For example, for fields of type `JS<T>`, `JS<T>::trace()` calls
//! `trace_reflector()`. //! `trace_reflector()`.
//! 4. `trace_reflector()` calls `JS_CallUnbarrieredObjectTracer()` with a //! 4. `trace_reflector()` calls `JS::TraceEdge()` with a
//! pointer to the `JSObject` for the reflector. This notifies the GC, which //! pointer to the `JSObject` for the reflector. This notifies the GC, which
//! will add the object to the graph, and will trace that object as well. //! will add the object to the graph, and will trace that object as well.
//! 5. When the GC finishes tracing, it [`finalizes`](../index.html#destruction) //! 5. When the GC finishes tracing, it [`finalizes`](../index.html#destruction)
@ -54,7 +54,7 @@ use hyper::method::Method;
use hyper::mime::Mime; use hyper::mime::Mime;
use hyper::status::StatusCode; use hyper::status::StatusCode;
use ipc_channel::ipc::{IpcReceiver, IpcSender}; use ipc_channel::ipc::{IpcReceiver, IpcSender};
use js::glue::{CallObjectTracer, CallUnbarrieredObjectTracer, CallValueTracer}; use js::glue::{CallObjectTracer, CallValueTracer};
use js::jsapi::{GCTraceKindToAscii, Heap, JSObject, JSTracer, TraceKind}; use js::jsapi::{GCTraceKindToAscii, Heap, JSObject, JSTracer, TraceKind};
use js::jsval::JSVal; use js::jsval::JSVal;
use js::rust::Runtime; use js::rust::Runtime;
@ -139,12 +139,8 @@ pub fn trace_jsval(tracer: *mut JSTracer, description: &str, val: &Heap<JSVal>)
/// Trace the `JSObject` held by `reflector`. /// Trace the `JSObject` held by `reflector`.
#[allow(unrooted_must_root)] #[allow(unrooted_must_root)]
pub fn trace_reflector(tracer: *mut JSTracer, description: &str, reflector: &Reflector) { pub fn trace_reflector(tracer: *mut JSTracer, description: &str, reflector: &Reflector) {
unsafe { trace!("tracing reflector {}", description);
trace!("tracing reflector {}", description); trace_object(tracer, description, reflector.rootable())
CallUnbarrieredObjectTracer(tracer,
reflector.rootable(),
GCTraceKindToAscii(TraceKind::Object));
}
} }
/// Trace a `JSObject`. /// Trace a `JSObject`.

View File

@ -90,12 +90,12 @@ impl Promise {
#[allow(unsafe_code, unrooted_must_root)] #[allow(unsafe_code, unrooted_must_root)]
unsafe fn new_with_js_promise(obj: HandleObject, cx: *mut JSContext) -> Rc<Promise> { unsafe fn new_with_js_promise(obj: HandleObject, cx: *mut JSContext) -> Rc<Promise> {
assert!(IsPromiseObject(obj)); assert!(IsPromiseObject(obj));
let mut promise = Promise { let promise = Promise {
reflector: Reflector::new(), reflector: Reflector::new(),
permanent_js_root: MutHeapJSVal::new(), permanent_js_root: MutHeapJSVal::new(),
}; };
promise.init_reflector(obj.get()); let mut promise = Rc::new(promise);
let promise = Rc::new(promise); Rc::get_mut(&mut promise).unwrap().init_reflector(obj.get());
promise.initialize(cx); promise.initialize(cx);
promise promise
} }