Skip to content

Commit

Permalink
compositor: Handle synchronous messages while shutting down
Browse files Browse the repository at this point in the history 
During the shutdown process, various threads (such as the
font cache thread) may be finishing up their work. If those threads make
synchronous requests to the compositor, answer them -- even if the
results will be unused. This is at least enough processing for them to
finish their work and exit cleanly.

This addresses crashes that are sometimes seen at exit, particuarly when
the font cache thread tries to register a font during shutdown.

In addition, this change also removes an unused compositor message.
  • Loading branch information
@mrobinson
mrobinson committed Mar 19, 2024
1 parent 291fbce commit 0eeb8b8
Show file tree
Hide file tree
Showing 2 changed files with 96 additions and 51 deletions.
142 changes: 96 additions & 46 deletions components/compositing/compositor.rs
View file
Expand Up @@ -524,38 +524,38 @@ impl<Window: WindowMethods + ?Sized> IOCompositor<Window> {
}

fn handle_browser_message(&mut self, msg: CompositorMsg) -> bool {
if matches!(msg, CompositorMsg::NewWebRenderFrameReady(..)) {
self.pending_frames -= 1;
}

match (msg, self.shutdown_state) {
(_, ShutdownState::FinishedShuttingDown) => {
match self.shutdown_state {
ShutdownState::NotShuttingDown => {},
ShutdownState::ShuttingDown => {
return self.handle_browser_message_while_shutting_down(msg)
},
ShutdownState::FinishedShuttingDown => {
error!("compositor shouldn't be handling messages after shutting down");
return false;
},
}

(CompositorMsg::ShutdownComplete, _) => {
match msg {
CompositorMsg::ShutdownComplete => {
warn!("Received `ShutdownComplete` while not shutting down.");
self.finish_shutting_down();
return false;
},

(
CompositorMsg::ChangeRunningAnimationsState(pipeline_id, animation_state),
ShutdownState::NotShuttingDown,
) => {
CompositorMsg::ChangeRunningAnimationsState(pipeline_id, animation_state) => {
self.change_running_animations_state(pipeline_id, animation_state);
},

(CompositorMsg::SetFrameTree(frame_tree), ShutdownState::NotShuttingDown) => {
CompositorMsg::SetFrameTree(frame_tree) => {
self.set_frame_tree(&frame_tree);
self.send_scroll_positions_to_layout_for_pipeline(&frame_tree.pipeline.id);
},

(CompositorMsg::TouchEventProcessed(result), ShutdownState::NotShuttingDown) => {
CompositorMsg::TouchEventProcessed(result) => {
self.touch_handler.on_event_processed(result);
},

(CompositorMsg::CreatePng(page_rect, reply), ShutdownState::NotShuttingDown) => {
CompositorMsg::CreatePng(page_rect, reply) => {
let res = self.composite_specific_target(CompositeTarget::SharedMemory, page_rect);
if let Err(ref e) = res {
info!("Error retrieving PNG: {:?}", e);
Expand All @@ -566,7 +566,7 @@ impl<Window: WindowMethods + ?Sized> IOCompositor<Window> {
}
},

(CompositorMsg::IsReadyToSaveImageReply(is_ready), ShutdownState::NotShuttingDown) => {
CompositorMsg::IsReadyToSaveImageReply(is_ready) => {
assert_eq!(
self.ready_to_save_state,
ReadyState::WaitingForConstellationReply
Expand All @@ -579,24 +579,20 @@ impl<Window: WindowMethods + ?Sized> IOCompositor<Window> {
self.composite_if_necessary(CompositingReason::Headless);
},

(
CompositorMsg::PipelineVisibilityChanged(pipeline_id, visible),
ShutdownState::NotShuttingDown,
) => {
CompositorMsg::PipelineVisibilityChanged(pipeline_id, visible) => {
self.pipeline_details(pipeline_id).visible = visible;
self.process_animations(true);
},

(CompositorMsg::PipelineExited(pipeline_id, sender), _) => {
CompositorMsg::PipelineExited(pipeline_id, sender) => {
debug!("Compositor got pipeline exited: {:?}", pipeline_id);
self.remove_pipeline_root_layer(pipeline_id);
let _ = sender.send(());
},

(
CompositorMsg::NewWebRenderFrameReady(recomposite_needed),
ShutdownState::NotShuttingDown,
) => {
CompositorMsg::NewWebRenderFrameReady(recomposite_needed) => {
self.pending_frames -= 1;

if recomposite_needed {
if let Some(result) = self.hit_test_at_point(self.cursor_pos) {
self.update_cursor(result);
Expand All @@ -608,13 +604,7 @@ impl<Window: WindowMethods + ?Sized> IOCompositor<Window> {
}
},

(CompositorMsg::Dispatch(func), ShutdownState::NotShuttingDown) => {
// The functions sent here right now are really dumb, so they can't panic.
// But if we start running more complex code here, we should really catch panic here.
func();
},

(CompositorMsg::LoadComplete(_), ShutdownState::NotShuttingDown) => {
CompositorMsg::LoadComplete(_) => {
// If we're painting in headless mode, schedule a recomposite.
if matches!(self.composite_target, CompositeTarget::PngFile(_)) ||
self.exit_after_load
Expand All @@ -623,10 +613,7 @@ impl<Window: WindowMethods + ?Sized> IOCompositor<Window> {
}
},

(
CompositorMsg::WebDriverMouseButtonEvent(mouse_event_type, mouse_button, x, y),
ShutdownState::NotShuttingDown,
) => {
CompositorMsg::WebDriverMouseButtonEvent(mouse_event_type, mouse_button, x, y) => {
let dppx = self.device_pixels_per_page_pixel();
let point = dppx.transform_point(Point2D::new(x, y));
self.on_mouse_window_event_class(match mouse_event_type {
Expand All @@ -636,29 +623,29 @@ impl<Window: WindowMethods + ?Sized> IOCompositor<Window> {
});
},

(CompositorMsg::WebDriverMouseMoveEvent(x, y), ShutdownState::NotShuttingDown) => {
CompositorMsg::WebDriverMouseMoveEvent(x, y) => {
let dppx = self.device_pixels_per_page_pixel();
let point = dppx.transform_point(Point2D::new(x, y));
self.on_mouse_window_move_event_class(DevicePoint::new(point.x, point.y));
},

(CompositorMsg::PendingPaintMetric(pipeline_id, epoch), _) => {
CompositorMsg::PendingPaintMetric(pipeline_id, epoch) => {
self.pending_paint_metrics.insert(pipeline_id, epoch);
},

(CompositorMsg::GetClientWindow(req), ShutdownState::NotShuttingDown) => {
CompositorMsg::GetClientWindow(req) => {
if let Err(e) = req.send(self.embedder_coordinates.window) {
warn!("Sending response to get client window failed ({:?}).", e);
}
},

(CompositorMsg::GetScreenSize(req), ShutdownState::NotShuttingDown) => {
CompositorMsg::GetScreenSize(req) => {
if let Err(e) = req.send(self.embedder_coordinates.screen) {
warn!("Sending response to get screen size failed ({:?}).", e);
}
},

(CompositorMsg::GetScreenAvailSize(req), ShutdownState::NotShuttingDown) => {
CompositorMsg::GetScreenAvailSize(req) => {
if let Err(e) = req.send(self.embedder_coordinates.screen_avail) {
warn!(
"Sending response to get screen avail size failed ({:?}).",
Expand All @@ -667,14 +654,9 @@ impl<Window: WindowMethods + ?Sized> IOCompositor<Window> {
}
},

(CompositorMsg::Forwarded(msg), ShutdownState::NotShuttingDown) => {
CompositorMsg::Forwarded(msg) => {
self.handle_webrender_message(msg);
},

// When we are shutting_down, we need to avoid performing operations
// such as Paint that may crash because we have begun tearing down
// the rest of our resources.
(_, ShutdownState::ShuttingDown) => {},
}

true
Expand Down Expand Up @@ -917,6 +899,74 @@ impl<Window: WindowMethods + ?Sized> IOCompositor<Window> {
}
}

/// Handle messages sent to the compositor during the shutdown process. In general,
/// the things the compositor can do in this state are limited. It's very important to
/// answer any synchronous messages though as other threads might be waiting on the
/// results to finish their own shut down process. We try to do as little as possible
/// during this time.
///
/// When that involves generating WebRender ids, our approach here is to simply
/// generate them, but assume they will never be used, since once shutting down the
/// compositor no longer does any WebRender frame generation.
fn handle_browser_message_while_shutting_down(&mut self, msg: CompositorMsg) -> bool {
match msg {
CompositorMsg::ShutdownComplete => {
self.finish_shutting_down();
return false;
},
CompositorMsg::PipelineExited(pipeline_id, sender) => {
debug!("Compositor got pipeline exited: {:?}", pipeline_id);
self.remove_pipeline_root_layer(pipeline_id);
let _ = sender.send(());
},
CompositorMsg::Forwarded(ForwardedToCompositorMsg::Font(
FontToCompositorMsg::AddFontInstance(_, _, sender),
)) => {
let _ = sender.send(self.webrender_api.generate_font_instance_key());
},
CompositorMsg::Forwarded(ForwardedToCompositorMsg::Font(
FontToCompositorMsg::AddFont(_, sender),
)) => {
let _ = sender.send(self.webrender_api.generate_font_key());
},
CompositorMsg::Forwarded(ForwardedToCompositorMsg::Canvas(
CanvasToCompositorMsg::GenerateKey(sender),
)) => {
let _ = sender.send(self.webrender_api.generate_image_key());
},
CompositorMsg::GetClientWindow(sender) => {
if let Err(e) = sender.send(self.embedder_coordinates.window) {
warn!("Sending response to get client window failed ({:?}).", e);
}
},
CompositorMsg::GetScreenSize(sender) => {
if let Err(e) = sender.send(self.embedder_coordinates.screen) {
warn!("Sending response to get screen size failed ({:?}).", e);
}
},
CompositorMsg::GetScreenAvailSize(sender) => {
if let Err(e) = sender.send(self.embedder_coordinates.screen_avail) {
warn!(
"Sending response to get screen avail size failed ({:?}).",
e
);
}
},
CompositorMsg::NewWebRenderFrameReady(_) => {
// Subtract from the number of pending frames, but do not do any compositing.
self.pending_frames -= 1;
},
CompositorMsg::PendingPaintMetric(pipeline_id, epoch) => {
self.pending_paint_metrics.insert(pipeline_id, epoch);
},

_ => {
debug!("Ignoring message ({:?} while shutting down", msg);
},
}
true
}

/// Queue a new frame in the transaction and increase the pending frames count.
fn generate_frame(&mut self, transaction: &mut Transaction, reason: RenderReasons) {
self.pending_frames += 1;
Expand Down
5 changes: 0 additions & 5 deletions components/shared/compositing/lib.rs
View file
Expand Up @@ -92,10 +92,6 @@ pub enum CompositorMsg {
// sends a reply on the IpcSender, the constellation knows it's safe to
// tear down the other threads associated with this pipeline.
PipelineExited(PipelineId, IpcSender<()>),
/// Runs a closure in the compositor thread.
/// It's used to dispatch functions from webrender to the main thread's event loop.
/// Required to allow WGL GLContext sharing in Windows.
Dispatch(Box<dyn Fn() + Send>),
/// Indicates to the compositor that it needs to record the time when the frame with
/// the given ID (epoch) is painted and report it to the layout of the given
/// pipeline ID.
Expand Down Expand Up @@ -164,7 +160,6 @@ impl Debug for CompositorMsg {
CompositorMsg::PipelineVisibilityChanged(..) => write!(f, "PipelineVisibilityChanged"),
CompositorMsg::PipelineExited(..) => write!(f, "PipelineExited"),
CompositorMsg::NewWebRenderFrameReady(..) => write!(f, "NewWebRenderFrameReady"),
CompositorMsg::Dispatch(..) => write!(f, "Dispatch"),
CompositorMsg::PendingPaintMetric(..) => write!(f, "PendingPaintMetric"),
CompositorMsg::LoadComplete(..) => write!(f, "LoadComplete"),
CompositorMsg::WebDriverMouseButtonEvent(..) => write!(f, "WebDriverMouseButtonEvent"),
Expand Down

0 comments on commit 0eeb8b8

Please sign in to comment.