Auto merge of #29792 - mukilan:fix-layout-2020-crashes, r=mrobinson

Fix layout 2020 crashes

<!-- Please describe your changes on the following line: -->

This PR contains (temporary) fixes for two issues discovered in layout-2020.

1. servo crashing due to 'already mutably borrowed' error when loading 'https://mozdevs.github.io/servo-experiments/experiments/bookshelf/'
2. Dev builds crashing when loading servo.org, but not in release mode.

For the first issue, when rendering a page containing an iframe, layout 2020 creates parallel 'Layout' threads which share workers in the stylo thread pool. Because of the way the 'StyleSharingCache' structures are designed using TLS for storage of LRU cache, this leads to a double borrow of the cache when both layout threads run concurrently.

More details about the issue can be found here: https://gist.github.com/mukilan/ed57eb61b83237a05fbf6360ec5e33b0

This PR is a workaround until we find a more elegant/optimal design that also can work for gecko. The fix for now is simply to not allow multiple layouts in parallel.

For the second issue, it seems like the style pool threads overrun their allocated stack space more quickly in debug mode than in release mode. Based on https://bugzilla.mozilla.org/show_bug.cgi?id=1376883 it seems the stack size was chosen after several experiments. It is possible the results of those experiments are no longer valid.

The temporary workaround is to increase the stack size of the worker threads to avoid layout 2020 builds from crashing when servo.org is loaded, until we can confirm the theory and implement a more robust solution.

---
<!-- Thank you for contributing to Servo! Please replace each `[ ]` by `[X]` when the step is complete, and replace `___` with appropriate data: -->
- [x] `./mach build -d` does not report any errors
- [x] `./mach test-tidy` does not report any errors
- [ ] These changes fix #___ (GitHub issue number if applicable)

<!-- Either: -->
- [ ] There are tests for these changes OR
- [x] These changes do not require tests because they fix crashing behaviour

<!-- Also, please make sure that "Allow edits from maintainers" checkbox is checked, so that we can help you if you get stuck somewhere along the way.-->

<!-- Pull requests that do not address these steps are welcome, but they will require additional verification as part of the review process. -->

4 files changed, 23 insertions, 19 deletions

diff --git a/components/layout_thread/lib.rs b/components/layout_thread/lib.rs
index 1fbc41cab06..05666dac383 100644
--- a/components/layout_thread/lib.rs
+++ b/components/layout_thread/lib.rs
@@ -1332,10 +1332,10 @@ impl LayoutThread {
             data.stylesheets_changed,
         );
 
-        let pool;
+        let pool = STYLE_THREAD_POOL.lock().unwrap();
+        let thread_pool = pool.pool();
         let (thread_pool, num_threads) = if self.parallel_flag {
-            pool = STYLE_THREAD_POOL.pool();
-            (pool.as_ref(), STYLE_THREAD_POOL.num_threads.unwrap_or(1))
+            (thread_pool.as_ref(), pool.num_threads.unwrap_or(1))
         } else {
             (None, 1)
         };
@@ -1419,6 +1419,7 @@ impl LayoutThread {
                 Some(&document),
                 &mut rw_data,
                 &mut layout_context,
+                thread_pool,
             );
         }
 
