Mirrors/servo
1
0
Fork 0
mirror of https://github.com/servo/servo.git synced 2025-07-22 14:53:49 +01:00
Code Issues Projects Releases Packages Wiki Activity

Make LOCAL_CONTEXT_KEY safe and non-leaky.

Browse source 
`LOCAL_CONTEXT_KEY` is currently a `Cell<*mut LocalLayoutContext>`. The use
of the raw pointer means that the `LocalLayoutContext` is not dropped when
the thread dies; this leaks FreeType instances and probably other
things. There are also some unsafe getter functions in `LayoutContext`
(`font_context`, `applicable_declarations_cache` and
`style_sharing_candidate_cache`) that @eddyb says involve undefined
behaviour.

This changeset changes `LOCAL_CONTEXT_KEY` to
`RefCell<Option<Rc<LocalLayoutContext>>>`. This fixes the leak and also
results in safe getters.

(Fixes #6282.)
This commit is contained in:
Nicholas Nethercote 2015-06-04 20:08:19 -07:00
parent 2ca606aaba
commit 9b4d39d6d1
7 changed files with 38 additions and 48 deletions
Download patch file Download diff file Expand all files Collapse all files

6
components/layout/construct.rs
View file

@ -423,7 +423,7 @@ impl<'a> FlowConstructor<'a> {
// for runs might collapse so much whitespace away that only hypothetical fragments
// remain. In that case the inline flow will compute its ascent and descent to be zero.
let scanned_fragments =
TextRunScanner::new().scan_for_runs(self.layout_context.font_context(),
TextRunScanner::new().scan_for_runs(&mut self.layout_context.font_context(),
fragments.fragments);
let mut inline_flow_ref =
FlowRef::new(box InlineFlow::from_fragments(scanned_fragments,
@ -446,7 +446,7 @@ impl<'a> FlowConstructor<'a> {
let (ascent, descent) =
inline_flow.compute_minimum_ascent_and_descent(self.layout_context.font_context(),
inline_flow.compute_minimum_ascent_and_descent(&mut self.layout_context.font_context(),
&**node.style());
inline_flow.minimum_block_size_above_baseline = ascent;
inline_flow.minimum_depth_below_baseline = descent;
@ -1140,7 +1140,7 @@ impl<'a> FlowConstructor<'a> {
SpecificFragmentInfo::UnscannedText(
UnscannedTextFragmentInfo::from_text(text))));
let marker_fragments = TextRunScanner::new().scan_for_runs(
self.layout_context.font_context(),
&mut self.layout_context.font_context(),
unscanned_marker_fragments);
debug_assert!(marker_fragments.len() == 1);
marker_fragments.fragments.into_iter().next()

65
components/layout/context.rs
View file

@ -19,11 +19,10 @@ use net_traits::image::base::Image;
use net_traits::image_cache_task::{ImageCacheChan, ImageCacheTask, ImageResponse, ImageState};
use net_traits::image_cache_task::{UsePlaceholder};
use script::layout_interface::{Animation, LayoutChan, ReflowGoal};
use std::boxed;
use std::cell::Cell;
use std::cell::{RefCell, RefMut};
use std::collections::HashMap;
use std::collections::hash_state::DefaultState;
use std::ptr;
use std::rc::Rc;
use std::sync::{Arc, Mutex};
use std::sync::mpsc::{channel, Sender};
use style::selector_matching::Stylist;
@ -33,30 +32,31 @@ use util::geometry::Au;
use util::opts;
struct LocalLayoutContext {
font_context: FontContext,
applicable_declarations_cache: ApplicableDeclarationsCache,
style_sharing_candidate_cache: StyleSharingCandidateCache,
font_context: RefCell<FontContext>,
applicable_declarations_cache: RefCell<ApplicableDeclarationsCache>,
style_sharing_candidate_cache: RefCell<StyleSharingCandidateCache>,
}
thread_local!(static LOCAL_CONTEXT_KEY: Cell<*mut LocalLayoutContext> = Cell::new(ptr::null_mut()));
thread_local!(static LOCAL_CONTEXT_KEY: RefCell<Option<Rc<LocalLayoutContext>>> = RefCell::new(None));
fn create_or_get_local_context(shared_layout_context: &SharedLayoutContext)
-> *mut LocalLayoutContext {
LOCAL_CONTEXT_KEY.with(|ref r| {
if r.get().is_null() {
let context = box LocalLayoutContext {
font_context: FontContext::new(shared_layout_context.font_cache_task.clone()),
applicable_declarations_cache: ApplicableDeclarationsCache::new(),
style_sharing_candidate_cache: StyleSharingCandidateCache::new(),
};
r.set(boxed::into_raw(context));
} else if shared_layout_context.screen_size_changed {
unsafe {
(*r.get()).applicable_declarations_cache.evict_all();
-> Rc<LocalLayoutContext> {
LOCAL_CONTEXT_KEY.with(|r| {
let mut r = r.borrow_mut();
if let Some(context) = r.clone() {
if shared_layout_context.screen_size_changed {
context.applicable_declarations_cache.borrow_mut().evict_all();
}
context
} else {
let context = Rc::new(LocalLayoutContext {
font_context: RefCell::new(FontContext::new(shared_layout_context.font_cache_task.clone())),
applicable_declarations_cache: RefCell::new(ApplicableDeclarationsCache::new()),
style_sharing_candidate_cache: RefCell::new(StyleSharingCandidateCache::new()),
});
*r = Some(context.clone());
context
}
r.get()
})
}
@ -121,7 +121,7 @@ unsafe impl Send for SharedLayoutContextWrapper {}
pub struct LayoutContext<'a> {
pub shared: &'a SharedLayoutContext,
cached_local_layout_context: *mut LocalLayoutContext,
cached_local_layout_context: Rc<LocalLayoutContext>,
}
impl<'a> LayoutContext<'a> {
@ -136,27 +136,18 @@ impl<'a> LayoutContext<'a> {
}
#[inline(always)]
pub fn font_context<'b>(&'b self) -> &'b mut FontContext {
unsafe {
let cached_context = &mut *self.cached_local_layout_context;
&mut cached_context.font_context
}
pub fn font_context(&self) -> RefMut<FontContext> {
self.cached_local_layout_context.font_context.borrow_mut()
}
#[inline(always)]
pub fn applicable_declarations_cache<'b>(&'b self) -> &'b mut ApplicableDeclarationsCache {
unsafe {
let cached_context = &mut *self.cached_local_layout_context;
&mut cached_context.applicable_declarations_cache
}
pub fn applicable_declarations_cache(&self) -> RefMut<ApplicableDeclarationsCache> {
self.cached_local_layout_context.applicable_declarations_cache.borrow_mut()
}
#[inline(always)]
pub fn style_sharing_candidate_cache<'b>(&'b self) -> &'b mut StyleSharingCandidateCache {
unsafe {
let cached_context = &mut *self.cached_local_layout_context;
&mut cached_context.style_sharing_candidate_cache
}
pub fn style_sharing_candidate_cache(&self) -> RefMut<StyleSharingCandidateCache> {
self.cached_local_layout_context.style_sharing_candidate_cache.borrow_mut()
}
pub fn get_or_request_image(&self, url: Url, use_placeholder: UsePlaceholder)

6
components/layout/fragment.rs
View file

@ -856,7 +856,7 @@ impl Fragment {
self.border_box.size,
SpecificFragmentInfo::UnscannedText(UnscannedTextFragmentInfo::from_text(
"…".to_owned()))));
let ellipsis_fragments = TextRunScanner::new().scan_for_runs(layout_context.font_context(),
let ellipsis_fragments = TextRunScanner::new().scan_for_runs(&mut layout_context.font_context(),
unscanned_ellipsis_fragments);
debug_assert!(ellipsis_fragments.len() == 1);
ellipsis_fragments.fragments.into_iter().next().unwrap()
@ -1005,7 +1005,7 @@ impl Fragment {
pub fn calculate_line_height(&self, layout_context: &LayoutContext) -> Au {
let font_style = self.style.get_font_arc();
let font_metrics = text::font_metrics_for_style(layout_context.font_context(), font_style);
let font_metrics = text::font_metrics_for_style(&mut layout_context.font_context(), font_style);
text::line_height_from_style(&*self.style, &font_metrics)
}
@ -1819,7 +1819,7 @@ impl Fragment {
// See CSS 2.1 § 10.8.1.
let block_flow = info.flow_ref.as_immutable_block();
let font_style = self.style.get_font_arc();
let font_metrics = text::font_metrics_for_style(layout_context.font_context(),
let font_metrics = text::font_metrics_for_style(&mut layout_context.font_context(),
font_style);
InlineMetrics::from_block_height(&font_metrics,
block_flow.base.position.size.block,

2
components/layout/generated_content.rs
View file

@ -429,7 +429,7 @@ fn render_text(layout_context: &LayoutContext,
info));
// FIXME(pcwalton): This should properly handle multiple marker fragments. This could happen
// due to text run splitting.
let fragments = TextRunScanner::new().scan_for_runs(layout_context.font_context(), fragments);
let fragments = TextRunScanner::new().scan_for_runs(&mut layout_context.font_context(), fragments);
debug_assert!(fragments.len() >= 1);
fragments.fragments.into_iter().next().unwrap().specific
}

1
components/layout/layout_task.rs
View file

@ -312,7 +312,6 @@ impl LayoutTask {
let reporter_name = format!("layout-reporter-{}", id.0);
mem_profiler_chan.send(mem::ProfilerMsg::RegisterReporter(reporter_name.clone(), reporter));
// Create the channel on which new animations can be sent.
let (new_animations_sender, new_animations_receiver) = channel();
let (image_cache_sender, image_cache_receiver) = channel();

2
components/layout/list_item.rs
View file

@ -106,7 +106,7 @@ impl Flow for ListItemFlow {
marker.assign_replaced_block_size_if_necessary(containing_block_block_size);
let font_metrics =
text::font_metrics_for_style(layout_context.font_context(),
text::font_metrics_for_style(&mut layout_context.font_context(),
marker.style.get_font_arc());
let line_height = text::line_height_from_style(&*marker.style, &font_metrics);
let item_inline_metrics = InlineMetrics::from_font_metrics(&font_metrics, line_height);

4
components/layout/traversal.rs
View file

@ -154,7 +154,7 @@ impl<'a> PreorderDomTraversal for RecalcStyleForNode<'a> {
// Check to see whether we can share a style with someone.
let style_sharing_candidate_cache =
self.layout_context.style_sharing_candidate_cache();
&mut self.layout_context.style_sharing_candidate_cache();
let sharing_result = unsafe {
node.share_style_if_possible(style_sharing_candidate_cache,
parent_opt.clone())
@ -181,7 +181,7 @@ impl<'a> PreorderDomTraversal for RecalcStyleForNode<'a> {
node.cascade_node(self.layout_context.shared,
parent_opt,
&applicable_declarations,
self.layout_context.applicable_declarations_cache(),
&mut self.layout_context.applicable_declarations_cache(),
&self.layout_context.shared.new_animations_sender);
}