Auto merge of #12468 - asajeffrey:constellation-remove-panic-channel,…

… r=emilio

Removed panic channel, replaced by integrated logging and issue reporting

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

Remove the previous ad hoc panic channel, replace it by an integrated logging and panicking mechanism, including crash reporting. All thread panics are now reported, not just content threads.

---
<!-- 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
- [X] These changes fix #11838
- [X] These changes do not require tests because we don't test error reporting

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

<!-- Reviewable:start -->
---
This change is [<img src="https://reviewable.io/review_button.svg" height="34" align="absmiddle" alt="Reviewable"/>](https://reviewable.io/reviews/servo/servo/12468)
<!-- Reviewable:end -->

components/constellation/constellation.rs

@@ -29,7 +29,6 @@ use msg::constellation_msg::{FrameId, FrameType, PipelineId};
 use msg::constellation_msg::{Key, KeyModifiers, KeyState, LoadData};
 use msg::constellation_msg::{PipelineNamespace, PipelineNamespaceId, NavigationDirection};
 use msg::constellation_msg::{SubpageId, WindowSizeType};
-use msg::constellation_msg::{self, PanicMsg};
 use net_traits::bluetooth_thread::BluetoothMethodMsg;
 use net_traits::filemanager_thread::FileManagerThreadMsg;
 use net_traits::image_cache_thread::ImageCacheThread;
@@ -91,9 +90,6 @@ pub struct Constellation<Message, LTF, STF> {
     /// A channel through which layout thread messages can be sent to this object.
     layout_sender: IpcSender<FromLayoutMsg>,
 
-    /// A channel through which panic messages can be sent to this object.
-    panic_sender: IpcSender<PanicMsg>,
-
     /// Receives messages from scripts.
     script_receiver: Receiver<FromScriptMsg>,
 
@@ -103,9 +99,6 @@ pub struct Constellation<Message, LTF, STF> {
     /// Receives messages from the layout thread
     layout_receiver: Receiver<FromLayoutMsg>,
 
-    /// Receives panic messages.
-    panic_receiver: Receiver<PanicMsg>,
-
     /// A channel (the implementation of which is port-specific) through which messages can be sent
     /// to the compositor.
     compositor_proxy: Box<CompositorProxy>,
@@ -189,9 +182,6 @@ pub struct Constellation<Message, LTF, STF> {
     /// Are we shutting down?
     shutting_down: bool,
 
-    /// Have we seen any panics? Hopefully always false!
-    handled_panic: bool,
-
     /// Have we seen any warnings? Hopefully always empty!
     /// The buffer contains `(thread_name, reason)` entries.
     handled_warnings: VecDeque<(Option<String>, String)>,
@@ -346,9 +336,10 @@ impl Log for FromScriptLogger {
 
     fn log(&self, record: &LogRecord) {
         if let Some(entry) = log_entry(record) {
-            // TODO: Store the pipeline id in TLS so we can recover it here
+            debug!("Sending log entry {:?}.", entry);
+            let pipeline_id = PipelineId::installed();
             let thread_name = thread::current().name().map(ToOwned::to_owned);
-            let msg = FromScriptMsg::LogEntry(None, thread_name, entry);
+            let msg = FromScriptMsg::LogEntry(pipeline_id, thread_name, entry);
             if let Ok(chan) = self.constellation_chan.lock() {
                 let _ = chan.send(msg);
             }
@@ -384,9 +375,10 @@ impl Log for FromCompositorLogger {
 
     fn log(&self, record: &LogRecord) {
         if let Some(entry) = log_entry(record) {
-            // TODO: Store the pipeline id in TLS so we can recover it here
+            debug!("Sending log entry {:?}.", entry);
+            let pipeline_id = PipelineId::installed();
             let thread_name = thread::current().name().map(ToOwned::to_owned);
-            let msg = FromCompositorMsg::LogEntry(None, thread_name, entry);
+            let msg = FromCompositorMsg::LogEntry(pipeline_id, thread_name, entry);
             if let Ok(chan) = self.constellation_chan.lock() {
                 let _ = chan.send(msg);
             }
@@ -430,19 +422,14 @@ impl<Message, LTF, STF> Constellation<Message, LTF, STF>
             let (ipc_layout_sender, ipc_layout_receiver) = ipc::channel().expect("ipc channel failure");
             let layout_receiver = ROUTER.route_ipc_receiver_to_new_mpsc_receiver(ipc_layout_receiver);
 
-            let (ipc_panic_sender, ipc_panic_receiver) = ipc::channel().expect("ipc channel failure");
-            let panic_receiver = ROUTER.route_ipc_receiver_to_new_mpsc_receiver(ipc_panic_receiver);
-
             let swmanager_receiver = ROUTER.route_ipc_receiver_to_new_mpsc_receiver(swmanager_receiver);
 
             let mut constellation: Constellation<Message, LTF, STF> = Constellation {
                 script_sender: ipc_script_sender,
                 layout_sender: ipc_layout_sender,
                 script_receiver: script_receiver,
-                panic_sender: ipc_panic_sender,
                 compositor_receiver: compositor_receiver,
                 layout_receiver: layout_receiver,
-                panic_receiver: panic_receiver,
                 compositor_proxy: state.compositor_proxy,
                 devtools_chan: state.devtools_chan,
                 bluetooth_thread: state.bluetooth_thread,
@@ -478,7 +465,6 @@ impl<Message, LTF, STF> Constellation<Message, LTF, STF>
                 document_states: HashMap::new(),
                 webrender_api_sender: state.webrender_api_sender,
                 shutting_down: false,
-                handled_panic: false,
                 handled_warnings: VecDeque::new(),
                 random_pipeline_closure: opts::get().random_pipeline_closure_probability.map(|prob| {
                     let seed = opts::get().random_pipeline_closure_seed.unwrap_or_else(random);
@@ -539,7 +525,6 @@ impl<Message, LTF, STF> Constellation<Message, LTF, STF>
             parent_info: parent_info,
             constellation_chan: self.script_sender.clone(),
             layout_to_constellation_chan: self.layout_sender.clone(),
-            panic_chan: self.panic_sender.clone(),
             scheduler_chan: self.scheduler_chan.clone(),
             compositor_proxy: self.compositor_proxy.clone_compositor_proxy(),
             devtools_chan: self.devtools_chan.clone(),
@@ -617,8 +602,7 @@ impl<Message, LTF, STF> Constellation<Message, LTF, STF>
             Script(FromScriptMsg),
             Compositor(FromCompositorMsg),
             Layout(FromLayoutMsg),
-            Panic(PanicMsg),
-            FromSWManager(SWManagerMsg)
+            FromSWManager(SWManagerMsg),
         }
 
         // Get one incoming request.
@@ -636,7 +620,6 @@ impl<Message, LTF, STF> Constellation<Message, LTF, STF>
             let receiver_from_script = &self.script_receiver;
             let receiver_from_compositor = &self.compositor_receiver;
             let receiver_from_layout = &self.layout_receiver;
-            let receiver_from_panic = &self.panic_receiver;
             let receiver_from_swmanager = &self.swmanager_receiver;
             select! {
                 msg = receiver_from_script.recv() =>
@@ -645,8 +628,6 @@ impl<Message, LTF, STF> Constellation<Message, LTF, STF>
                     Request::Compositor(msg.expect("Unexpected compositor channel panic in constellation")),
                 msg = receiver_from_layout.recv() =>
                     Request::Layout(msg.expect("Unexpected layout channel panic in constellation")),
-                msg = receiver_from_panic.recv() =>
-                    Request::Panic(msg.expect("Unexpected panic channel panic in constellation")),
                 msg = receiver_from_swmanager.recv() =>
                     Request::FromSWManager(msg.expect("Unexpected panic channel panic in constellation"))
             }
@@ -662,9 +643,6 @@ impl<Message, LTF, STF> Constellation<Message, LTF, STF>
             Request::Layout(message) => {
                 self.handle_request_from_layout(message);
             },
-            Request::Panic(message) => {
-                self.handle_request_from_panic(message);
-            },
             Request::FromSWManager(message) => {
                 self.handle_request_from_swmanager(message);
             }
@@ -957,15 +935,6 @@ impl<Message, LTF, STF> Constellation<Message, LTF, STF>
         }
     }
 
-    fn handle_request_from_panic(&mut self, message: PanicMsg) {
-        match message {
-            (pipeline_id, panic_reason, backtrace) => {
-                debug!("handling panic message ({:?})", pipeline_id);
-                self.handle_panic(pipeline_id, panic_reason, backtrace);
-            }
-        }
-    }
-
     fn handle_register_serviceworker(&self, scope_things: ScopeThings, scope: Url) {
         if let Some(ref mgr) = self.swmanager_chan {
             let _ = mgr.send(ServiceWorkerMsg::RegisterServiceWorker(scope_things, scope));
@@ -1053,22 +1022,14 @@ impl<Message, LTF, STF> Constellation<Message, LTF, STF>
     fn handle_send_error(&mut self, pipeline_id: PipelineId, err: IOError) {
         // Treat send error the same as receiving a panic message
         debug!("Pipeline {:?} send error ({}).", pipeline_id, err);
-        self.handle_panic(Some(pipeline_id), format!("Send failed ({})", err), String::from("<none>"));
+        self.handle_panic(Some(pipeline_id), format!("Send failed ({})", err), None);
     }
 
-    fn handle_panic(&mut self, pipeline_id: Option<PipelineId>, reason: String, backtrace: String) {
-        error!("Panic: {}", reason);
-        if !self.handled_panic || opts::get().full_backtraces {
-            // For the first panic, we print the full backtrace
-            error!("Backtrace:\n{}", backtrace);
-        } else {
-            error!("Backtrace skipped (run with -Z full-backtraces to see every backtrace).");
-        }
-
+    fn handle_panic(&mut self, pipeline_id: Option<PipelineId>, reason: String, backtrace: Option<String>) {
         if opts::get().hard_fail {
             // It's quite difficult to make Servo exit cleanly if some threads have failed.
             // Hard fail exists for test runners so we crash and that's good enough.
-            error!("Pipeline failed in hard-fail mode.  Crashing!");
+            println!("Pipeline failed in hard-fail mode.  Crashing!\n{}\n{}", reason, backtrace.unwrap_or_default());
             process::exit(1);
         }
 
@@ -1109,13 +1070,12 @@ impl<Message, LTF, STF> Constellation<Message, LTF, STF>
             self.push_pending_frame(new_pipeline_id, Some(pipeline_id));
 
         }
-
-        self.handled_panic = true;
     }
 
     fn handle_log_entry(&mut self, pipeline_id: Option<PipelineId>, thread_name: Option<String>, entry: LogEntry) {
+        debug!("Received log entry {:?}.", entry);
         match entry {
-            LogEntry::Panic(reason, backtrace) => self.trigger_mozbrowsererror(pipeline_id, reason, backtrace),
+            LogEntry::Panic(reason, backtrace) => self.handle_panic(pipeline_id, reason, Some(backtrace)),
             LogEntry::Error(reason) | LogEntry::Warn(reason) => {
                 // VecDeque::truncate is unstable
                 if WARNINGS_BUFFER_SIZE <= self.handled_warnings.len() {
@@ -1439,7 +1399,7 @@ impl<Message, LTF, STF> Constellation<Message, LTF, STF>
 
     fn handle_navigate_msg(&mut self,
                            pipeline_info: Option<(PipelineId, SubpageId)>,
-                           direction: constellation_msg::NavigationDirection) {
+                           direction: NavigationDirection) {
         debug!("received message to navigate {:?}", direction);
 
         // Get the frame id associated with the pipeline that sent
@@ -2310,7 +2270,7 @@ impl<Message, LTF, STF> Constellation<Message, LTF, STF>
 
     // https://developer.mozilla.org/en-US/docs/Web/Events/mozbrowsererror
     // Note that this does not require the pipeline to be an immediate child of the root
-    fn trigger_mozbrowsererror(&mut self, pipeline_id: Option<PipelineId>, reason: String, backtrace: String) {
+    fn trigger_mozbrowsererror(&mut self, pipeline_id: Option<PipelineId>, reason: String, backtrace: Option<String>) {
         if !PREFS.is_mozbrowser_enabled() { return; }
 
         let mut report = String::new();
@@ -2325,10 +2285,12 @@ impl<Message, LTF, STF> Constellation<Message, LTF, STF>
         }
         report.push_str("\nERROR: ");
         report.push_str(&*reason);
-        report.push_str("\n\n");
-        report.push_str(&*backtrace);
+        if let Some(backtrace) = backtrace {
+            report.push_str("\n\n");
+            report.push_str(&*backtrace);
+        }
 
-        let event = MozBrowserEvent::Error(MozBrowserErrorType::Fatal, Some(reason), Some(report));
+        let event = MozBrowserEvent::Error(MozBrowserErrorType::Fatal, reason, report);
 
         if let Some(pipeline_id) = pipeline_id {
             if let Some((ancestor_id, subpage_id)) = self.get_mozbrowser_ancestor_info(pipeline_id) {

components/constellation/pipeline.rs

@@ -17,7 +17,7 @@ use ipc_channel::ipc::{self, IpcReceiver, IpcSender};
 use ipc_channel::router::ROUTER;
 use layers::geometry::DevicePixel;
 use layout_traits::LayoutThreadFactory;
-use msg::constellation_msg::{FrameId, FrameType, LoadData, PanicMsg, PipelineId};
+use msg::constellation_msg::{FrameId, FrameType, LoadData, PipelineId};
 use msg::constellation_msg::{PipelineNamespaceId, SubpageId};
 use net_traits::bluetooth_thread::BluetoothMethodMsg;
 use net_traits::image_cache_thread::ImageCacheThread;
@@ -89,8 +89,6 @@ pub struct InitialPipelineState {
     pub constellation_chan: IpcSender<ScriptMsg>,
     /// A channel for the layout thread to send messages to the constellation.
     pub layout_to_constellation_chan: IpcSender<LayoutMsg>,
-    /// A channel to report panics
-    pub panic_chan: IpcSender<PanicMsg>,
     /// A channel to schedule timer events.
     pub scheduler_chan: IpcSender<TimerEventRequest>,
     /// A channel to the compositor.
@@ -159,7 +157,6 @@ impl Pipeline {
                     frame_type: frame_type,
                     load_data: state.load_data.clone(),
                     paint_chan: layout_to_paint_chan.clone().to_opaque(),
-                    panic_chan: state.panic_chan.clone(),
                     pipeline_port: pipeline_port,
                     layout_to_constellation_chan: state.layout_to_constellation_chan.clone(),
                     content_process_shutdown_chan: layout_content_process_shutdown_chan.clone(),
@@ -182,7 +179,6 @@ impl Pipeline {
                             layout_to_paint_port,
                             chrome_to_paint_port,
                             state.compositor_proxy.clone_compositor_proxy(),
-                            state.panic_chan.clone(),
                             state.font_cache_thread.clone(),
                             state.time_profiler_chan.clone(),
                             state.mem_profiler_chan.clone());
@@ -234,7 +230,6 @@ impl Pipeline {
                 layout_to_constellation_chan: state.layout_to_constellation_chan,
                 script_chan: script_chan.clone(),
                 load_data: state.load_data.clone(),
-                panic_chan: state.panic_chan,
                 script_port: script_port,
                 opts: (*opts::get()).clone(),
                 prefs: PREFS.cloned(),
@@ -426,7 +421,6 @@ pub struct UnprivilegedPipelineContent {
     window_size: Option<WindowSizeData>,
     script_chan: IpcSender<ConstellationControlMsg>,
     load_data: LoadData,
-    panic_chan: IpcSender<PanicMsg>,
     script_port: IpcReceiver<ConstellationControlMsg>,
     layout_to_paint_chan: OptionalIpcSender<LayoutToPaintMsg>,
     opts: Opts,
@@ -452,7 +446,6 @@ impl UnprivilegedPipelineContent {
             control_port: self.script_port,
             constellation_chan: self.constellation_chan,
             scheduler_chan: self.scheduler_chan,
-            panic_chan: self.panic_chan.clone(),
             bluetooth_thread: self.bluetooth_thread,
             resource_threads: self.resource_threads,
             image_cache_thread: self.image_cache_thread.clone(),
@@ -470,7 +463,6 @@ impl UnprivilegedPipelineContent {
                     layout_pair,
                     self.pipeline_port,
                     self.layout_to_constellation_chan,
-                    self.panic_chan,
                     self.script_chan,
                     self.layout_to_paint_chan,
                     self.image_cache_thread,

components/gfx/paint_thread.rs

@@ -17,10 +17,9 @@ use font_cache_thread::FontCacheThread;
 use font_context::FontContext;
 use gfx_traits::{ChromeToPaintMsg, Epoch, LayerId, LayerKind, LayerProperties};
 use gfx_traits::{PaintListener, PaintRequest, StackingContextId};
-use ipc_channel::ipc::IpcSender;
 use layers::layers::{BufferRequest, LayerBuffer, LayerBufferSet};
 use layers::platform::surface::{NativeDisplay, NativeSurface};
-use msg::constellation_msg::{PanicMsg, PipelineId};
+use msg::constellation_msg::PipelineId;
 use paint_context::PaintContext;
 use profile_traits::mem;
 use profile_traits::time;
@@ -377,13 +376,14 @@ impl<C> PaintThread<C> where C: PaintListener + Send + 'static {
                   layout_to_paint_port: Receiver<LayoutToPaintMsg>,
                   chrome_to_paint_port: Receiver<ChromeToPaintMsg>,
                   mut compositor: C,
-                  panic_chan: IpcSender<PanicMsg>,
                   font_cache_thread: FontCacheThread,
                   time_profiler_chan: time::ProfilerChan,
                   mem_profiler_chan: mem::ProfilerChan) {
-        thread::spawn_named_with_send_on_panic(format!("PaintThread {:?}", id),
-                                               thread_state::PAINT,
-                                               move || {
+        thread::spawn_named(format!("PaintThread {:?}", id),
+                            move || {
+            thread_state::initialize(thread_state::PAINT);
+            PipelineId::install(id);
+
             let native_display = compositor.native_display();
             let worker_threads = WorkerThreadProxy::spawn(native_display,
                                                           font_cache_thread,
@@ -412,7 +412,7 @@ impl<C> PaintThread<C> where C: PaintListener + Send + 'static {
             for worker_thread in &mut paint_thread.worker_threads {
                 worker_thread.exit()
             }
-        }, Some(id), panic_chan);
+        });
     }
 
     #[allow(unsafe_code)]

components/layout_thread/lib.rs

@@ -76,7 +76,7 @@ use layout::traversal::RecalcStyleAndConstructFlows;
 use layout::webrender_helpers::{WebRenderDisplayListConverter, WebRenderFrameBuilder};
 use layout::wrapper::{LayoutNodeLayoutData, NonOpaqueStyleAndLayoutData};
 use layout_traits::LayoutThreadFactory;
-use msg::constellation_msg::{PanicMsg, PipelineId};
+use msg::constellation_msg::PipelineId;
 use net_traits::image_cache_thread::UsePlaceholder;
 use net_traits::image_cache_thread::{ImageCacheChan, ImageCacheResult, ImageCacheThread};
 use profile_traits::mem::{self, Report, ReportKind, ReportsChan};
@@ -244,7 +244,6 @@ impl LayoutThreadFactory for LayoutThread {
               chan: (Sender<Msg>, Receiver<Msg>),
               pipeline_port: IpcReceiver<LayoutControlMsg>,
               constellation_chan: IpcSender<ConstellationMsg>,
-              panic_chan: IpcSender<PanicMsg>,
               script_chan: IpcSender<ConstellationControlMsg>,
               paint_chan: OptionalIpcSender<LayoutToPaintMsg>,
               image_cache_thread: ImageCacheThread,
@@ -253,9 +252,10 @@ impl LayoutThreadFactory for LayoutThread {
               mem_profiler_chan: mem::ProfilerChan,
               content_process_shutdown_chan: IpcSender<()>,
               webrender_api_sender: Option<webrender_traits::RenderApiSender>) {
-        thread::spawn_named_with_send_on_panic(format!("LayoutThread {:?}", id),
-                                               thread_state::LAYOUT,
-                                               move || {
+        thread::spawn_named(format!("LayoutThread {:?}", id),
+                      move || {
+            thread_state::initialize(thread_state::LAYOUT);
+            PipelineId::install(id);
             { // Ensures layout thread is destroyed before we send shutdown message
                 let sender = chan.0;
                 let layout = LayoutThread::new(id,
@@ -278,7 +278,7 @@ impl LayoutThreadFactory for LayoutThread {
                 }, reporter_name, sender, Msg::CollectReports);
             }
             let _ = content_process_shutdown_chan.send(());
-        }, Some(id), panic_chan);
+        });
     }
 }
 
@@ -747,7 +747,6 @@ impl LayoutThread {
                              info.layout_pair,
                              info.pipeline_port,
                              info.constellation_chan,
-                             info.panic_chan,
                              info.script_chan.clone(),
                              info.paint_chan.to::<LayoutToPaintMsg>(),
                              self.image_cache_thread.clone(),