diff --git a/Cargo.lock b/Cargo.lock index 8cdc27d20e1..08fce2cd728 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -976,6 +976,7 @@ source = "registry+https://github.com/rust-lang/crates.io-index" name = "fallible" version = "0.0.1" dependencies = [ + "hashglobe 0.1.0", "smallvec 0.4.3 (registry+https://github.com/rust-lang/crates.io-index)", ] diff --git a/components/fallible/Cargo.toml b/components/fallible/Cargo.toml index b9425d39acb..eae094d5776 100644 --- a/components/fallible/Cargo.toml +++ b/components/fallible/Cargo.toml @@ -11,3 +11,15 @@ path = "lib.rs" [dependencies] smallvec = "0.4" +hashglobe = { path = "../hashglobe" } + +# This crate effectively does nothing except if the `known_system_malloc` +# feature is specified. +# +# In that case, we actually call the system malloc functions to reserve space, +# otherwise we just let rust do its thing (aborting on OOM). +# +# This is effectively a stop-gap measure until we can do this properly in +# stable rust. +[features] +known_system_malloc = [] diff --git a/components/fallible/lib.rs b/components/fallible/lib.rs index 388a4de12bd..816e20573f9 100644 --- a/components/fallible/lib.rs +++ b/components/fallible/lib.rs @@ -2,14 +2,15 @@ * 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/. */ +extern crate hashglobe; extern crate smallvec; +use hashglobe::FailedAllocationError; use smallvec::Array; use smallvec::SmallVec; -use std::mem; -use std::ptr::copy_nonoverlapping; use std::vec::Vec; +#[cfg(feature = "known_system_malloc")] extern "C" { fn realloc(ptr: *mut u8, bytes: usize) -> *mut u8; fn malloc(bytes: usize) -> *mut u8; @@ -17,8 +18,8 @@ extern "C" { pub trait FallibleVec { /// Append |val| to the end of |vec|. Returns Ok(()) on success, - /// Err(()) if it fails, which can only be due to lack of memory. - fn try_push(&mut self, value: T) -> Result<(), ()>; + /// Err(reason) if it fails, with |reason| describing the failure. + fn try_push(&mut self, value: T) -> Result<(), FailedAllocationError>; } @@ -27,10 +28,13 @@ pub trait FallibleVec { impl FallibleVec for Vec { #[inline] - fn try_push(&mut self, val: T) -> Result<(), ()> { - if self.capacity() == self.len() { - try_double_vec(self)?; - debug_assert!(self.capacity() > self.len()); + fn try_push(&mut self, val: T) -> Result<(), FailedAllocationError> { + #[cfg(feature = "known_system_malloc")] + { + if self.capacity() == self.len() { + try_double_vec(self)?; + debug_assert!(self.capacity() > self.len()); + } } self.push(val); Ok(()) @@ -38,19 +42,28 @@ impl FallibleVec for Vec { } // Double the capacity of |vec|, or fail to do so due to lack of memory. -// Returns Ok(()) on success, Err(()) on failure. +// Returns Ok(()) on success, Err(..) on failure. +#[cfg(feature = "known_system_malloc")] #[inline(never)] #[cold] -fn try_double_vec(vec: &mut Vec) -> Result<(), ()> { +fn try_double_vec(vec: &mut Vec) -> Result<(), FailedAllocationError> { + use std::mem; + let old_ptr = vec.as_mut_ptr(); let old_len = vec.len(); let old_cap: usize = vec.capacity(); - let new_cap: usize = - if old_cap == 0 { 4 } else { old_cap.checked_mul(2).ok_or(()) ? }; + let new_cap: usize = if old_cap == 0 { + 4 + } else { + old_cap.checked_mul(2).ok_or(FailedAllocationError::new( + "capacity overflow for Vec", + ))? + }; - let new_size_bytes = - new_cap.checked_mul(mem::size_of::()).ok_or(()) ? ; + let new_size_bytes = new_cap.checked_mul(mem::size_of::()).ok_or( + FailedAllocationError::new("capacity overflow for Vec"), + )?; let new_ptr = unsafe { if old_cap == 0 { @@ -61,7 +74,9 @@ fn try_double_vec(vec: &mut Vec) -> Result<(), ()> { }; if new_ptr.is_null() { - return Err(()); + return Err(FailedAllocationError::new( + "out of memory when allocating Vec", + )); } let new_vec = unsafe { @@ -78,38 +93,53 @@ fn try_double_vec(vec: &mut Vec) -> Result<(), ()> { impl FallibleVec for SmallVec { #[inline] - fn try_push(&mut self, val: T::Item) -> Result<(), ()> { - if self.capacity() == self.len() { - try_double_small_vec(self)?; - debug_assert!(self.capacity() > self.len()); + fn try_push(&mut self, val: T::Item) -> Result<(), FailedAllocationError> { + #[cfg(feature = "known_system_malloc")] + { + if self.capacity() == self.len() { + try_double_small_vec(self)?; + debug_assert!(self.capacity() > self.len()); + } } self.push(val); Ok(()) } } -// Double the capacity of |vec|, or fail to do so due to lack of memory. -// Returns Ok(()) on success, Err(()) on failure. +// Double the capacity of |svec|, or fail to do so due to lack of memory. +// Returns Ok(()) on success, Err(..) on failure. +#[cfg(feature = "known_system_malloc")] #[inline(never)] #[cold] -fn try_double_small_vec(svec: &mut SmallVec) -> Result<(), ()> +fn try_double_small_vec(svec: &mut SmallVec) +-> Result<(), FailedAllocationError> where T: Array, { + use std::mem; + use std::ptr::copy_nonoverlapping; + let old_ptr = svec.as_mut_ptr(); let old_len = svec.len(); let old_cap: usize = svec.capacity(); - let new_cap: usize = - if old_cap == 0 { 4 } else { old_cap.checked_mul(2).ok_or(()) ? }; + let new_cap: usize = if old_cap == 0 { + 4 + } else { + old_cap.checked_mul(2).ok_or(FailedAllocationError::new( + "capacity overflow for SmallVec", + ))? + }; // This surely shouldn't fail, if |old_cap| was previously accepted as a // valid value. But err on the side of caution. - let old_size_bytes = - old_cap.checked_mul(mem::size_of::()).ok_or(()) ? ; + let old_size_bytes = old_cap.checked_mul(mem::size_of::()).ok_or( + FailedAllocationError::new("capacity overflow for SmallVec"), + )?; - let new_size_bytes = - new_cap.checked_mul(mem::size_of::()).ok_or(()) ? ; + let new_size_bytes = new_cap.checked_mul(mem::size_of::()).ok_or( + FailedAllocationError::new("capacity overflow for SmallVec"), + )?; let new_ptr; if svec.spilled() { @@ -130,7 +160,9 @@ where } if new_ptr.is_null() { - return Err(()); + return Err(FailedAllocationError::new( + "out of memory when allocating SmallVec", + )); } let new_vec = unsafe { diff --git a/components/hashglobe/src/lib.rs b/components/hashglobe/src/lib.rs index a8b6fb2ffb1..cbbdacb4c8d 100644 --- a/components/hashglobe/src/lib.rs +++ b/components/hashglobe/src/lib.rs @@ -33,6 +33,13 @@ pub struct FailedAllocationError { reason: &'static str, } +impl FailedAllocationError { + #[inline] + pub fn new(reason: &'static str) -> Self { + Self { reason } + } +} + impl error::Error for FailedAllocationError { fn description(&self) -> &str { self.reason diff --git a/components/style/Cargo.toml b/components/style/Cargo.toml index 7c46a2c69bd..d2c1b13892c 100644 --- a/components/style/Cargo.toml +++ b/components/style/Cargo.toml @@ -16,7 +16,7 @@ path = "lib.rs" doctest = false [features] -gecko = ["nsstring_vendor", "num_cpus", "style_traits/gecko"] +gecko = ["nsstring_vendor", "num_cpus", "style_traits/gecko", "fallible/known_system_malloc"] use_bindgen = ["bindgen", "regex", "toml"] servo = ["serde", "heapsize", "heapsize_derive", "style_traits/servo", "servo_atoms", "servo_config", "html5ever", diff --git a/components/style/invalidation/element/invalidation_map.rs b/components/style/invalidation/element/invalidation_map.rs index 58d612af122..eba4d028b8c 100644 --- a/components/style/invalidation/element/invalidation_map.rs +++ b/components/style/invalidation/element/invalidation_map.rs @@ -7,6 +7,8 @@ use {Atom, LocalName, Namespace}; use context::QuirksMode; use element_state::ElementState; +use fallible::FallibleVec; +use hashglobe::FailedAllocationError; use selector_map::{MaybeCaseInsensitiveHashMap, SelectorMap, SelectorMapEntry}; use selector_parser::SelectorImpl; use selectors::attr::NamespaceConstraint; @@ -184,12 +186,13 @@ impl InvalidationMap { }) } - /// Adds a selector to this `InvalidationMap`. + /// Adds a selector to this `InvalidationMap`. Returns Err(..) to + /// signify OOM. pub fn note_selector( &mut self, selector: &Selector, - quirks_mode: QuirksMode) - { + quirks_mode: QuirksMode + ) -> Result<(), FailedAllocationError> { self.collect_invalidations_for(selector, quirks_mode) } @@ -203,11 +206,12 @@ impl InvalidationMap { self.has_class_attribute_selectors = false; } + // Returns Err(..) to signify OOM. fn collect_invalidations_for( &mut self, selector: &Selector, - quirks_mode: QuirksMode) - { + quirks_mode: QuirksMode + ) -> Result<(), FailedAllocationError> { debug!("InvalidationMap::collect_invalidations_for({:?})", selector); let mut iter = selector.iter(); @@ -246,20 +250,20 @@ impl InvalidationMap { self.class_to_selector .entry(class, quirks_mode) .or_insert_with(SmallVec::new) - .push(Dependency { + .try_push(Dependency { selector: selector.clone(), selector_offset: sequence_start, - }) + })?; } for id in compound_visitor.ids { self.id_to_selector .entry(id, quirks_mode) .or_insert_with(SmallVec::new) - .push(Dependency { + .try_push(Dependency { selector: selector.clone(), selector_offset: sequence_start, - }) + })?; } if !compound_visitor.state.is_empty() { @@ -270,7 +274,7 @@ impl InvalidationMap { selector_offset: sequence_start, }, state: compound_visitor.state, - }, quirks_mode); + }, quirks_mode)?; } if compound_visitor.other_attributes { @@ -278,7 +282,7 @@ impl InvalidationMap { .insert(Dependency { selector: selector.clone(), selector_offset: sequence_start, - }, quirks_mode); + }, quirks_mode)?; } combinator = iter.next_sequence(); @@ -288,6 +292,8 @@ impl InvalidationMap { index += 1; // Account for the combinator. } + + Ok(()) } /// Measures heap usage. diff --git a/components/style/lib.rs b/components/style/lib.rs index e70a92d5bbc..b05150c39fc 100644 --- a/components/style/lib.rs +++ b/components/style/lib.rs @@ -47,6 +47,7 @@ extern crate bitflags; #[cfg(feature = "gecko")] #[macro_use] #[no_link] extern crate cfg_if; #[macro_use] extern crate cssparser; extern crate euclid; +extern crate fallible; extern crate fnv; #[cfg(feature = "gecko")] #[macro_use] pub mod gecko_string_cache; extern crate hashglobe; diff --git a/components/style/selector_map.rs b/components/style/selector_map.rs index 79b3e303503..873b7559576 100644 --- a/components/style/selector_map.rs +++ b/components/style/selector_map.rs @@ -9,8 +9,10 @@ use {Atom, LocalName}; use applicable_declarations::ApplicableDeclarationBlock; use context::QuirksMode; use dom::TElement; +use fallible::FallibleVec; use hash::{HashMap, HashSet}; use hash::map as hash_map; +use hashglobe::FailedAllocationError; use pdqsort::sort_by; use precomputed_hash::PrecomputedHash; use rule_tree::CascadeLevel; @@ -272,18 +274,20 @@ impl SelectorMap { impl SelectorMap { /// Inserts into the correct hash, trying id, class, and localname. - pub fn insert(&mut self, entry: T, quirks_mode: QuirksMode) { + pub fn insert( + &mut self, + entry: T, + quirks_mode: QuirksMode + ) -> Result<(), FailedAllocationError> { self.count += 1; let vector = match find_bucket(entry.selector()) { Bucket::ID(id) => { - self.id_hash - .entry(id.clone(), quirks_mode) + self.id_hash.try_entry(id.clone(), quirks_mode)? .or_insert_with(SmallVec::new) } Bucket::Class(class) => { - self.class_hash - .entry(class.clone(), quirks_mode) + self.class_hash.try_entry(class.clone(), quirks_mode)? .or_insert_with(SmallVec::new) } Bucket::LocalName { name, lower_name } => { @@ -300,12 +304,11 @@ impl SelectorMap { // subsequent selector matching work will filter them out. if name != lower_name { self.local_name_hash - .entry(lower_name.clone()) + .try_entry(lower_name.clone())? .or_insert_with(SmallVec::new) - .push(entry.clone()); + .try_push(entry.clone())?; } - self.local_name_hash - .entry(name.clone()) + self.local_name_hash.try_entry(name.clone())? .or_insert_with(SmallVec::new) } Bucket::Universal => { @@ -313,7 +316,7 @@ impl SelectorMap { } }; - vector.push(entry); + vector.try_push(entry) } /// Looks up entries by id, class, local name, and other (in order). @@ -510,6 +513,18 @@ impl MaybeCaseInsensitiveHashMap { self.0.entry(key) } + /// HashMap::try_entry + pub fn try_entry( + &mut self, + mut key: Atom, + quirks_mode: QuirksMode + ) -> Result, FailedAllocationError> { + if quirks_mode == QuirksMode::Quirks { + key = key.to_ascii_lowercase() + } + self.0.try_entry(key) + } + /// HashMap::iter pub fn iter(&self) -> hash_map::Iter { self.0.iter() diff --git a/components/style/stylesheets/stylesheet.rs b/components/style/stylesheets/stylesheet.rs index 6ca971ea89a..c647077f7e7 100644 --- a/components/style/stylesheets/stylesheet.rs +++ b/components/style/stylesheets/stylesheet.rs @@ -6,6 +6,7 @@ use {Prefix, Namespace}; use context::QuirksMode; use cssparser::{Parser, RuleListParser, ParserInput}; use error_reporting::{ParseErrorReporter, ContextualParseError}; +use fallible::FallibleVec; use fnv::FnvHashMap; use invalidation::media_queries::{MediaListKey, ToMediaListKey}; use media_queries::{MediaList, Device}; @@ -382,7 +383,14 @@ impl Stylesheet { while let Some(result) = iter.next() { match result { - Ok(rule) => rules.push(rule), + Ok(rule) => { + // Use a fallible push here, and if it fails, just + // fall out of the loop. This will cause the page to + // be shown incorrectly, but it's better than OOMing. + if rules.try_push(rule).is_err() { + break; + } + }, Err(err) => { let error = ContextualParseError::InvalidRule(err.slice, err.error); iter.parser.context.log_css_error(&iter.parser.error_context, diff --git a/components/style/stylist.rs b/components/style/stylist.rs index 29a0ae59a48..b8d6f1cb549 100644 --- a/components/style/stylist.rs +++ b/components/style/stylist.rs @@ -13,6 +13,7 @@ use element_state::ElementState; use font_metrics::FontMetricsProvider; #[cfg(feature = "gecko")] use gecko_bindings::structs::{nsIAtom, ServoStyleSetSizes, StyleRuleInclusion}; +use hashglobe::FailedAllocationError; use invalidation::element::invalidation_map::InvalidationMap; use invalidation::media_queries::{EffectiveMediaQueryResults, ToMediaListKey}; use media_queries::Device; @@ -87,7 +88,8 @@ impl DocumentCascadeData { } /// Rebuild the cascade data for the given document stylesheets, and - /// optionally with a set of user agent stylesheets. + /// optionally with a set of user agent stylesheets. Returns Err(..) + /// to signify OOM. fn rebuild<'a, 'b, S>( &mut self, device: &Device, @@ -96,7 +98,7 @@ impl DocumentCascadeData { guards: &StylesheetGuards, ua_stylesheets: Option<&UserAgentStylesheets>, extra_data: &mut PerOrigin, - ) + ) -> Result<(), FailedAllocationError> where 'b: 'a, S: StylesheetInDocument + ToMediaListKey + PartialEq + 'static, @@ -154,7 +156,7 @@ impl DocumentCascadeData { guards.ua_or_user, extra_data, SheetRebuildKind::Full, - ); + )?; } if quirks_mode != QuirksMode::NoQuirks { @@ -183,7 +185,7 @@ impl DocumentCascadeData { guards.ua_or_user, extra_data, SheetRebuildKind::Full, - ); + )?; } } } @@ -196,10 +198,13 @@ impl DocumentCascadeData { guards.author, extra_data, rebuild_kind, - ); + )?; } + + Ok(()) } + // Returns Err(..) to signify OOM fn add_stylesheet( &mut self, device: &Device, @@ -208,13 +213,13 @@ impl DocumentCascadeData { guard: &SharedRwLockReadGuard, _extra_data: &mut PerOrigin, rebuild_kind: SheetRebuildKind, - ) + ) -> Result<(), FailedAllocationError> where S: StylesheetInDocument + ToMediaListKey + 'static, { if !stylesheet.enabled() || !stylesheet.is_effective_for_device(device, guard) { - return; + return Ok(()); } let origin = stylesheet.origin(guard); @@ -277,12 +282,12 @@ impl DocumentCascadeData { origin_cascade_data.rules_source_order ); - map.insert(rule, quirks_mode); + map.insert(rule, quirks_mode)?; if rebuild_kind.should_rebuild_invalidation() { origin_cascade_data .invalidation_map - .note_selector(selector, quirks_mode); + .note_selector(selector, quirks_mode)?; let mut visitor = StylistSelectorVisitor { needs_revalidation: false, passed_rightmost_selector: false, @@ -298,7 +303,7 @@ impl DocumentCascadeData { origin_cascade_data.selectors_for_cache_revalidation.insert( RevalidationSelectorAndHashes::new(selector.clone(), hashes), quirks_mode - ); + )?; } } } @@ -336,7 +341,8 @@ impl DocumentCascadeData { let animation = KeyframesAnimation::from_keyframes( &keyframes_rule.keyframes, keyframes_rule.vendor_prefix.clone(), guard); debug!("Found valid keyframe animation: {:?}", animation); - origin_cascade_data.animations.insert(keyframes_rule.name.as_atom().clone(), animation); + origin_cascade_data.animations + .try_insert(keyframes_rule.name.as_atom().clone(), animation)?; } } #[cfg(feature = "gecko")] @@ -367,6 +373,8 @@ impl DocumentCascadeData { _ => {} } } + + Ok(()) } /// Measures heap usage. @@ -604,7 +612,7 @@ impl Stylist { guards, ua_sheets, extra_data, - ); + ).unwrap_or_else(|_| warn!("OOM in Stylist::flush")); had_invalidations } diff --git a/tests/unit/style/stylist.rs b/tests/unit/style/stylist.rs index a2e18b4906e..33faf7b5210 100644 --- a/tests/unit/style/stylist.rs +++ b/tests/unit/style/stylist.rs @@ -169,9 +169,11 @@ fn test_rule_ordering_same_specificity() { fn test_insert() { let (rules_list, _) = get_mock_rules(&[".intro.foo", "#top"]); let mut selector_map = SelectorMap::new(); - selector_map.insert(rules_list[1][0].clone(), QuirksMode::NoQuirks); + selector_map.insert(rules_list[1][0].clone(), QuirksMode::NoQuirks) + .expect("OOM"); assert_eq!(1, selector_map.id_hash.get(&Atom::from("top"), QuirksMode::NoQuirks).unwrap()[0].source_order); - selector_map.insert(rules_list[0][0].clone(), QuirksMode::NoQuirks); + selector_map.insert(rules_list[0][0].clone(), QuirksMode::NoQuirks) + .expect("OOM"); assert_eq!(0, selector_map.class_hash.get(&Atom::from("foo"), QuirksMode::NoQuirks).unwrap()[0].source_order); assert!(selector_map.class_hash.get(&Atom::from("intro"), QuirksMode::NoQuirks).is_none()); }