Skip to content

Commit

Permalink
Adds borrow_for_script_deallocation and unsafe_mut_js_info method to …
Browse files Browse the repository at this point in the history
…avoid 'DOMRefCell already mutably borrowed' messages. This is just a temporary fix until the Rust standard library allows borrowing already-borrowed RefCell values during unwinding.

It also removes LiveDOMReferences destructor that it's a no-op but it contains an assert that was being violated causing an endless cycle of destructor calls ending up in a stack overflow.
  • Loading branch information
dmarcos committed Jan 30, 2015
1 parent 648b499 commit 7b9c902
Show file tree
Hide file tree
Showing 4 changed files with 17 additions and 10 deletions.
7 changes: 7 additions & 0 deletions components/script/dom/bindings/cell.rs
Expand Up @@ -41,6 +41,13 @@ impl<T> DOMRefCell<T> {
&*self.value.as_unsafe_cell().get()
}

/// Borrow the contents for the purpose of script deallocation.
///
pub unsafe fn borrow_for_script_deallocation<'a>(&'a self) -> &'a mut T {
debug_assert!(task_state::get().contains(SCRIPT));
&mut *self.value.as_unsafe_cell().get()
}

/// Is the cell mutably borrowed?
///
/// For safety checks in debug builds only.
Expand Down
6 changes: 0 additions & 6 deletions components/script/dom/bindings/refcounted.rs
Expand Up @@ -188,9 +188,3 @@ impl LiveDOMReferences {
})
}
}

impl Drop for LiveDOMReferences {
fn drop(&mut self) {
assert!(self.table.borrow().keys().count() == 0);
}
}
4 changes: 4 additions & 0 deletions components/script/page.rs
Expand Up @@ -273,6 +273,10 @@ impl Page {
self.js_info.borrow_mut()
}

pub unsafe fn unsafe_mut_js_info<'a>(&'a self) -> &'a mut Option<JSPageInfo> {
self.js_info.borrow_for_script_deallocation()
}

pub fn js_info<'a>(&'a self) -> Ref<'a, Option<JSPageInfo>> {
self.js_info.borrow()
}
Expand Down
10 changes: 6 additions & 4 deletions components/script/script_task.rs
Expand Up @@ -248,11 +248,13 @@ impl<'a> Drop for ScriptMemoryFailsafe<'a> {
fn drop(&mut self) {
match self.owner {
Some(owner) => {
let page = owner.page.borrow_mut();
for page in page.iter() {
*page.mut_js_info() = None;
unsafe {
let page = owner.page.borrow_for_script_deallocation();
for page in page.iter() {
*page.unsafe_mut_js_info() = None;
}
*owner.js_context.borrow_for_script_deallocation() = None;
}
*owner.js_context.borrow_mut() = None;
}
None => (),
}
Expand Down

5 comments on commit 7b9c902

@bors-servo
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

saw approval from jdm
at dmarcos@7b9c902

@bors-servo
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

merging dmarcos/servo/issue4692 = 7b9c902 into auto

@bors-servo
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

dmarcos/servo/issue4692 = 7b9c902 merged ok, testing candidate = a7e2993

@bors-servo
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@bors-servo
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

fast-forwarding master to auto = a7e2993

Please sign in to comment.