Skip to content

Commit

Permalink
Ignore the HTML parser's borrow flag in GC tracing
Browse files Browse the repository at this point in the history
Adds some other dynamic checks in debug builds.
  • Loading branch information
kmcallister committed Oct 24, 2014
1 parent 6ec0939 commit 4923448
Show file tree
Hide file tree
Showing 5 changed files with 59 additions and 5 deletions.
18 changes: 18 additions & 0 deletions components/script/dom/bindings/cell.rs
Expand Up @@ -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;
Expand Down Expand Up @@ -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() {
Expand Down
20 changes: 15 additions & 5 deletions components/script/dom/servohtmlparser.rs
Expand Up @@ -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;
Expand Down Expand Up @@ -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);
}
}
}
6 changes: 6 additions & 0 deletions components/script/parse/html.rs
Expand Up @@ -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};
Expand Down Expand Up @@ -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);
Expand Down Expand Up @@ -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);

Expand Down
18 changes: 18 additions & 0 deletions components/script/script_task.rs
Expand Up @@ -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;
Expand Down Expand Up @@ -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,
Expand Down Expand Up @@ -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)
}

Expand Down
2 changes: 2 additions & 0 deletions components/util/task_state.rs
Expand Up @@ -22,6 +22,8 @@ bitflags! {
static Render = 0x04,

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

Expand Down

0 comments on commit 4923448

Please sign in to comment.