diff --git a/components/gfx/render_task.rs b/components/gfx/render_task.rs index 976a574f200..0662eef3d99 100644 --- a/components/gfx/render_task.rs +++ b/components/gfx/render_task.rs @@ -30,6 +30,7 @@ use servo_util::geometry; use servo_util::opts; use servo_util::smallvec::{SmallVec, SmallVec1}; use servo_util::task::spawn_named_with_send_on_failure; +use servo_util::task_state; use servo_util::time::{TimeProfilerChan, profile}; use servo_util::time; use std::comm::{Receiver, Sender, channel}; @@ -151,7 +152,7 @@ impl RenderTask where C: RenderListener + Send { time_profiler_chan: TimeProfilerChan, shutdown_chan: Sender<()>) { let ConstellationChan(c) = constellation_chan.clone(); - spawn_named_with_send_on_failure("RenderTask", proc() { + spawn_named_with_send_on_failure("RenderTask", task_state::Render, proc() { { // Ensures RenderTask and graphics context are destroyed before shutdown msg let native_graphics_context = compositor.get_graphics_metadata().map( |md| NativePaintingGraphicsContext::from_metadata(&md)); diff --git a/components/layout/layout_task.rs b/components/layout/layout_task.rs index 114f9b85e84..db47f9e5801 100644 --- a/components/layout/layout_task.rs +++ b/components/layout/layout_task.rs @@ -53,6 +53,7 @@ use servo_util::logical_geometry::LogicalPoint; use servo_util::opts; use servo_util::smallvec::{SmallVec, SmallVec1, VecLike}; use servo_util::task::spawn_named_with_send_on_failure; +use servo_util::task_state; use servo_util::time::{TimeProfilerChan, profile}; use servo_util::time; use servo_util::workqueue::WorkQueue; @@ -181,7 +182,7 @@ impl LayoutTaskFactory for LayoutTask { time_profiler_chan: TimeProfilerChan, shutdown_chan: Sender<()>) { let ConstellationChan(con_chan) = constellation_chan.clone(); - spawn_named_with_send_on_failure("LayoutTask", proc() { + spawn_named_with_send_on_failure("LayoutTask", task_state::Layout, proc() { { // Ensures layout task is destroyed before we send shutdown message let sender = chan.sender(); let layout = @@ -251,7 +252,8 @@ impl LayoutTask { let screen_size = Size2D(Au(0), Au(0)); let device = Device::new(Screen, opts::get().initial_window_size.as_f32()); let parallel_traversal = if opts::get().layout_threads != 1 { - Some(WorkQueue::new("LayoutWorker", opts::get().layout_threads, ptr::null())) + Some(WorkQueue::new("LayoutWorker", task_state::Layout, + opts::get().layout_threads, ptr::null())) } else { None }; diff --git a/components/script/dom/bindings/cell.rs b/components/script/dom/bindings/cell.rs index a4a15b7d5e7..18385b66f39 100644 --- a/components/script/dom/bindings/cell.rs +++ b/components/script/dom/bindings/cell.rs @@ -5,64 +5,164 @@ use dom::bindings::trace::JSTraceable; use js::jsapi::{JSTracer}; -use std::cell; -use std::cell::RefCell; -use std::mem; +use servo_util::task_state; +use servo_util::task_state::{Script, InGC}; -/// A mutable field in DOM for large sized value. -/// This has a special method to return the pointer of itself -/// for used in layout task. -/// This simply wraps `RefCell` to add the special method. +use std::cell::{Cell, UnsafeCell}; +use std::kinds::marker; + +/// A mutable field in the DOM. +/// +/// This extends the API of `core::cell::RefCell` to allow unsafe access in +/// certain situations, with dynamic checking in debug builds. pub struct DOMRefCell { - base: RefCell, + value: UnsafeCell, + borrow: Cell, + nocopy: marker::NoCopy, + nosync: marker::NoSync, } -pub type Ref<'a, T> = cell::Ref<'a, T>; -pub type RefMut<'a, T> = cell::RefMut<'a, T>; - +// Functionality specific to Servo's `DOMRefCell` type +// =================================================== impl DOMRefCell { - #[inline(always)] - pub fn new(value: T) -> DOMRefCell { - DOMRefCell { - base: RefCell::new(value), + /// Return a reference to the contents. + /// + /// For use in the layout task only. + pub unsafe fn borrow_for_layout<'a>(&'a self) -> &'a T { + debug_assert!(task_state::get().is_layout()); + &*self.value.get() + } + + /// Borrow the contents for the purpose of GC tracing. + /// + /// This succeeds even if the object is mutably borrowed, + /// so you have to be careful in trace code! + pub unsafe fn borrow_for_gc_trace<'a>(&'a self) -> &'a T { + debug_assert!(task_state::get().contains(Script | InGC)); + &*self.value.get() + } + + /// Is the cell mutably borrowed? + /// + /// For safety checks in debug builds only. + pub fn is_mutably_borrowed(&self) -> bool { + self.borrow.get() == WRITING + } + + pub fn try_borrow<'a>(&'a self) -> Option> { + debug_assert!(task_state::get().is_script()); + match self.borrow.get() { + WRITING => None, + borrow => { + self.borrow.set(borrow + 1); + Some(Ref { _parent: self }) + } } } - #[inline(always)] - pub fn unwrap(self) -> T { - self.base.unwrap() - } - - #[inline(always)] - pub fn try_borrow<'a>(&'a self) -> Option> { - self.base.try_borrow() - } - - #[inline(always)] - pub fn borrow<'a>(&'a self) -> Ref<'a, T> { - self.base.borrow() - } - - #[inline(always)] pub fn try_borrow_mut<'a>(&'a self) -> Option> { - self.base.try_borrow_mut() - } - - #[inline(always)] - pub fn borrow_mut<'a>(&'a self) -> RefMut<'a, T> { - self.base.borrow_mut() - } - - /// This returns the pointer which refers T in `RefCell` directly. - pub unsafe fn borrow_for_layout<'a>(&'a self) -> &'a T { - let val = mem::transmute::<&RefCell, &T>(&self.base); - val + debug_assert!(task_state::get().is_script()); + match self.borrow.get() { + UNUSED => { + self.borrow.set(WRITING); + Some(RefMut { _parent: self }) + }, + _ => None + } } } impl JSTraceable for DOMRefCell { fn trace(&self, trc: *mut JSTracer) { - (*self).base.borrow().trace(trc) + (*self).borrow().trace(trc) + } +} + +// Functionality duplicated with `core::cell::RefCell` +// =================================================== +// +// This can shrink once rust-lang/rust#18131 is fixed. + +// Values [1, MAX-1] represent the number of `Ref` active +// (will not outgrow its range since `uint` is the size of the address space) +type BorrowFlag = uint; +static UNUSED: BorrowFlag = 0; +static WRITING: BorrowFlag = -1; + +impl DOMRefCell { + pub fn new(value: T) -> DOMRefCell { + DOMRefCell { + value: UnsafeCell::new(value), + borrow: Cell::new(UNUSED), + nocopy: marker::NoCopy, + nosync: marker::NoSync, + } + } + + pub fn unwrap(self) -> T { + debug_assert!(self.borrow.get() == UNUSED); + unsafe{self.value.unwrap()} + } + + pub fn borrow<'a>(&'a self) -> Ref<'a, T> { + match self.try_borrow() { + Some(ptr) => ptr, + None => fail!("DOMRefCell already mutably borrowed") + } + } + + pub fn borrow_mut<'a>(&'a self) -> RefMut<'a, T> { + match self.try_borrow_mut() { + Some(ptr) => ptr, + None => fail!("DOMRefCell already borrowed") + } + } +} + +pub struct Ref<'b, T:'b> { + _parent: &'b DOMRefCell +} + +#[unsafe_destructor] +impl<'b, T> Drop for Ref<'b, T> { + fn drop(&mut self) { + let borrow = self._parent.borrow.get(); + debug_assert!(borrow != WRITING && borrow != UNUSED); + self._parent.borrow.set(borrow - 1); + } +} + +impl<'b, T> Deref for Ref<'b, T> { + #[inline] + fn deref<'a>(&'a self) -> &'a T { + unsafe { &*self._parent.value.get() } + } +} + +pub struct RefMut<'b, T:'b> { + _parent: &'b DOMRefCell +} + +#[unsafe_destructor] +impl<'b, T> Drop for RefMut<'b, T> { + fn drop(&mut self) { + let borrow = self._parent.borrow.get(); + debug_assert!(borrow == WRITING); + self._parent.borrow.set(UNUSED); + } +} + +impl<'b, T> Deref for RefMut<'b, T> { + #[inline] + fn deref<'a>(&'a self) -> &'a T { + unsafe { &*self._parent.value.get() } + } +} + +impl<'b, T> DerefMut for RefMut<'b, T> { + #[inline] + fn deref_mut<'a>(&'a mut self) -> &'a mut T { + unsafe { &mut *self._parent.value.get() } } } diff --git a/components/script/dom/dedicatedworkerglobalscope.rs b/components/script/dom/dedicatedworkerglobalscope.rs index 84fc77d0249..76c30db5b4c 100644 --- a/components/script/dom/dedicatedworkerglobalscope.rs +++ b/components/script/dom/dedicatedworkerglobalscope.rs @@ -24,6 +24,8 @@ use script_task::WorkerPostMessage; use script_task::StackRootTLS; use servo_net::resource_task::{ResourceTask, load_whole_resource}; +use servo_util::task_state; +use servo_util::task_state::{Script, InWorker}; use js::glue::JS_STRUCTURED_CLONE_VERSION; use js::jsapi::{JSContext, JS_ReadStructuredClone, JS_WriteStructuredClone, JS_ClearPendingException}; @@ -90,6 +92,9 @@ impl DedicatedWorkerGlobalScope { .native() .named(format!("Web Worker at {}", worker_url.serialize())) .spawn(proc() { + + task_state::initialize(Script | InWorker); + let roots = RootCollection::new(); let _stack_roots_tls = StackRootTLS::new(&roots); diff --git a/components/script/dom/htmlimageelement.rs b/components/script/dom/htmlimageelement.rs index bc0e58db9be..635839805c3 100644 --- a/components/script/dom/htmlimageelement.rs +++ b/components/script/dom/htmlimageelement.rs @@ -90,7 +90,7 @@ pub trait LayoutHTMLImageElementHelpers { impl LayoutHTMLImageElementHelpers for JS { unsafe fn image(&self) -> Option { - (*self.unsafe_get()).image.borrow().clone() + (*self.unsafe_get()).image.borrow_for_layout().clone() } } diff --git a/components/script/dom/servohtmlparser.rs b/components/script/dom/servohtmlparser.rs index 6b81ab512c5..88f0c9c8275 100644 --- a/components/script/dom/servohtmlparser.rs +++ b/components/script/dom/servohtmlparser.rs @@ -15,6 +15,8 @@ use dom::node::TrustedNodeAddress; use dom::document::{Document, DocumentHelpers}; use parse::html::JSMessage; +use servo_util::task_state; + use std::default::Default; use url::Url; use js::jsapi::JSTracer; @@ -91,15 +93,23 @@ impl tree_builder::Tracer for Tracer { impl JSTraceable for ServoHTMLParser { fn trace(&self, trc: *mut JSTracer) { + self.reflector_.trace(trc); + let tracer = Tracer { trc: trc, }; let tracer = &tracer as &tree_builder::Tracer; - self.reflector_.trace(trc); - let tokenizer = self.tokenizer.borrow(); - let tree_builder = tokenizer.sink(); - tree_builder.trace_handles(tracer); - tree_builder.sink().trace(trc); + unsafe { + // Assertion: If the parser is mutably borrowed, we're in the + // parsing code paths. + debug_assert!(task_state::get().contains(task_state::InHTMLParser) + || !self.tokenizer.is_mutably_borrowed()); + + let tokenizer = self.tokenizer.borrow_for_gc_trace(); + let tree_builder = tokenizer.sink(); + tree_builder.trace_handles(tracer); + tree_builder.sink().trace(trc); + } } } diff --git a/components/script/parse/html.rs b/components/script/parse/html.rs index 933202e81f6..8eca823236a 100644 --- a/components/script/parse/html.rs +++ b/components/script/parse/html.rs @@ -25,6 +25,8 @@ use encoding::types::{Encoding, DecodeReplace}; use servo_net::resource_task::{Load, LoadData, Payload, Done, ResourceTask, load_whole_resource}; use servo_msg::constellation_msg::LoadData as MsgLoadData; use servo_util::task::spawn_named; +use servo_util::task_state; +use servo_util::task_state::InHTMLParser; use servo_util::str::DOMString; use std::ascii::StrAsciiExt; use std::comm::{channel, Sender, Receiver}; @@ -480,6 +482,8 @@ pub fn parse_html(page: &Page, let parser = ServoHTMLParser::new(js_chan.clone(), base_url.clone(), document).root(); let parser: JSRef = *parser; + task_state::enter(InHTMLParser); + match input { InputString(s) => { parser.tokenizer().borrow_mut().feed(s); @@ -512,6 +516,8 @@ pub fn parse_html(page: &Page, parser.tokenizer().borrow_mut().end(); + task_state::exit(InHTMLParser); + debug!("finished parsing"); js_chan.send(JSTaskExit); diff --git a/components/script/script_task.rs b/components/script/script_task.rs index f07fff94983..a5658ac6c5e 100644 --- a/components/script/script_task.rs +++ b/components/script/script_task.rs @@ -54,11 +54,13 @@ use servo_net::resource_task::ResourceTask; use servo_util::geometry::to_frac_px; use servo_util::smallvec::{SmallVec1, SmallVec}; use servo_util::task::spawn_named_with_send_on_failure; +use servo_util::task_state; use geom::point::Point2D; use js::jsapi::{JS_SetWrapObjectCallbacks, JS_SetGCZeal, JS_DEFAULT_ZEAL_FREQ, JS_GC}; use js::jsapi::{JSContext, JSRuntime, JSTracer}; use js::jsapi::{JS_SetGCParameter, JSGC_MAX_BYTES}; +use js::jsapi::{JS_SetGCCallback, JSGCStatus, JSGC_BEGIN, JSGC_END}; use js::rust::{Cx, RtUtils}; use js::rust::with_compartment; use js; @@ -262,7 +264,7 @@ impl ScriptTaskFactory for ScriptTask { let ConstellationChan(const_chan) = constellation_chan.clone(); let (script_chan, script_port) = channel(); let layout_chan = LayoutChan(layout_chan.sender()); - spawn_named_with_send_on_failure("ScriptTask", proc() { + spawn_named_with_send_on_failure("ScriptTask", task_state::Script, proc() { let script_task = ScriptTask::new(id, compositor as Box, layout_chan, @@ -284,6 +286,16 @@ impl ScriptTaskFactory for ScriptTask { } } +unsafe extern "C" fn debug_gc_callback(rt: *mut JSRuntime, status: JSGCStatus) { + js::rust::gc_callback(rt, status); + + match status { + JSGC_BEGIN => task_state::enter(task_state::InGC), + JSGC_END => task_state::exit(task_state::InGC), + _ => (), + } +} + impl ScriptTask { /// Creates a new script task. pub fn new(id: PipelineId, @@ -375,6 +387,13 @@ impl ScriptTask { JS_SetGCZeal((*js_context).ptr, 0, JS_DEFAULT_ZEAL_FREQ); } + // Needed for debug assertions about whether GC is running. + if !cfg!(ndebug) { + unsafe { + JS_SetGCCallback(js_runtime.ptr, Some(debug_gc_callback)); + } + } + (js_runtime, js_context) } diff --git a/components/util/lib.rs b/components/util/lib.rs index a53f3a1c3d9..0c5c5f65e55 100644 --- a/components/util/lib.rs +++ b/components/util/lib.rs @@ -52,6 +52,7 @@ pub mod task; pub mod tid; pub mod time; pub mod taskpool; +pub mod task_state; pub mod vec; pub mod workqueue; diff --git a/components/util/task.rs b/components/util/task.rs index b3e03771610..f286efe5bc3 100644 --- a/components/util/task.rs +++ b/components/util/task.rs @@ -8,22 +8,29 @@ use std::comm::Sender; use std::task::TaskBuilder; use native::task::NativeTaskBuilder; +use task_state; + pub fn spawn_named>(name: S, f: proc():Send) { let builder = task::TaskBuilder::new().named(name); builder.spawn(f); } -/// Arrange to send a particular message to a channel if the task built by -/// this `TaskBuilder` fails. +/// Arrange to send a particular message to a channel if the task fails. pub fn spawn_named_with_send_on_failure(name: &'static str, + state: task_state::TaskState, f: proc(): Send, msg: T, dest: Sender, native: bool) { + let with_state = proc() { + task_state::initialize(state); + f() + }; + let future_result = if native { - TaskBuilder::new().named(name).native().try_future(f) + TaskBuilder::new().named(name).native().try_future(with_state) } else { - TaskBuilder::new().named(name).try_future(f) + TaskBuilder::new().named(name).try_future(with_state) }; let watched_name = name.to_string(); diff --git a/components/util/task_state.rs b/components/util/task_state.rs new file mode 100644 index 00000000000..8ea42949aec --- /dev/null +++ b/components/util/task_state.rs @@ -0,0 +1,91 @@ +/* This Source Code Form is subject to the terms of the Mozilla Public + * License, v. 2.0. If a copy of the MPL was not distributed with this + * file, You can obtain one at http://mozilla.org/MPL/2.0/. */ + +//! Supports dynamic assertions in debug builds about what sort of task is +//! running and what state it's in. +//! +//! In release builds, `get` returns 0. All of the other functions inline +//! away to nothing. + +pub use self::imp::{initialize, get, enter, exit}; + +bitflags! { + #[deriving(Show)] + flags TaskState: u32 { + static Script = 0x01, + static Layout = 0x02, + static Render = 0x04, + + static InWorker = 0x0100, + static InGC = 0x0200, + static InHTMLParser = 0x0400, + } +} + +macro_rules! task_types ( ( $( $fun:ident = $flag:ident ; )* ) => ( + impl TaskState { + $( + pub fn $fun(self) -> bool { + self.contains($flag) + } + )* + } + + #[cfg(not(ndebug))] + static TYPES: &'static [TaskState] + = &[ $( $flag ),* ]; +)) + +task_types! { + is_script = Script; + is_layout = Layout; + is_render = Render; +} + +#[cfg(not(ndebug))] +mod imp { + use super::{TaskState, TYPES}; + + local_data_key!(STATE: TaskState) + + pub fn initialize(x: TaskState) { + match STATE.replace(Some(x)) { + None => (), + Some(s) => fail!("Task state already initialized as {}", s), + }; + get(); // check the assertion below + } + + pub fn get() -> TaskState { + let state = match STATE.get() { + None => fail!("Task state not initialized"), + Some(s) => *s, + }; + + // Exactly one of the task type flags should be set. + assert_eq!(1, TYPES.iter().filter(|&&ty| state.contains(ty)).count()); + state + } + + pub fn enter(x: TaskState) { + let state = get(); + assert!(!state.intersects(x)); + STATE.replace(Some(state | x)); + } + + pub fn exit(x: TaskState) { + let state = get(); + assert!(state.contains(x)); + STATE.replace(Some(state & !x)); + } +} + +#[cfg(ndebug)] +mod imp { + use super::TaskState; + #[inline(always)] pub fn initialize(_: TaskState) { } + #[inline(always)] pub fn get() -> TaskState { TaskState::empty() } + #[inline(always)] pub fn enter(_: TaskState) { } + #[inline(always)] pub fn exit(_: TaskState) { } +} diff --git a/components/util/workqueue.rs b/components/util/workqueue.rs index a9f9419d81c..3a0b3e32f91 100644 --- a/components/util/workqueue.rs +++ b/components/util/workqueue.rs @@ -7,6 +7,8 @@ //! Data associated with queues is simply a pair of unsigned integers. It is expected that a //! higher-level API on top of this could allow safe fork-join parallelism. +use task_state; + use native::task::NativeTaskBuilder; use rand::{Rng, XorShiftRng}; use std::mem; @@ -196,7 +198,10 @@ pub struct WorkQueue { impl WorkQueue { /// Creates a new work queue and spawns all the threads associated with /// it. - pub fn new(task_name: &'static str, thread_count: uint, user_data: QueueData) -> WorkQueue { + pub fn new(task_name: &'static str, + state: task_state::TaskState, + thread_count: uint, + user_data: QueueData) -> WorkQueue { // Set up data structures. let (supervisor_chan, supervisor_port) = channel(); let (mut infos, mut threads) = (vec!(), vec!()); @@ -231,6 +236,7 @@ impl WorkQueue { // Spawn threads. for thread in threads.into_iter() { TaskBuilder::new().named(task_name).native().spawn(proc() { + task_state::initialize(state | task_state::InWorker); let mut thread = thread; thread.start() })