Dynamically check DOMRefCell access from layout in debug builds

9 files changed, 150 insertions, 30 deletions

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<C> RenderTask<C> 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 a8b8902f729..4aa993fec45 100644
--- a/components/script/dom/bindings/cell.rs
+++ b/components/script/dom/bindings/cell.rs
@@ -5,13 +5,15 @@
 use dom::bindings::trace::JSTraceable;
 use js::jsapi::{JSTracer};
 
+use servo_util::task_state;
+
 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.
+/// certain situations, with dynamic checking in debug builds.
 pub struct DOMRefCell<T> {
     value: UnsafeCell<T>,
     borrow: Cell<BorrowFlag>,
@@ -27,8 +29,31 @@ impl<T> DOMRefCell<T> {
     ///
     /// 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()
     }
+
+    pub fn try_borrow<'a>(&'a self) -> Option<Ref<'a, T>> {
+        debug_assert!(task_state::get().is_script());
+        match self.borrow.get() {
+            WRITING => None,
+            borrow => {
+                self.borrow.set(borrow + 1);
+                Some(Ref { _parent: self })
+            }
+        }
+    }
+
+    pub fn try_borrow_mut<'a>(&'a self) -> Option<RefMut<'a, T>> {
+        debug_assert!(task_state::get().is_script());
+        match self.borrow.get() {
+            UNUSED => {
+                self.borrow.set(WRITING);
+                Some(RefMut { _parent: self })
+            },
+            _ => None
+        }
+    }
 }
 
 impl<T: JSTraceable> JSTraceable for DOMRefCell<T> {
@@ -63,16 +88,6 @@ impl<T> DOMRefCell<T> {
         unsafe{self.value.unwrap()}
     }
 
-    pub fn try_borrow<'a>(&'a self) -> Option<Ref<'a, T>> {
-        match self.borrow.get() {
-            WRITING => None,
-            borrow => {
-                self.borrow.set(borrow + 1);
-                Some(Ref { _parent: self })
-            }
-        }
-    }
-
     pub fn borrow<'a>(&'a self) -> Ref<'a, T> {
         match self.try_borrow() {
             Some(ptr) => ptr,
@@ -80,16 +95,6 @@ impl<T> DOMRefCell<T> {
         }
     }
 
-    pub fn try_borrow_mut<'a>(&'a self) -> Option<RefMut<'a, T>> {
-        match self.borrow.get() {
-            UNUSED => {
-                self.borrow.set(WRITING);
-                Some(RefMut { _parent: self })
-            },
-            _ => None
-        }
-    }
-
     pub fn borrow_mut<'a>(&'a self) -> RefMut<'a, T> {
         match self.try_borrow_mut() {
             Some(ptr) => ptr,
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/script_task.rs b/components/script/script_task.rs
index f07fff94983..398e9b8d4db 100644
--- a/components/script/script_task.rs
+++ b/components/script/script_task.rs
@@ -54,6 +54,7 @@ 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};
@@ -262,7 +263,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<ScriptListener>,
                                               layout_chan,
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<S: IntoMaybeOwned<'static>>(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<T: Send>(name: &'static str,
+                                                 state: task_state::TaskState,
                                                  f: proc(): Send,
                                                  msg: T,
                                                  dest: Sender<T>,
                                                  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..7c714118b09
--- /dev/null
+++ b/components/util/task_state.rs
@@ -0,0 +1,92 @@
+/* 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` is not available; calls must be inside
+//! `debug_assert!` or similar.  All of the other functions inline away to
+//! nothing.
+
+pub use self::imp::{initialize, enter, exit};
+
+#[cfg(not(ndebug))]
+pub use self::imp::get;
+
+bitflags! {
+    #[deriving(Show)]
+    flags TaskState: u32 {
+        static Script        = 0x01,
+        static Layout        = 0x02,
+        static Render        = 0x04,
+
+        static InWorker      = 0x0100,
+    }
+}
+
+// Exactly one of these should be set.
+static task_types: &'static [TaskState]
+    = &[Script, Layout, Render];
+
+macro_rules! predicates ( ( $( $f:ident = $c:ident ; )* ) => (
+    impl TaskState {
+        $(
+            pub fn $f(self) -> bool {
+                self.contains($c)
+            }
+        )*
+    }
+))
+
+predicates! {
+    is_script = Script;
+    is_layout = Layout;
+    is_render = Render;
+}
+
+#[cfg(not(ndebug))]
+mod imp {
+    use super::{TaskState, task_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, task_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 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<QueueData, WorkData> {
 impl<QueueData: Send, WorkData: Send> WorkQueue<QueueData, WorkData> {
     /// 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<QueueData, WorkData> {
+    pub fn new(task_name: &'static str,
+               state: task_state::TaskState,
+               thread_count: uint,
+               user_data: QueueData) -> WorkQueue<QueueData, WorkData> {
         // Set up data structures.
         let (supervisor_chan, supervisor_port) = channel();
         let (mut infos, mut threads) = (vec!(), vec!());
@@ -231,6 +236,7 @@ impl<QueueData: Send, WorkData: Send> WorkQueue<QueueData, WorkData> {
         // 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()
             })