Skip to content

Commit

Permalink
Dynamically check DOMRefCell access from layout in debug builds
Browse files Browse the repository at this point in the history
  • Loading branch information
kmcallister committed Oct 24, 2014
1 parent 0162214 commit 6ec0939
Show file tree
Hide file tree
Showing 9 changed files with 150 additions and 30 deletions.
3 changes: 2 additions & 1 deletion components/gfx/render_task.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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};
Expand Down Expand Up @@ -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));
Expand Down
6 changes: 4 additions & 2 deletions components/layout/layout_task.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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 =
Expand Down Expand Up @@ -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
};
Expand Down
47 changes: 26 additions & 21 deletions components/script/dom/bindings/cell.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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>,
Expand All @@ -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> {
Expand Down Expand Up @@ -63,33 +88,13 @@ 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,
None => fail!("DOMRefCell<T> already mutably borrowed")
}
}

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,
Expand Down
5 changes: 5 additions & 0 deletions components/script/dom/dedicatedworkerglobalscope.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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};
Expand Down Expand Up @@ -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);

Expand Down
3 changes: 2 additions & 1 deletion components/script/script_task.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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};
Expand Down Expand Up @@ -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,
Expand Down
1 change: 1 addition & 0 deletions components/util/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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;

Expand Down
15 changes: 11 additions & 4 deletions components/util/task.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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();
Expand Down
92 changes: 92 additions & 0 deletions components/util/task_state.rs
Original file line number Diff line number Diff line change
@@ -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) { }
}
8 changes: 7 additions & 1 deletion components/util/workqueue.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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!());
Expand Down Expand Up @@ -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()
})
Expand Down

0 comments on commit 6ec0939

Please sign in to comment.