@@ -1626,6 +1627,7 @@ impl LayoutThread {
         document: Option<&ServoLayoutDocument<LayoutData>>,
         rw_data: &mut LayoutThreadData,
         context: &mut LayoutContext,
+        thread_pool: Option<&rayon::ThreadPool>,
     ) {
         Self::cancel_animations_for_nodes_not_in_flow_tree(
             &mut *(context.style_context.animations.sets.write()),
@@ -1683,14 +1685,6 @@ impl LayoutThread {
                 || {
                     let profiler_metadata = self.profiler_metadata();
 
-                    let pool;
-                    let thread_pool = if self.parallel_flag {
-                        pool = STYLE_THREAD_POOL.pool();
-                        pool.as_ref()
-                    } else {
-                        None
-                    };
-
                     if let Some(pool) = thread_pool {
                         // Parallel mode.
                         LayoutThread::solve_constraints_parallel(
diff --git a/components/layout_thread_2020/lib.rs b/components/layout_thread_2020/lib.rs
index 06675f0fe3b..006d18fc67e 100644
--- a/components/layout_thread_2020/lib.rs
+++ b/components/layout_thread_2020/lib.rs
@@ -517,6 +517,7 @@ impl LayoutThread {
         animation_timeline_value: f64,
         animations: &DocumentAnimationSet,
         stylesheets_changed: bool,
+        use_rayon: bool,
     ) -> LayoutContext<'a> {
         let traversal_flags = match stylesheets_changed {
             true => TraversalFlags::ForCSSRuleChanges,
@@ -541,7 +542,7 @@ impl LayoutThread {
             font_cache_thread: Mutex::new(self.font_cache_thread.clone()),
             webrender_image_cache: self.webrender_image_cache.clone(),
             pending_images: Mutex::new(vec![]),
-            use_rayon: STYLE_THREAD_POOL.pool().is_some(),
+            use_rayon,
         }
     }
 
@@ -989,6 +990,10 @@ impl LayoutThread {
 
         self.stylist.flush(&guards, Some(root_element), Some(&map));
 
+        let rayon_pool = STYLE_THREAD_POOL.lock().unwrap();
+        let rayon_pool = rayon_pool.pool();
+        let rayon_pool = rayon_pool.as_ref();
+
         // Create a layout context for use throughout the following passes.
         let mut layout_context = self.build_layout_context(
             guards.clone(),
@@ -997,6 +1002,7 @@ impl LayoutThread {
             data.animation_timeline_value,
             &data.animations,
             data.stylesheets_changed,
+            rayon_pool.is_some(),
         );
 
         let dirty_root = unsafe {
@@ -1012,9 +1018,6 @@ impl LayoutThread {
             RecalcStyle::pre_traverse(dirty_root, shared)
         };
 
-        let rayon_pool = STYLE_THREAD_POOL.pool();
-        let rayon_pool = rayon_pool.as_ref();
-
         if token.should_traverse() {
             let dirty_root: ServoLayoutNode<DOMLayoutData> =
                 driver::traverse_dom(&traversal, token, rayon_pool).as_node();
diff --git a/components/style/global_style_data.rs b/components/style/global_style_data.rs
index 6a22c5a11e9..80626267cdd 100644
--- a/components/style/global_style_data.rs
+++ b/components/style/global_style_data.rs
@@ -16,6 +16,7 @@ use parking_lot::{RwLock, RwLockReadGuard};
 use rayon;
 use std::env;
 use std::sync::atomic::{AtomicUsize, Ordering};
+use std::sync::Mutex;
 
 /// Global style data
 pub struct GlobalStyleData {
@@ -74,7 +75,7 @@ impl StyleThreadPool {
         }
         {
             // Drop the pool.
-            let _ = STYLE_THREAD_POOL.style_thread_pool.write().take();
+            let _ = STYLE_THREAD_POOL.lock().unwrap().style_thread_pool.write().take();
         }
         // Spin until all our threads are done. This will usually be pretty
         // fast, as on shutdown there should be basically no threads left
@@ -104,7 +105,7 @@ impl StyleThreadPool {
 
 lazy_static! {
     /// Global thread pool
-    pub static ref STYLE_THREAD_POOL: StyleThreadPool = {
+    pub static ref STYLE_THREAD_POOL: Mutex<StyleThreadPool> = {
         let stylo_threads = env::var("STYLO_THREADS")
             .map(|s| s.parse::<usize>().expect("invalid STYLO_THREADS value"));
         let mut num_threads = match stylo_threads {
@@ -157,14 +158,14 @@ lazy_static! {
             workers.ok()
         };
 
-        StyleThreadPool {
+        Mutex::new(StyleThreadPool {
             num_threads: if num_threads > 0 {
                 Some(num_threads)
             } else {
                 None
             },
             style_thread_pool: RwLock::new(pool),
-        }
+        })
     };
 
     /// Global style data
diff --git a/components/style/parallel.rs b/components/style/parallel.rs
index 82f003cff71..e578c6bbe51 100644
--- a/components/style/parallel.rs
+++ b/components/style/parallel.rs
@@ -32,8 +32,14 @@ use rayon;
 use smallvec::SmallVec;
 
 /// The minimum stack size for a thread in the styling pool, in kilobytes.
+#[cfg(feature = "gecko")]
 pub const STYLE_THREAD_STACK_SIZE_KB: usize = 256;
 
+/// The minimum stack size for a thread in the styling pool, in kilobytes.
+/// Servo requires a bigger stack in debug builds.
+#[cfg(feature = "servo")]
+pub const STYLE_THREAD_STACK_SIZE_KB: usize = 512;
+
 /// The stack margin. If we get this deep in the stack, we will skip recursive
 /// optimizations to ensure that there is sufficient room for non-recursive work.
 ///