Mirrors/servo
1
0
Fork 0
mirror of https://github.com/servo/servo.git synced 2025-06-06 16:45:39 +00:00
Code Issues Projects Releases Packages Wiki Activity

Prevent moving CanGc values between threads/tasks (#33902)

Browse source 
* Make CanGc non-sendable, and add documentation.

Signed-off-by: Josh Matthews <josh@joshmatthews.net>

* Update CanGc usage to fix usages that were moved between threads/tasks.

Signed-off-by: Josh Matthews <josh@joshmatthews.net>

---------

Signed-off-by: Josh Matthews <josh@joshmatthews.net>
This commit is contained in:
Josh Matthews 2024-10-18 13:26:54 -04:00 • committed by GitHub
parent a58da5aa83
commit b85093ad74
No known key found for this signature in database
GPG key ID: B5690EEEBB952194
10 changed files with 69 additions and 89 deletions
Download patch file Download diff file Expand all files Collapse all files

3
components/script/dom/dedicatedworkerglobalscope.rs
View file

@ -333,7 +333,6 @@ impl DedicatedWorkerGlobalScope {
gpu_id_hub: Arc<IdentityHub>,
control_receiver: Receiver<DedicatedWorkerControlMsg>,
context_sender: Sender<ThreadSafeJSContext>,
can_gc: CanGc,
) -> JoinHandle<()> {
let serialized_worker_url = worker_url.to_string();
let top_level_browsing_context_id = TopLevelBrowsingContextId::installed();
@ -479,7 +478,7 @@ impl DedicatedWorkerGlobalScope {
// until the event loop is destroyed,
// which happens after the closing flag is set to true.
while !scope.is_closing() {
run_worker_event_loop(&*global, Some(&worker), can_gc);
run_worker_event_loop(&*global, Some(&worker), CanGc::note());
}
},
reporter_name,

10
components/script/dom/document.rs
View file

@ -694,7 +694,7 @@ impl Document {
self.activity.get() != DocumentActivity::Inactive
}
pub fn set_activity(&self, activity: DocumentActivity, can_gc: CanGc) {
pub fn set_activity(&self, activity: DocumentActivity) {
// This function should only be called on documents with a browsing context
assert!(self.has_browsing_context);
if activity == self.activity.get() {
@ -752,7 +752,7 @@ impl Document {
false, // bubbles
false, // cancelable
true, // persisted
can_gc,
CanGc::note(),
);
let event = event.upcast::<Event>();
event.set_trusted(true);
@ -2379,7 +2379,7 @@ impl Document {
}
// https://html.spec.whatwg.org/multipage/#the-end
pub fn maybe_queue_document_completion(&self, can_gc: CanGc) {
pub fn maybe_queue_document_completion(&self) {
// https://html.spec.whatwg.org/multipage/#delaying-load-events-mode
let is_in_delaying_load_events_mode = match self.window.undiscarded_window_proxy() {
Some(window_proxy) => window_proxy.is_delaying_load_events_mode(),
@ -2429,7 +2429,7 @@ impl Document {
atom!("load"),
EventBubbles::DoesNotBubble,
EventCancelable::NotCancelable,
can_gc,
CanGc::note(),
);
event.set_trusted(true);
@ -2472,7 +2472,7 @@ impl Document {
false, // bubbles
false, // cancelable
false, // persisted
can_gc,
CanGc::note(),
);
let event = event.upcast::<Event>();
event.set_trusted(true);

8
components/script/dom/globalscope.rs
View file

@ -3185,7 +3185,7 @@ impl GlobalScope {
}
}
pub fn handle_gamepad_event(&self, gamepad_event: GamepadEvent, can_gc: CanGc) {
pub fn handle_gamepad_event(&self, gamepad_event: GamepadEvent) {
match gamepad_event {
GamepadEvent::Connected(index, name, bounds, supported_haptic_effects) => {
self.handle_gamepad_connect(
@ -3194,7 +3194,6 @@ impl GlobalScope {
bounds.axis_bounds,
bounds.button_bounds,
supported_haptic_effects,
can_gc,
);
},
GamepadEvent::Disconnected(index) => {
@ -3207,7 +3206,7 @@ impl GlobalScope {
}
/// <https://www.w3.org/TR/gamepad/#dfn-gamepadconnected>
pub fn handle_gamepad_connect(
fn handle_gamepad_connect(
&self,
// As the spec actually defines how to set the gamepad index, the GilRs index
// is currently unused, though in practice it will almost always be the same.
@ -3217,7 +3216,6 @@ impl GlobalScope {
axis_bounds: (f64, f64),
button_bounds: (f64, f64),
supported_haptic_effects: GamepadSupportedHapticEffects,
can_gc: CanGc,
) {
// TODO: 2. If document is not null and is not allowed to use the "gamepad" permission,
// then abort these steps.
@ -3239,7 +3237,7 @@ impl GlobalScope {
button_bounds,
supported_haptic_effects,
false,
can_gc,
CanGc::note(),
);
navigator.set_gamepad(selected_index as usize, &gamepad);
}

17
components/script/dom/htmlimageelement.rs
View file

@ -516,12 +516,7 @@ impl HTMLImageElement {
});
self.pending_request.borrow_mut().final_url = Some(url);
self.pending_request.borrow_mut().image = Some(image);
self.finish_reacting_to_environment_change(
src,
generation,
selected_pixel_density,
can_gc,
);
self.finish_reacting_to_environment_change(src, generation, selected_pixel_density);
},
ImageResponse::MetadataLoaded(meta) => {
self.pending_request.borrow_mut().metadata = Some(meta);
@ -1067,7 +1062,6 @@ impl HTMLImageElement {
elem: &HTMLImageElement,
selected_source: String,
selected_pixel_density: f64,
can_gc: CanGc,
) -> IpcSender<PendingImageResponse> {
let trusted_node = Trusted::new(elem);
let (responder_sender, responder_receiver) = ipc::channel().unwrap();
@ -1094,7 +1088,7 @@ impl HTMLImageElement {
if generation == element.generation.get() {
element.process_image_response_for_environment_change(image.response,
USVString::from(selected_source_clone), generation,
selected_pixel_density, can_gc);
selected_pixel_density, CanGc::note());
}
}),
&canceller,
@ -1160,7 +1154,6 @@ impl HTMLImageElement {
self,
selected_source.0.clone(),
selected_pixel_density,
can_gc,
);
let cache_result = image_cache.track_image(
img_url.clone(),
@ -1177,7 +1170,6 @@ impl HTMLImageElement {
selected_source,
generation,
selected_pixel_density,
can_gc,
)
},
ImageCacheResult::Available(ImageOrMetadataAvailable::MetadataAvailable(m)) => {
@ -1251,7 +1243,6 @@ impl HTMLImageElement {
src: USVString,
generation: u32,
selected_pixel_density: f64,
can_gc: CanGc,
) {
let this = Trusted::new(self);
let window = window_from_node(self);
@ -1262,7 +1253,7 @@ impl HTMLImageElement {
let relevant_mutation = this.generation.get() != generation;
// Step 15.1
if relevant_mutation {
this.abort_request(State::Unavailable, ImageRequestPhase::Pending, can_gc);
this.abort_request(State::Unavailable, ImageRequestPhase::Pending, CanGc::note());
return;
}
// Step 15.2
@ -1280,7 +1271,7 @@ impl HTMLImageElement {
// Step 15.5
mem::swap(&mut this.current_request.borrow_mut(), &mut pending_request);
this.abort_request(State::Unavailable, ImageRequestPhase::Pending, can_gc);
this.abort_request(State::Unavailable, ImageRequestPhase::Pending, CanGc::note());
}
// Step 15.6

62
components/script/dom/htmlmediaelement.rs
View file

@ -611,7 +611,7 @@ impl HTMLMediaElement {
}
// https://html.spec.whatwg.org/multipage/#ready-states
fn change_ready_state(&self, ready_state: ReadyState, can_gc: CanGc) {
fn change_ready_state(&self, ready_state: ReadyState) {
let old_ready_state = self.ready_state.get();
self.ready_state.set(ready_state);
@ -642,7 +642,7 @@ impl HTMLMediaElement {
task!(media_reached_current_data: move || {
let this = this.root();
this.upcast::<EventTarget>().fire_event(atom!("loadeddata"));
this.delay_load_event(false, can_gc);
this.delay_load_event(false, CanGc::note());
}),
window.upcast(),
);
@ -796,12 +796,12 @@ impl HTMLMediaElement {
// Step 9.obj.3.
// Note that the resource fetch algorithm itself takes care
// of the cleanup in case of failure itself.
self.resource_fetch_algorithm(Resource::Object, can_gc);
self.resource_fetch_algorithm(Resource::Object);
},
Mode::Attribute(src) => {
// Step 9.attr.1.
if src.is_empty() {
self.queue_dedicated_media_source_failure_steps(can_gc);
self.queue_dedicated_media_source_failure_steps();
return;
}
@ -809,7 +809,7 @@ impl HTMLMediaElement {
let url_record = match base_url.join(&src) {
Ok(url) => url,
Err(_) => {
self.queue_dedicated_media_source_failure_steps(can_gc);
self.queue_dedicated_media_source_failure_steps();
return;
},
};
@ -820,7 +820,7 @@ impl HTMLMediaElement {
// Step 9.attr.4.
// Note that the resource fetch algorithm itself takes care
// of the cleanup in case of failure itself.
self.resource_fetch_algorithm(Resource::Url(url_record), can_gc);
self.resource_fetch_algorithm(Resource::Url(url_record));
},
// Step 9.children.
Mode::Children(source) => {
@ -830,7 +830,7 @@ impl HTMLMediaElement {
// Step 9.attr.2.
if src.is_empty() {
source.upcast::<EventTarget>().fire_event(atom!("error"));
self.queue_dedicated_media_source_failure_steps(can_gc);
self.queue_dedicated_media_source_failure_steps();
return;
}
// Step 9.attr.3.
@ -838,20 +838,20 @@ impl HTMLMediaElement {
Ok(url) => url,
Err(_) => {
source.upcast::<EventTarget>().fire_event(atom!("error"));
self.queue_dedicated_media_source_failure_steps(can_gc);
self.queue_dedicated_media_source_failure_steps();
return;
},
};
// Step 9.attr.8.
self.resource_fetch_algorithm(Resource::Url(url_record), can_gc);
self.resource_fetch_algorithm(Resource::Url(url_record));
},
}
}
fn fetch_request(&self, offset: Option<u64>, seek_lock: Option<SeekLock>, can_gc: CanGc) {
fn fetch_request(&self, offset: Option<u64>, seek_lock: Option<SeekLock>) {
if self.resource_url.borrow().is_none() && self.blob_url.borrow().is_none() {
eprintln!("Missing request url");
self.queue_dedicated_media_source_failure_steps(can_gc);
self.queue_dedicated_media_source_failure_steps();
return;
}
@ -899,10 +899,10 @@ impl HTMLMediaElement {
}
// https://html.spec.whatwg.org/multipage/#concept-media-load-resource
fn resource_fetch_algorithm(&self, resource: Resource, can_gc: CanGc) {
if let Err(e) = self.setup_media_player(&resource, can_gc) {
fn resource_fetch_algorithm(&self, resource: Resource) {
if let Err(e) = self.setup_media_player(&resource) {
eprintln!("Setup media player error {:?}", e);
self.queue_dedicated_media_source_failure_steps(can_gc);
self.queue_dedicated_media_source_failure_steps();
return;
}
@ -935,7 +935,7 @@ impl HTMLMediaElement {
.media_element_task_source()
.queue(
task!(set_media_delay_load_event_flag_to_false: move || {
this.root().delay_load_event(false, can_gc);
this.root().delay_load_event(false, CanGc::note());
}),
window.upcast(),
)
@ -954,7 +954,7 @@ impl HTMLMediaElement {
// Step 4.remote.2.
*self.resource_url.borrow_mut() = Some(url);
self.fetch_request(None, None, can_gc);
self.fetch_request(None, None);
},
Resource::Object => {
if let Some(ref src_object) = *self.src_object.borrow() {
@ -963,7 +963,7 @@ impl HTMLMediaElement {
let blob_url = URL::CreateObjectURL(&self.global(), blob);
*self.blob_url.borrow_mut() =
Some(ServoUrl::parse(&blob_url).expect("infallible"));
self.fetch_request(None, None, can_gc);
self.fetch_request(None, None);
},
SrcObject::MediaStream(ref stream) => {
let tracks = &*stream.get_tracks();
@ -978,7 +978,7 @@ impl HTMLMediaElement {
.set_stream(&track.id(), pos == tracks.len() - 1)
.is_err()
{
self.queue_dedicated_media_source_failure_steps(can_gc);
self.queue_dedicated_media_source_failure_steps();
}
}
},
@ -991,7 +991,7 @@ impl HTMLMediaElement {
/// Queues a task to run the [dedicated media source failure steps][steps].
///
/// [steps]: https://html.spec.whatwg.org/multipage/#dedicated-media-source-failure-steps
fn queue_dedicated_media_source_failure_steps(&self, can_gc: CanGc) {
fn queue_dedicated_media_source_failure_steps(&self) {
let window = window_from_node(self);
let this = Trusted::new(self);
let generation_id = self.generation_id.get();
@ -1036,7 +1036,7 @@ impl HTMLMediaElement {
});
// Step 7.
this.delay_load_event(false, can_gc);
this.delay_load_event(false, CanGc::note());
}),
window.upcast(),
);
@ -1123,7 +1123,7 @@ impl HTMLMediaElement {
// Step 6.5.
if self.ready_state.get() != ReadyState::HaveNothing {
self.change_ready_state(ReadyState::HaveNothing, can_gc);
self.change_ready_state(ReadyState::HaveNothing);
}
// Step 6.6.
@ -1329,7 +1329,7 @@ impl HTMLMediaElement {
}
}
fn setup_media_player(&self, resource: &Resource, can_gc: CanGc) -> Result<(), ()> {
fn setup_media_player(&self, resource: &Resource) -> Result<(), ()> {
let stream_type = match *resource {
Resource::Object => {
if let Some(ref src_object) = *self.src_object.borrow() {
@ -1380,7 +1380,7 @@ impl HTMLMediaElement {
let this = trusted_node.clone();
if let Err(err) = task_source.queue_with_canceller(
task!(handle_player_event: move || {
this.root().handle_player_event(&event, can_gc);
this.root().handle_player_event(&event, CanGc::note());
}),
&canceller,
) {
@ -1480,7 +1480,7 @@ impl HTMLMediaElement {
// => "If the media data can be fetched but is found by inspection to be in
// an unsupported format, or can otherwise not be rendered at all"
if self.ready_state.get() < ReadyState::HaveMetadata {
self.queue_dedicated_media_source_failure_steps(can_gc);
self.queue_dedicated_media_source_failure_steps();
} else {
// https://html.spec.whatwg.org/multipage/#reaches-the-end
match self.direction_of_playback() {
@ -1526,7 +1526,7 @@ impl HTMLMediaElement {
);
// https://html.spec.whatwg.org/multipage/#dom-media-have_current_data
self.change_ready_state(ReadyState::HaveCurrentData, can_gc);
self.change_ready_state(ReadyState::HaveCurrentData);
}
},
@ -1730,7 +1730,7 @@ impl HTMLMediaElement {
}
// Step 6.
self.change_ready_state(ReadyState::HaveMetadata, can_gc);
self.change_ready_state(ReadyState::HaveMetadata);
// Step 7.
let mut jumped = false;
@ -1800,7 +1800,7 @@ impl HTMLMediaElement {
}
},
PlayerEvent::EnoughData => {
self.change_ready_state(ReadyState::HaveEnoughData, can_gc);
self.change_ready_state(ReadyState::HaveEnoughData);
// The player has enough data and it is asking us to stop pushing
// bytes, so we cancel the ongoing fetch request iff we are able
@ -1833,7 +1833,7 @@ impl HTMLMediaElement {
));
},
PlayerEvent::SeekData(p, ref seek_lock) => {
self.fetch_request(Some(p), Some(seek_lock.clone()), can_gc);
self.fetch_request(Some(p), Some(seek_lock.clone()));
},
PlayerEvent::SeekDone(_) => {
// Continuation of
@ -1852,7 +1852,7 @@ impl HTMLMediaElement {
PlaybackState::Paused => {
media_session_playback_state = MediaSessionPlaybackState::Paused;
if self.ready_state.get() == ReadyState::HaveMetadata {
self.change_ready_state(ReadyState::HaveEnoughData, can_gc);
self.change_ready_state(ReadyState::HaveEnoughData);
}
},
PlaybackState::Playing => {
@ -2731,7 +2731,7 @@ impl FetchResponseListener for HTMLMediaElementFetchListener {
if let Some(ref mut current_fetch_context) = *elem.current_fetch_context.borrow_mut() {
current_fetch_context.cancel(CancelReason::Error);
}
elem.queue_dedicated_media_source_failure_steps(CanGc::note());
elem.queue_dedicated_media_source_failure_steps();
}
}
@ -2867,7 +2867,7 @@ impl FetchResponseListener for HTMLMediaElementFetchListener {
elem.upcast::<EventTarget>().fire_event(atom!("error"));
} else {
// => "If the media data cannot be fetched at all..."
elem.queue_dedicated_media_source_failure_steps(CanGc::note());
elem.queue_dedicated_media_source_failure_steps();
}
}

5
components/script/dom/serviceworkerglobalscope.rs
View file

@ -294,7 +294,6 @@ impl ServiceWorkerGlobalScope {
control_receiver: Receiver<ServiceWorkerControlMsg>,
context_sender: Sender<ThreadSafeJSContext>,
closing: Arc<AtomicBool>,
can_gc: CanGc,
) -> JoinHandle<()> {
let ScopeThings {
script_url,
@ -384,7 +383,7 @@ impl ServiceWorkerGlobalScope {
scope.execute_script(DOMString::from(source));
}
global.dispatch_activate(can_gc);
global.dispatch_activate(CanGc::note());
let reporter_name = format!("service-worker-reporter-{}", random::<u64>());
scope
.upcast::<GlobalScope>()
@ -398,7 +397,7 @@ impl ServiceWorkerGlobalScope {
// which happens after the closing flag is set to true,
// or until the worker has run beyond its allocated time.
while !scope.is_closing() && !global.has_timed_out() {
run_worker_event_loop(&*global, None, can_gc);
run_worker_event_loop(&*global, None, CanGc::note());
}
},
reporter_name,

1
components/script/dom/worker.rs
View file

@ -232,7 +232,6 @@ impl WorkerMethods for Worker {
global.wgpu_id_hub(),
control_receiver,
context_sender,
can_gc,
);
let context = context_receiver

11
components/script/script_runtime.rs
View file

@ -1115,10 +1115,17 @@ impl Runnable {
}
#[derive(Clone, Copy, Debug)]
pub struct CanGc(());
/// A compile-time marker that there are operations that could trigger a JS garbage collection
/// operation within the current stack frame. It is trivially copyable, so it should be passed
/// as a function argument and reused when calling other functions whenever possible. Since it
/// is only meaningful within the current stack frame, it is impossible to move it to a different
/// thread or into a task that will execute asynchronously.
pub struct CanGc(std::marker::PhantomData<*mut ()>);
impl CanGc {
/// Create a new CanGc value, representing that a GC operation is possible within the
/// current stack frame.
pub fn note() -> CanGc {
CanGc(())
CanGc(std::marker::PhantomData)
}
}

15
components/script/script_thread.rs
View file

@ -1637,7 +1637,7 @@ impl ScriptThread {
CompositorEvent::GamepadEvent(gamepad_event) => {
let global = window.upcast::<GlobalScope>();
global.handle_gamepad_event(gamepad_event, can_gc);
global.handle_gamepad_event(gamepad_event);
},
}
}
@ -2034,7 +2034,7 @@ impl ScriptThread {
let mut docs = self.docs_with_no_blocking_loads.borrow_mut();
for document in docs.iter() {
let _realm = enter_realm(&**document);
document.maybe_queue_document_completion(can_gc);
document.maybe_queue_document_completion();
// Document load is a rendering opportunity.
ScriptThread::note_rendering_opportunity(document.window().pipeline_id());
@ -2331,7 +2331,7 @@ impl ScriptThread {
self.handle_get_title_msg(pipeline_id)
},
ConstellationControlMsg::SetDocumentActivity(pipeline_id, activity) => {
self.handle_set_document_activity_msg(pipeline_id, activity, can_gc)
self.handle_set_document_activity_msg(pipeline_id, activity)
},
ConstellationControlMsg::SetThrottled(pipeline_id, throttled) => {
self.handle_set_throttled_msg(pipeline_id, throttled)
@ -3003,12 +3003,7 @@ impl ScriptThread {
}
/// Handles activity change message
fn handle_set_document_activity_msg(
&self,
id: PipelineId,
activity: DocumentActivity,
can_gc: CanGc,
) {
fn handle_set_document_activity_msg(&self, id: PipelineId, activity: DocumentActivity) {
debug!(
"Setting activity of {} to be {:?} in {:?}.",
id,
@ -3017,7 +3012,7 @@ impl ScriptThread {
);
let document = self.documents.borrow().find_document(id);
if let Some(document) = document {
document.set_activity(activity, can_gc);
document.set_activity(activity);
return;
}
let mut loads = self.incomplete_loads.borrow_mut();

26
components/script/serviceworker_manager.rs
View file

@ -29,7 +29,7 @@ use crate::dom::serviceworkerglobalscope::{
ServiceWorkerControlMsg, ServiceWorkerGlobalScope, ServiceWorkerScriptMsg,
};
use crate::dom::serviceworkerregistration::longest_prefix_match;
use crate::script_runtime::{CanGc, ThreadSafeJSContext};
use crate::script_runtime::ThreadSafeJSContext;
enum Message {
FromResource(CustomResponseMediator),
@ -250,12 +250,10 @@ impl ServiceWorkerManager {
None
}
fn handle_message(&mut self, can_gc: CanGc) {
fn handle_message(&mut self) {
while let Ok(message) = self.receive_message() {
let should_continue = match message {
Message::FromConstellation(msg) => {
self.handle_message_from_constellation(*msg, can_gc)
},
Message::FromConstellation(msg) => self.handle_message_from_constellation(*msg),
Message::FromResource(msg) => self.handle_message_from_resource(msg),
};
if !should_continue {
@ -290,7 +288,7 @@ impl ServiceWorkerManager {
}
}
fn handle_message_from_constellation(&mut self, msg: ServiceWorkerMsg, can_gc: CanGc) -> bool {
fn handle_message_from_constellation(&mut self, msg: ServiceWorkerMsg) -> bool {
match msg {
ServiceWorkerMsg::Timeout(_scope) => {
// TODO: https://w3c.github.io/ServiceWorker/#terminate-service-worker
@ -307,7 +305,7 @@ impl ServiceWorkerManager {
self.handle_register_job(job);
},
JobType::Update => {
self.handle_update_job(job, can_gc);
self.handle_update_job(job);
},
JobType::Unregister => {
// TODO: https://w3c.github.io/ServiceWorker/#unregister-algorithm
@ -382,7 +380,7 @@ impl ServiceWorkerManager {
}
/// <https://w3c.github.io/ServiceWorker/#update>
fn handle_update_job(&mut self, job: Job, can_gc: CanGc) {
fn handle_update_job(&mut self, job: Job) {
// Step 1: Get registation
if let Some(registration) = self.registrations.get_mut(&job.scope_url) {
// Step 3.
@ -405,12 +403,8 @@ impl ServiceWorkerManager {
// Very roughly steps 5 to 18.
// TODO: implement all steps precisely.
let (new_worker, join_handle, control_sender, context, closing) = update_serviceworker(
self.own_sender.clone(),
job.scope_url.clone(),
scope_things,
can_gc,
);
let (new_worker, join_handle, control_sender, context, closing) =
update_serviceworker(self.own_sender.clone(), job.scope_url.clone(), scope_things);
// Since we've just started the worker thread, ensure we can shut it down later.
registration.note_worker_thread(join_handle, control_sender, context, closing);
@ -449,7 +443,6 @@ fn update_serviceworker(
own_sender: IpcSender<ServiceWorkerMsg>,
scope_url: ServoUrl,
scope_things: ScopeThings,
can_gc: CanGc,
) -> (
ServiceWorker,
JoinHandle<()>,
@ -475,7 +468,6 @@ fn update_serviceworker(
control_receiver,
context_sender,
closing.clone(),
can_gc,
);
let context = context_receiver
@ -512,7 +504,7 @@ impl ServiceWorkerManagerFactory for ServiceWorkerManager {
resource_port,
constellation_sender,
)
.handle_message(CanGc::note())
.handle_message()
};
if thread::Builder::new()
.name("SvcWorkerManager".to_owned())