mirror of
https://github.com/servo/servo.git
synced 2025-08-03 12:40:06 +01:00
Fix intermittent crashes in paint worklets (#30671)
Garbage collection means that the worklets might drop after the script head has been cleaned up. The worklet now caches the thread pool in the DOM object itself which should prevent it from needing to access script thread TLS when being cleaned up. The value is stored as a OnceCell to maintain the same lazy thread pool creation pattern as before. Fixes #25838. Fixes #25258.
This commit is contained in:
Martin Robinson
GitHub
parent
c2af95d2fc
commit
f8ec3df495
76 changed files with 59 additions and 137 deletions
|
|
@ -29,6 +29,7 @@
|
|||
//! The `unsafe_no_jsmanaged_fields!()` macro adds an empty implementation of
|
||||
//! `JSTraceable` to a datatype.
|
||||
|
||||
use std::cell::OnceCell;
|
||||
use std::collections::hash_map::RandomState;
|
||||
use std::collections::HashMap;
|
||||
use std::fmt::Display;
|
||||
|
|
@ -89,6 +90,12 @@ unsafe impl<T: CustomTraceable> CustomTraceable for DomRefCell<T> {
|
|||
}
|
||||
}
|
||||
|
||||
unsafe impl<T: JSTraceable> CustomTraceable for OnceCell<T> {
|
||||
unsafe fn trace(&self, tracer: *mut JSTracer) {
|
||||
self.get().map(|value| value.trace(tracer));
|
||||
}
|
||||
}
|
||||
|
||||
/// Wrapper type for nop traceble
|
||||
///
|
||||
/// SAFETY: Inner type must not impl JSTraceable
|
||||
|
|
|
|||
|
|
@ -10,6 +10,7 @@
|
|||
//! thread pool implementation, which only performs GC or code loading on
|
||||
//! a backup thread, not on the primary worklet thread.
|
||||
|
||||
use std::cell::OnceCell;
|
||||
use std::cmp::max;
|
||||
use std::collections::{hash_map, HashMap};
|
||||
use std::rc::Rc;
|
||||
|
|
@ -38,7 +39,7 @@ use crate::dom::bindings::refcounted::TrustedPromise;
|
|||
use crate::dom::bindings::reflector::{reflect_dom_object, Reflector};
|
||||
use crate::dom::bindings::root::{Dom, DomRoot, RootCollection, ThreadLocalStackRoots};
|
||||
use crate::dom::bindings::str::USVString;
|
||||
use crate::dom::bindings::trace::{JSTraceable, RootedTraceableBox};
|
||||
use crate::dom::bindings::trace::{CustomTraceable, JSTraceable, RootedTraceableBox};
|
||||
use crate::dom::globalscope::GlobalScope;
|
||||
use crate::dom::promise::Promise;
|
||||
use crate::dom::testworkletglobalscope::TestWorkletTask;
|
||||
|
|
@ -60,12 +61,18 @@ const MIN_GC_THRESHOLD: u32 = 1_000_000;
|
|||
#[derive(JSTraceable, MallocSizeOf)]
|
||||
struct DroppableField {
|
||||
worklet_id: WorkletId,
|
||||
/// The cached version of the script thread's WorkletThreadPool. We keep this cached
|
||||
/// because we may need to access it after the script thread has terminated.
|
||||
#[ignore_malloc_size_of = "Difficult to measure memory usage of Rc<...> types"]
|
||||
thread_pool: OnceCell<Rc<WorkletThreadPool>>,
|
||||
}
|
||||
|
||||
impl Drop for DroppableField {
|
||||
fn drop(&mut self) {
|
||||
let script_thread = ScriptThread::worklet_thread_pool();
|
||||
script_thread.exit_worklet(self.worklet_id);
|
||||
let worklet_id = self.worklet_id;
|
||||
self.thread_pool.get_mut().map(|thread_pool| {
|
||||
thread_pool.exit_worklet(worklet_id);
|
||||
});
|
||||
}
|
||||
}
|
||||
|
||||
|
|
@ -86,6 +93,7 @@ impl Worklet {
|
|||
global_type: global_type,
|
||||
droppable_field: DroppableField {
|
||||
worklet_id: WorkletId::new(),
|
||||
thread_pool: OnceCell::new(),
|
||||
},
|
||||
}
|
||||
}
|
||||
|
|
@ -134,9 +142,11 @@ impl WorkletMethods for Worklet {
|
|||
// Steps 6-12 in parallel.
|
||||
let pending_tasks_struct = PendingTasksStruct::new();
|
||||
let global = self.window.upcast::<GlobalScope>();
|
||||
let pool = ScriptThread::worklet_thread_pool();
|
||||
|
||||
pool.fetch_and_invoke_a_worklet_script(
|
||||
self.droppable_field
|
||||
.thread_pool
|
||||
.get_or_init(ScriptThread::worklet_thread_pool)
|
||||
.fetch_and_invoke_a_worklet_script(
|
||||
global.pipeline_id(),
|
||||
self.droppable_field.worklet_id,
|
||||
self.global_type,
|
||||
|
|
@ -501,9 +511,14 @@ impl WorkletThread {
|
|||
// this total ordering on thread roles is what guarantees deadlock-freedom.
|
||||
WorkletData::StartSwapRoles(sender) => {
|
||||
let (our_swapper, their_swapper) = swapper();
|
||||
sender
|
||||
.send(WorkletData::FinishSwapRoles(their_swapper))
|
||||
.unwrap();
|
||||
match sender.send(WorkletData::FinishSwapRoles(their_swapper)) {
|
||||
Ok(_) => {},
|
||||
Err(_) => {
|
||||
// This might happen if the script thread shuts down while
|
||||
// waiting for the worklet to finish.
|
||||
return;
|
||||
},
|
||||
};
|
||||
let _ = our_swapper.swap(&mut self.role);
|
||||
},
|
||||
// To finish swapping roles, perform the atomic swap.
|
||||
|
|
|
|||
|
|
@ -1,2 +1,2 @@
|
|||
[background-image-alpha.https.html]
|
||||
expected: CRASH
|
||||
expected: PASS
|
||||
|
|
|
|||
|
|
@ -1,2 +0,0 @@
|
|||
[background-image-multiple.https.html]
|
||||
expected: CRASH
|
||||
|
|
@ -1,3 +1,3 @@
|
|||
[background-image-tiled.https.html]
|
||||
type: reftest
|
||||
expected: CRASH
|
||||
expected: FAIL
|
||||
|
|
|
|||
|
|
@ -1,2 +0,0 @@
|
|||
[background-repeat-x.https.html]
|
||||
expected: CRASH
|
||||
|
|
@ -1,2 +1,2 @@
|
|||
[custom-property-animation-on-main-thread.https.html]
|
||||
expected: CRASH
|
||||
expected: FAIL
|
||||
|
|
|
|||
|
|
@ -1,2 +0,0 @@
|
|||
[geometry-background-image-001.https.html]
|
||||
expected: CRASH
|
||||
|
|
@ -1,2 +0,0 @@
|
|||
[geometry-background-image-002.https.html]
|
||||
expected: CRASH
|
||||
|
|
@ -1,2 +0,0 @@
|
|||
[geometry-background-image-tiled-001.https.html]
|
||||
expected: CRASH
|
||||
|
|
@ -1,2 +0,0 @@
|
|||
[geometry-background-image-tiled-002.https.html]
|
||||
expected: CRASH
|
||||
|
|
@ -1,2 +0,0 @@
|
|||
[geometry-background-image-tiled-003.https.html]
|
||||
expected: CRASH
|
||||
|
|
@ -1,2 +0,0 @@
|
|||
[geometry-border-image-001.https.html]
|
||||
expected: CRASH
|
||||
|
|
@ -1,2 +0,0 @@
|
|||
[geometry-border-image-002.https.html]
|
||||
expected: CRASH
|
||||
|
|
@ -1,2 +0,0 @@
|
|||
[geometry-border-image-003.https.html]
|
||||
expected: CRASH
|
||||
|
|
@ -1,2 +0,0 @@
|
|||
[geometry-border-image-004.https.html]
|
||||
expected: CRASH
|
||||
|
|
@ -1,2 +0,0 @@
|
|||
[geometry-border-image-005.https.html]
|
||||
expected: CRASH
|
||||
|
|
@ -1,3 +1,3 @@
|
|||
[geometry-with-float-size.https.html]
|
||||
type: reftest
|
||||
expected: CRASH
|
||||
expected: FAIL
|
||||
|
|
|
|||
|
|
@ -1,2 +1,2 @@
|
|||
[canvas-transform.https.html]
|
||||
expected: CRASH
|
||||
expected: FAIL
|
||||
|
|
|
|||
|
|
@ -1,3 +1,3 @@
|
|||
[device-pixel-ratio.https.html]
|
||||
type: reftest
|
||||
expected: CRASH
|
||||
expected: FAIL
|
||||
|
|
|
|||
|
|
@ -1,2 +0,0 @@
|
|||
[invalid-image-constructor-error.https.html]
|
||||
expected: CRASH
|
||||
|
|
@ -1,2 +0,0 @@
|
|||
[invalid-image-paint-error.https.html]
|
||||
expected: CRASH
|
||||
|
|
@ -1,2 +0,0 @@
|
|||
[invalid-image-pending-script.https.html]
|
||||
expected: CRASH
|
||||
|
|
@ -1,2 +0,0 @@
|
|||
[non-registered-property-value.https.html]
|
||||
expected: CRASH
|
||||
|
|
@ -1,2 +0,0 @@
|
|||
[overdraw.https.html]
|
||||
expected: CRASH
|
||||
|
|
@ -1,2 +1,2 @@
|
|||
[paint-arguments.https.html]
|
||||
expected: CRASH
|
||||
expected: PASS
|
||||
|
|
|
|||
|
|
@ -1,2 +1,2 @@
|
|||
[paint-function-arguments-var.https.html]
|
||||
expected: CRASH
|
||||
expected: FAIL
|
||||
|
|
|
|||
|
|
@ -1,2 +1,2 @@
|
|||
[paint-function-arguments.https.html]
|
||||
expected: CRASH
|
||||
expected: PASS
|
||||
|
|
|
|||
|
|
@ -1,2 +0,0 @@
|
|||
[paint-function-this-value.https.html]
|
||||
expected: CRASH
|
||||
|
|
@ -1,2 +0,0 @@
|
|||
[paint2d-canvasFilter.tentative.https.html]
|
||||
expected: CRASH
|
||||
|
|
@ -1,2 +0,0 @@
|
|||
[paint2d-composite.https.html]
|
||||
expected: CRASH
|
||||
|
|
@ -1,2 +0,0 @@
|
|||
[paint2d-conicGradient.https.html]
|
||||
expected: CRASH
|
||||
|
|
@ -1,2 +1,2 @@
|
|||
[paint2d-filter.https.html]
|
||||
expected: CRASH
|
||||
expected: TIMEOUT
|
||||
|
|
|
|||
|
|
@ -1,2 +0,0 @@
|
|||
[paint2d-gradient.https.html]
|
||||
expected: CRASH
|
||||
|
|
@ -1,2 +0,0 @@
|
|||
[paint2d-image.https.html]
|
||||
expected: CRASH
|
||||
|
|
@ -1,4 +1,4 @@
|
|||
[paint2d-paths.https.html]
|
||||
type: reftest
|
||||
bug: https://github.com/servo/servo/issues/17597
|
||||
expected: CRASH
|
||||
expected: FAIL
|
||||
|
|
|
|||
|
|
@ -1,2 +0,0 @@
|
|||
[paint2d-rects.https.html]
|
||||
expected: CRASH
|
||||
|
|
@ -1,2 +1,2 @@
|
|||
[paint2d-reset.https.html]
|
||||
expected: CRASH
|
||||
expected: FAIL
|
||||
|
|
|
|||
|
|
@ -1,2 +0,0 @@
|
|||
[paint2d-roundRect.https.html]
|
||||
expected: CRASH
|
||||
|
|
@ -1,2 +0,0 @@
|
|||
[paint2d-shadows.https.html]
|
||||
expected: CRASH
|
||||
|
|
@ -1,2 +0,0 @@
|
|||
[paint2d-transform.https.html]
|
||||
expected: CRASH
|
||||
|
|
@ -1,2 +0,0 @@
|
|||
[parse-input-arguments-001.https.html]
|
||||
expected: CRASH
|
||||
|
|
@ -1,4 +1,3 @@
|
|||
[parse-input-arguments-002.https.html]
|
||||
type: reftest
|
||||
bug: https://github.com/servo/servo/issues/17852
|
||||
expected: CRASH
|
||||
|
|
|
|||
|
|
@ -1,4 +1,3 @@
|
|||
[parse-input-arguments-003.https.html]
|
||||
type: reftest
|
||||
bug: https://github.com/servo/servo/issues/17852
|
||||
expected: CRASH
|
||||
|
|
|
|||
|
|
@ -1,2 +0,0 @@
|
|||
[parse-input-arguments-004.https.html]
|
||||
expected: CRASH
|
||||
|
|
@ -1,4 +1,3 @@
|
|||
[parse-input-arguments-005.https.html]
|
||||
type: reftest
|
||||
bug: https://github.com/servo/servo/issues/17852
|
||||
expected: CRASH
|
||||
|
|
|
|||
|
|
@ -1,4 +1,3 @@
|
|||
[parse-input-arguments-006.https.html]
|
||||
type: reftest
|
||||
bug: https://github.com/servo/servo/issues/17852
|
||||
expected: CRASH
|
||||
|
|
|
|||
|
|
@ -1,2 +0,0 @@
|
|||
[parse-input-arguments-007.https.html]
|
||||
expected: CRASH
|
||||
|
|
@ -1,4 +1,3 @@
|
|||
[parse-input-arguments-008.https.html]
|
||||
type: reftest
|
||||
bug: https://github.com/servo/servo/issues/17852
|
||||
expected: CRASH
|
||||
|
|
|
|||
|
|
@ -1,4 +1,3 @@
|
|||
[parse-input-arguments-009.https.html]
|
||||
type: reftest
|
||||
bug: https://github.com/servo/servo/issues/17852
|
||||
expected: CRASH
|
||||
|
|
|
|||
|
|
@ -1,4 +1,3 @@
|
|||
[parse-input-arguments-010.https.html]
|
||||
type: reftest
|
||||
bug: https://github.com/servo/servo/issues/17852
|
||||
expected: CRASH
|
||||
|
|
|
|||
|
|
@ -1,4 +1,3 @@
|
|||
[parse-input-arguments-011.https.html]
|
||||
type: reftest
|
||||
bug: https://github.com/servo/servo/issues/17852
|
||||
expected: CRASH
|
||||
|
|
|
|||
|
|
@ -1,4 +1,3 @@
|
|||
[parse-input-arguments-012.https.html]
|
||||
type: reftest
|
||||
bug: https://github.com/servo/servo/issues/17852
|
||||
expected: CRASH
|
||||
|
|
|
|||
|
|
@ -1,2 +0,0 @@
|
|||
[parse-input-arguments-013.https.html]
|
||||
expected: CRASH
|
||||
|
|
@ -1,2 +0,0 @@
|
|||
[parse-input-arguments-014.https.html]
|
||||
expected: CRASH
|
||||
|
|
@ -1,2 +0,0 @@
|
|||
[parse-input-arguments-015.https.html]
|
||||
expected: CRASH
|
||||
|
|
@ -1,4 +1,3 @@
|
|||
[parse-input-arguments-016.https.html]
|
||||
type: reftest
|
||||
bug: https://github.com/servo/servo/issues/17852
|
||||
expected: CRASH
|
||||
|
|
|
|||
|
|
@ -1,2 +0,0 @@
|
|||
[parse-input-arguments-017.https.html]
|
||||
expected: CRASH
|
||||
|
|
@ -1,3 +1,3 @@
|
|||
[parse-input-arguments-018.https.html]
|
||||
type: reftest
|
||||
expected: CRASH
|
||||
expected: FAIL
|
||||
|
|
|
|||
|
|
@ -1,2 +0,0 @@
|
|||
[parse-input-arguments-019.https.html]
|
||||
expected: CRASH
|
||||
|
|
@ -1,2 +0,0 @@
|
|||
[parse-input-arguments-020.https.html]
|
||||
expected: CRASH
|
||||
|
|
@ -1,2 +0,0 @@
|
|||
[parse-input-arguments-021.https.html]
|
||||
expected: CRASH
|
||||
|
|
@ -1,2 +0,0 @@
|
|||
[parse-input-arguments-022.https.html]
|
||||
expected: CRASH
|
||||
|
|
@ -1,2 +0,0 @@
|
|||
[roundrect.https.html]
|
||||
expected: CRASH
|
||||
|
|
@ -1,2 +0,0 @@
|
|||
[setTransform-001.https.html]
|
||||
expected: CRASH
|
||||
|
|
@ -1,2 +0,0 @@
|
|||
[setTransform-002.https.html]
|
||||
expected: CRASH
|
||||
|
|
@ -1,2 +0,0 @@
|
|||
[setTransform-003.https.html]
|
||||
expected: CRASH
|
||||
|
|
@ -1,2 +0,0 @@
|
|||
[setTransform-004.https.html]
|
||||
expected: CRASH
|
||||
|
|
@ -1,4 +1,4 @@
|
|||
[style-background-image.https.html]
|
||||
type: reftest
|
||||
bug: https://github.com/servo/servo/issues/17378
|
||||
expected: CRASH
|
||||
expected: FAIL
|
||||
|
|
|
|||
|
|
@ -1,4 +1,4 @@
|
|||
[style-before-pseudo.https.html]
|
||||
type: reftest
|
||||
bug: https://github.com/servo/servo/issues/17854
|
||||
expected: CRASH
|
||||
expected: FAIL
|
||||
|
|
|
|||
|
|
@ -1,4 +1,4 @@
|
|||
[style-first-letter-pseudo.https.html]
|
||||
type: reftest
|
||||
bug: https://github.com/servo/servo/issues/17854
|
||||
expected: CRASH
|
||||
expected: FAIL
|
||||
|
|
|
|||
|
|
@ -1,2 +0,0 @@
|
|||
[valid-image-after-load.https.html]
|
||||
expected: CRASH
|
||||
|
|
@ -1,2 +0,0 @@
|
|||
[valid-image-before-load.https.html]
|
||||
expected: CRASH
|
||||
|
|
@ -1,2 +1,2 @@
|
|||
[test_paint_worklet.html]
|
||||
expected: CRASH
|
||||
expected: PASS
|
||||
|
|
|
|||
|
|
@ -1,3 +1,3 @@
|
|||
[test_paint_worklet_size.html]
|
||||
type: reftest
|
||||
expected: CRASH
|
||||
expected: FAIL
|
||||
|
|
|
|||
|
|
@ -1,4 +1,4 @@
|
|||
[test_paint_worklet_timeout.html]
|
||||
type: testharness
|
||||
prefs: [dom.worklet.timeout_ms:10]
|
||||
expected: CRASH
|
||||
expected: PASS
|
||||
|
|
|
|||
Loading…
Add table
Add a link
Reference in a new issue