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

Debug-only dynamic checks for layout and GC use of DOMRefCell #3797

Merged
merged 6 commits into from Oct 25, 2014

Ignore the HTML parser's borrow flag in GC tracing

Adds some other dynamic checks in debug builds.
  • Loading branch information
kmcallister committed Oct 24, 2014
commit 49234484d6539a4d8df8374a9548c2004b8e68b7
@@ -6,6 +6,7 @@ use dom::bindings::trace::JSTraceable;
use js::jsapi::{JSTracer};

use servo_util::task_state;
use servo_util::task_state::{Script, InGC};

use std::cell::{Cell, UnsafeCell};
use std::kinds::marker;
@@ -33,6 +34,23 @@ impl<T> DOMRefCell<T> {
&*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.
#[cfg(not(ndebug))]
pub fn is_mutably_borrowed(&self) -> bool {
self.borrow.get() == WRITING
}

pub fn try_borrow<'a>(&'a self) -> Option<Ref<'a, T>> {
debug_assert!(task_state::get().is_script());
match self.borrow.get() {
@@ -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<TrustedNodeAddress> 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<TrustedNodeAddress>;

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);
}
}
}
@@ -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<ServoHTMLParser> = *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);

@@ -60,6 +60,7 @@ 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;
@@ -285,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,
@@ -376,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)
}

@@ -22,6 +22,8 @@ bitflags! {
static Render = 0x04,

static InWorker = 0x0100,
static InGC = 0x0200,
static InHTMLParser = 0x0400,
}
}

ProTip! Use n and p to navigate between commits in a pull request.
You can’t perform that action at this time.