Bug 1557208 - Block the caller during RenderBackend shutdown. r=sotaro
☠☠ backed out by 90e023688593 ☠ ☠
authorNicolas Silva <nsilva@mozilla.com>
Tue, 02 Jul 2019 12:32:56 +0200
changeset 482131 6dff6233acdc014a581d06a9a3963ee7fa658859
parent 482130 db01b84083ba6292edf78c512c061cad8aceacb3
child 482132 e5b264a27fa5bb7589c4264626b1c9082f3a41e5
push id36269
push useraciure@mozilla.com
push dateWed, 10 Jul 2019 15:46:20 +0000
treeherdermozilla-central@241af4dbb964 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewerssotaro
bugs1557208
milestone70.0a1
first release with
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
last release without
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
Bug 1557208 - Block the caller during RenderBackend shutdown. r=sotaro
gfx/webrender_bindings/src/bindings.rs
gfx/wr/webrender/src/render_backend.rs
gfx/wr/webrender_api/src/api.rs
--- a/gfx/webrender_bindings/src/bindings.rs
+++ b/gfx/webrender_bindings/src/bindings.rs
@@ -1315,17 +1315,17 @@ pub extern "C" fn wr_api_clone(
 
 #[no_mangle]
 pub unsafe extern "C" fn wr_api_delete(dh: *mut DocumentHandle) {
     let _ = Box::from_raw(dh);
 }
 
 #[no_mangle]
 pub unsafe extern "C" fn wr_api_shut_down(dh: &mut DocumentHandle) {
-    dh.api.shut_down();
+    dh.api.shut_down(true);
 }
 
 #[no_mangle]
 pub unsafe extern "C" fn wr_api_notify_memory_pressure(dh: &mut DocumentHandle) {
     dh.api.notify_memory_pressure();
 }
 
 #[no_mangle]
--- a/gfx/wr/webrender/src/render_backend.rs
+++ b/gfx/wr/webrender/src/render_backend.rs
@@ -129,16 +129,21 @@ impl ::std::ops::Add<usize> for FrameId 
 impl ::std::ops::Sub<usize> for FrameId {
     type Output = Self;
     fn sub(self, other: usize) -> FrameId {
         assert!(self.0 >= other, "Underflow subtracting FrameIds");
         FrameId(self.0 - other)
     }
 }
 
+enum RenderBackendStatus {
+    Continue,
+    ShutDown(Option<MsgSender<()>>),
+}
+
 /// Identifier to track a sequence of frames.
 ///
 /// This is effectively a `FrameId` with a ridealong timestamp corresponding
 /// to when advance() was called, which allows for more nuanced cache eviction
 /// decisions. As such, we use the `FrameId` for equality and comparison, since
 /// we should never have two `FrameStamps` with the same id but different
 /// timestamps.
 #[derive(Copy, Clone, Debug, MallocSizeOf)]
@@ -859,23 +864,23 @@ impl RenderBackend {
     }
 
     fn next_namespace_id(&self) -> IdNamespace {
         IdNamespace(NEXT_NAMESPACE_ID.fetch_add(1, Ordering::Relaxed) as u32)
     }
 
     pub fn run(&mut self, mut profile_counters: BackendProfileCounters) {
         let mut frame_counter: u32 = 0;
-        let mut keep_going = true;
+        let mut status = RenderBackendStatus::Continue;
 
         if let Some(ref sampler) = self.sampler {
             sampler.register();
         }
 
-        while keep_going {
+        while let RenderBackendStatus::Continue = status {
             profile_scope!("handle_msg");
 
             while let Ok(msg) = self.scene_rx.try_recv() {
                 match msg {
                     SceneBuilderResult::Transactions(mut txns, result_tx) => {
                         self.prepare_for_frames();
                         self.maybe_force_nop_documents(
                             &mut frame_counter,
@@ -947,24 +952,24 @@ impl RenderBackend {
                         self.documents.retain(|doc_id, _doc| doc_id.namespace_id != id);
                     }
                     SceneBuilderResult::Stopped => {
                         panic!("We haven't sent a Stop yet, how did we get a Stopped back?");
                     }
                 }
             }
 
-            keep_going = match self.api_rx.recv() {
+            status = match self.api_rx.recv() {
                 Ok(msg) => {
                     if let Some(ref mut r) = self.recorder {
                         r.write_msg(frame_counter, &msg);
                     }
                     self.process_api_msg(msg, &mut profile_counters, &mut frame_counter)
                 }
-                Err(..) => { false }
+                Err(..) => { RenderBackendStatus::ShutDown(None) }
             };
         }
 
         let _ = self.low_priority_scene_tx.send(SceneBuilderRequest::Stop);
         // Ensure we read everything the scene builder is sending us from
         // inflight messages, otherwise the scene builder might panic.
         while let Ok(msg) = self.scene_rx.recv() {
             match msg {
@@ -981,24 +986,28 @@ impl RenderBackend {
         }
 
         self.notifier.shut_down();
 
         if let Some(ref sampler) = self.sampler {
             sampler.deregister();
         }
 
+
+        if let RenderBackendStatus::ShutDown(Some(sender)) = status {
+            let _ = sender.send(());
+        }
     }
 
     fn process_api_msg(
         &mut self,
         msg: ApiMsg,
         profile_counters: &mut BackendProfileCounters,
         frame_counter: &mut u32,
-    ) -> bool {
+    ) -> RenderBackendStatus {
         match msg {
             ApiMsg::WakeUp => {}
             ApiMsg::WakeSceneBuilder => {
                 self.scene_tx.send(SceneBuilderRequest::WakeUp).unwrap();
             }
             ApiMsg::FlushSceneBuilder(tx) => {
                 self.low_priority_scene_tx.send(SceneBuilderRequest::Flush(tx)).unwrap();
             }
@@ -1093,17 +1102,17 @@ impl RenderBackend {
                         self.frame_config
                             .dual_source_blending_is_enabled = enable;
 
                         self.low_priority_scene_tx.send(SceneBuilderRequest::SetFrameBuilderConfig(
                             self.frame_config.clone()
                         )).unwrap();
 
                         // We don't want to forward this message to the renderer.
-                        return true;
+                        return RenderBackendStatus::Continue;
                     }
                     DebugCommand::FetchDocuments => {
                         let json = self.get_docs_for_debugger();
                         ResultMsg::DebugOutput(DebugOutput::FetchDocuments(json))
                     }
                     DebugCommand::FetchClipScrollTree => {
                         let json = self.get_clip_scroll_tree_for_debugger();
                         ResultMsg::DebugOutput(DebugOutput::FetchClipScrollTree(json))
@@ -1148,31 +1157,31 @@ impl RenderBackend {
                                     pipeline_id,
                                     pipeline.display_list.data(),
                                 ));
                             }
                         }
 
                         // Note: we can't pass `LoadCapture` here since it needs to arrive
                         // before the `PublishDocument` messages sent by `load_capture`.
-                        return true;
+                        return RenderBackendStatus::Continue;
                     }
                     DebugCommand::ClearCaches(mask) => {
                         self.resource_cache.clear(mask);
-                        return true;
+                        return RenderBackendStatus::Continue;
                     }
                     DebugCommand::SimulateLongSceneBuild(time_ms) => {
                         self.scene_tx.send(SceneBuilderRequest::SimulateLongSceneBuild(time_ms)).unwrap();
-                        return true;
+                        return RenderBackendStatus::Continue;
                     }
                     DebugCommand::SimulateLongLowPrioritySceneBuild(time_ms) => {
                         self.low_priority_scene_tx.send(
                             SceneBuilderRequest::SimulateLongLowPrioritySceneBuild(time_ms)
                         ).unwrap();
-                        return true;
+                        return RenderBackendStatus::Continue;
                     }
                     DebugCommand::SetFlags(flags) => {
                         self.resource_cache.set_debug_flags(flags);
                         self.gpu_cache.set_debug_flags(flags);
 
                         // If we're toggling on the GPU cache debug display, we
                         // need to blow away the cache. This is because we only
                         // send allocation/free notifications to the renderer
@@ -1190,31 +1199,31 @@ impl RenderBackend {
 
                         ResultMsg::DebugCommand(option)
                     }
                     _ => ResultMsg::DebugCommand(option),
                 };
                 self.result_tx.send(msg).unwrap();
                 self.notifier.wake_up();
             }
-            ApiMsg::ShutDown => {
+            ApiMsg::ShutDown(sender) => {
                 info!("Recycling stats: {:?}", self.recycler);
-                return false;
+                return RenderBackendStatus::ShutDown(sender);
             }
             ApiMsg::UpdateDocuments(document_ids, transaction_msgs) => {
                 self.prepare_transactions(
                     document_ids,
                     transaction_msgs,
                     frame_counter,
                     profile_counters,
                 );
             }
         }
 
-        true
+        RenderBackendStatus::Continue
     }
 
     fn prepare_for_frames(&mut self) {
         self.resource_cache.prepare_for_frames(SystemTime::now());
         self.gpu_cache.prepare_for_frames();
     }
 
     fn bookkeep_after_frames(&mut self) {
--- a/gfx/wr/webrender_api/src/api.rs
+++ b/gfx/wr/webrender_api/src/api.rs
@@ -752,17 +752,17 @@ pub enum ApiMsg {
     ReportMemory(MsgSender<MemoryReport>),
     /// Change debugging options.
     DebugCommand(DebugCommand),
     /// Wakes the render backend's event loop up. Needed when an event is communicated
     /// through another channel.
     WakeUp,
     WakeSceneBuilder,
     FlushSceneBuilder(MsgSender<()>),
-    ShutDown,
+    ShutDown(Option<MsgSender<()>>),
 }
 
 impl fmt::Debug for ApiMsg {
     fn fmt(&self, f: &mut fmt::Formatter) -> fmt::Result {
         f.write_str(match *self {
             ApiMsg::UpdateResources(..) => "ApiMsg::UpdateResources",
             ApiMsg::GetGlyphDimensions(..) => "ApiMsg::GetGlyphDimensions",
             ApiMsg::GetGlyphIndices(..) => "ApiMsg::GetGlyphIndices",
@@ -771,17 +771,17 @@ impl fmt::Debug for ApiMsg {
             ApiMsg::AddDocument(..) => "ApiMsg::AddDocument",
             ApiMsg::UpdateDocuments(..) => "ApiMsg::UpdateDocuments",
             ApiMsg::DeleteDocument(..) => "ApiMsg::DeleteDocument",
             ApiMsg::ExternalEvent(..) => "ApiMsg::ExternalEvent",
             ApiMsg::ClearNamespace(..) => "ApiMsg::ClearNamespace",
             ApiMsg::MemoryPressure => "ApiMsg::MemoryPressure",
             ApiMsg::ReportMemory(..) => "ApiMsg::ReportMemory",
             ApiMsg::DebugCommand(..) => "ApiMsg::DebugCommand",
-            ApiMsg::ShutDown => "ApiMsg::ShutDown",
+            ApiMsg::ShutDown(..) => "ApiMsg::ShutDown",
             ApiMsg::WakeUp => "ApiMsg::WakeUp",
             ApiMsg::WakeSceneBuilder => "ApiMsg::WakeSceneBuilder",
             ApiMsg::FlushSceneBuilder(..) => "ApiMsg::FlushSceneBuilder",
         })
     }
 }
 
 #[repr(C)]
@@ -1184,18 +1184,24 @@ impl RenderApi {
         rx.recv().unwrap()
     }
 
     pub fn set_debug_flags(&self, flags: DebugFlags) {
         let cmd = DebugCommand::SetFlags(flags);
         self.api_sender.send(ApiMsg::DebugCommand(cmd)).unwrap();
     }
 
-    pub fn shut_down(&self) {
-        self.api_sender.send(ApiMsg::ShutDown).unwrap();
+    pub fn shut_down(&self, synchronously: bool) {
+        if synchronously {
+            let (tx, rx) = channel::msg_channel().unwrap();
+            self.api_sender.send(ApiMsg::ShutDown(Some(tx))).unwrap();
+            rx.recv().unwrap();
+        } else {
+            self.api_sender.send(ApiMsg::ShutDown(None)).unwrap();
+        }
     }
 
     /// Create a new unique key that can be used for
     /// animated property bindings.
     pub fn generate_property_binding_key<T: Copy>(&self) -> PropertyBindingKey<T> {
         let new_id = self.next_unique_id();
         PropertyBindingKey {
             id: PropertyBindingId {