1
edit
(Added information about the minidump writers) |
(Minor revisions and link to the crash ping lifecycle.) |
||
| (15 intermediate revisions by 2 users not shown) | |||
| Line 8: | Line 8: | ||
== Exception handlers == | == Exception handlers == | ||
Status: not started<br> | |||
Developer(s): gsvelto<br> | |||
Source code:<br> | |||
* https://github.com/EmbarkStudios/crash-handling | |||
Original source code:<br> | |||
* https://hg.mozilla.org/mozilla-central/file/6f0a8dddad51/toolkit/crashreporter/breakpad-client | |||
* https://hg.mozilla.org/mozilla-central/file/6f0a8dddad51/toolkit/profile/nsProfileLock.cpp | |||
* https://hg.mozilla.org/mozilla-central/file/6f0a8dddad51/toolkit/xre/nsSigHandlers.cpp | |||
* https://hg.mozilla.org/mozilla-central/file/6f0a8dddad51/js/src/ds/MemoryProtectionExceptionHandler.cpp | |||
* https://hg.mozilla.org/mozilla-central/file/6f0a8dddad51/js/src/wasm/WasmSignalHandlers.cpp | |||
* https://hg.mozilla.org/mozilla-central/file/6f0a8dddad51/mozglue/android/APKOpen.cpp | |||
* https://hg.mozilla.org/mozilla-central/file/6f0a8dddad51/mozglue/linker/ElfLoader.cpp | |||
* https://hg.mozilla.org/mozilla-central/file/6f0a8dddad51/accessible/windows/msaa/Compatibility.cpp | |||
Bugs:<br> | |||
* {{bug|1620989}} - Rewrite the Linux exception-handler in Rust | |||
* {{bug|1620990}} - Rewrite the Windows exception-handler in Rust | |||
* {{bug|1620991}} - Rewrite the macOS exception-handler in Rust | |||
=== Description === | |||
Exception handlers and signal handlers are used to intercept abnormal conditions | |||
conditions within Firefox and respond to them. Depending on the affected code | |||
and the type of exception the handlers will either ignore it, conditionally | |||
process it or commence crash reporting in case of fatal ones. | |||
=== Rationale === | |||
We have several exception and signal handlers scattered through the code. The | |||
main one is provided by Breakpad and is used to catch fatal exceptions. In | |||
addition to it the JIT sets several handlers to process benign ones and we have | |||
a few more ranging from swallowing exceptions in certain processes to detecting | |||
and working around Windows bugs. This proliferation of handlers causes has | |||
several downsides: | |||
* There are no abstractions whatsoever. Every piece of code that needs to install a handler does so by using low-level platform-specific code (sometimes doing bare syscalls!) | |||
* Several handlers must be called in a specific sequence in order to work, with every step deciding if the exception should go further or not. On Windows this happens naturally as exception handling is structured, however the order is implicit and depends on the startup sequence. On macOS where the handling is non-hierarchical not only we have an implicit ordering but it relies on tricks to set mach message handlers at different levels (process VS threads) in order for them to be called in the desired sequence. Finally on Linux/Android there is no in-built ordering as only one signal handler can be active at the same time, this means that every handler has to take care of others that were installed before it and manually forward signals when necessary. | |||
* The combination of the lack of abstraction and implicit ordering means that their use is brittle. Coders are wary of touching them and sometimes scenarios like early crashes yield unpredictable results due to not all the handlers being in place yet. | |||
* Some handlers are covered by tests but not all of them nor is the sequence in which some must be called. | |||
=== Plan === | |||
We should rewrite the exception handlers starting with a crate that would | |||
provide proper abstractions and explicit ordering. The code relying on existing | |||
handlers would then need to be updated to use the crate instead, registering a | |||
callback, the conditions for it to be called and the order in which it needs to | |||
appear with regards to the other callbacks. The overall goal is to remove | |||
platform-specific code from the existing handler and shrink them down to just | |||
their core functionality, moving all the platform code into the crate. With the | |||
ordering explicitly set we'd also remove all sorts of ambiguity. Once all the | |||
handlers are migrated we should hook functions used to install handlers (such | |||
as signal(), sigaction(), SetUnhandledExceptionHandler(), | |||
task_set_exception_ports(), etc...) to prevent library code from injecting | |||
handlers under our nose. The hooks will either disregard the handlers or | |||
insert them in the right places in the hierarchy on a case-by-case basis. | |||
== Minidump writers == | == Minidump writers == | ||
| Line 16: | Line 70: | ||
* https://github.com/rust-minidump/minidump-writer | * https://github.com/rust-minidump/minidump-writer | ||
Original source code:<br> | Original source code:<br> | ||
* https:// | * https://hg.mozilla.org/mozilla-central/file/6f0a8dddad51/toolkit/crashreporter/breakpad-client | ||
Bugs:<br> | Bugs:<br> | ||
* {{bug| | * {{bug|1620993}} - Rewrite the Linux-specific minidump writer code in Rust | ||
* {{bug|1689358}} - Add ARM/AArch64 support to the oxidized minidump rust writer | |||
* {{bug|1620995}} - Rewrite the macOS-specific minidump writer code in Rust | |||
* {{bug|1620994}} - Rewrite the Windows-specific minidump writer code in Rust | |||
=== Description === | === Description === | ||
| Line 55: | Line 112: | ||
== Crash monitor == | == Crash monitor == | ||
Status: | Status: in progress<br> | ||
Developer(s): gsvelto<br> | Developer(s): gsvelto<br> | ||
Source code:<br> | Source code:<br> | ||
Original source code: N/A<br> | Original source code: N/A<br> | ||
Bugs:<br> | Bugs:<br> | ||
* {{bug|1620998}} | * {{bug|1620998}} - Write a crash monitor program to handle annotations and minidump writing | ||
=== Description === | === Description === | ||
| Line 108: | Line 165: | ||
== Crash reporter client == | == Crash reporter client == | ||
Status: | Status: completed<br> | ||
Developer(s):<br> | Developer(s): afranchuk<br> | ||
Source code: | Source code: https://searchfox.org/mozilla-central/source/toolkit/crashreporter/client | ||
Original source code:<br> | Original source code:<br> | ||
* https://hg.mozilla.org/mozilla-central/file/6f0a8dddad51/toolkit/crashreporter/client | * https://hg.mozilla.org/mozilla-central/file/6f0a8dddad51/toolkit/crashreporter/client | ||
Bugs:<br> | Bugs:<br> | ||
* {{bug|1759175}} | * {{bug|1759175}} - Rewrite the crash reporter client in Rust | ||
=== Description === | === Description === | ||
| Line 141: | Line 198: | ||
* Localization is done via an INI file and cannot use Fluent | * Localization is done via an INI file and cannot use Fluent | ||
* Given the hard-coded nature of the UI I don't know how it behave with RTL language, probably very poorly | * Given the hard-coded nature of the UI I don't know how it behave with RTL language, probably very poorly | ||
* It does not support Glean-based telemetry | |||
=== Plan === | === Plan === | ||
| Line 151: | Line 209: | ||
* We should leverage Rust's asynchronous facility to make processing non-blocking and the UI responsive | * We should leverage Rust's asynchronous facility to make processing non-blocking and the UI responsive | ||
* We should add platform-independent tests for the parts of the code that do not require UI interaction | * We should add platform-independent tests for the parts of the code that do not require UI interaction | ||
=== Results === | |||
The new crash reporter client is built around a simple platform-indepented UI | |||
Rust library which wraps the individual platform-specific widgets. This made | |||
the UI code almost completely platform-independent as opposed to the old | |||
implementation which was entirely platform dependent, effectively requiring | |||
three different indepented implementations. Additionally the new client is | |||
extensively covered in tests and has more robust networking. Integration of | |||
more features such as Glean-based crash pings and Necko-based networking is | |||
now possible. | |||
== Glean-based crash pings == | |||
Status: complete<br> | |||
Developer(s): afranchuk<br> | |||
Source code: https://github.com/mozilla/glean/<br> | |||
Original source code:<br> | |||
* https://hg.mozilla.org/mozilla-central/file/tip/toolkit/crashreporter/client/ping.cpp | |||
* https://hg.mozilla.org/mozilla-central/file/tip/toolkit/components/crashes/CrashManager.in.jsm#l711 | |||
* https://github.com/mozilla-mobile/android-components/tree/main/components/lib/crash | |||
'''New source code and details are available at https://firefox-source-docs.mozilla.org/toolkit/components/crashes/crash-manager/crash-ping-lifecycle.html'''. <br> | |||
Bugs:<br> | |||
* {{bug|1784069}} - [meta] Migrate crash pings to Glean | |||
=== Description === | |||
For every crash we detect Firefox Desktop sends a [https://firefox-source-docs.mozilla.org/toolkit/components/telemetry/data/crash-ping.html crash ping] holding information that | |||
would help us detect issues and prioritize which ones should be fixed. This | |||
information is highly structured, resembles the JSON output of Socorro's | |||
stackwalker, and unfortunately has seen relatively little use in the last few | |||
years mostly because it's hard to process. | |||
=== Rationale === | |||
The [https://firefox-source-docs.mozilla.org/toolkit/components/telemetry/data/crash-ping.html crash ping] | |||
uses legacy telemetry. This is problematic for a number of reasons: legacy telemetry is exclusively available in Firefox Desktop ('''not in GeckoView'''), we don't | |||
have good interfaces to process these pings, the tools we use to extract information | |||
can be complicated and mobile products are using a more modern data collection system, | |||
[https://docs.telemetry.mozilla.org/concepts/glean/glean.html Glean]. | |||
Firefox for Android (aka Fenix) does not submit the crash ping that Firefox Desktop submits. | |||
It instead records a [https://dictionary.telemetry.mozilla.org/apps/fenix/metrics/crash_metrics_crash_count crash_count metric] that's submitted in the metrics ping (via AC-'s [https://github.com/mozilla-mobile/android-components/tree/main/components/lib/crash lib-crash]). Fenix does collect crash-related information through Sentry. The old Firefox for Android (Fennec) had | |||
full-featured crash pings but Fenix doesn't have any at all, leading to a rather | |||
large blind spot in our telemetry. | |||
The best way to address all of the above is to migrate the crash ping to Glean: once this is implemented, all Mozilla products using Glean will be able to benefit from this improvement (Firefox Desktop, Firefox Android, Focus, etc.) | |||
=== Plan === | |||
This migration requires several steps with changes happening in different parts | |||
of the codebase: | |||
* Prepare the design of a minimal crash ping that can be implemented using existing client- and server-side machinery. | |||
* Add support for this minimal Glean-based crash ping to Firefox desktop (inside the CrashManager) and Fenix where it needs to be done from scratch. | |||
* Once the new ping's functionality has been validated, broaden the design to include parts that might require adding a new metric type (such as stack traces) and include all of the legacy crash ping information. | |||
* Modify the code previously introduced to fully populate the Glean-based ping and make its payload match the legacy one. This might need extra work on the Fenix side, especially to capture stack traces. | |||
* Last but not least the crash reporter client needs to be instructed to send Glean crash pings in addition to legacy telemetry pings. Currently Glean doesn't support C++ so this work will need to happen after we rewrite the crash reporter client. | |||
* Decommission the legacy telemetry crash ping and remove the relevant code from Firefox desktop. | |||
=== Results === | |||
Work on fully-featured Glean-based crash pings was completed in 2025H1 and | |||
legacy crash pings were removed in 2025H2. This work built upon several | |||
previous projects such as the Rust-based client-side minidump-analyzer which | |||
provided high-quality stacks on Android machines, the new Rust-based crash | |||
reporter client which allowed the use of Glean libraries (the previous C++ | |||
based crash reporter client could not send Glean telemetry) and several | |||
additions to Glean itself to enable carrying structured data such as full stack | |||
traces and module lists. Additionally a review of the data carried by the | |||
legacy crash ping was done to assess which parts would be included in the Glean | |||
pings and which would be dropped. The resulting data carried by the ping is | |||
higher quality than in legacy telemetry but also significantly leaner as Glean | |||
already provides information that we had to assemble manually. The ping also | |||
carries the same data across different platforms, and its consistency is | |||
guaranteed by its contents being generated from Glean metrics file instead of | |||
being assembled manually like the legacy ping. Last but not least the new | |||
telemetry data is used as the input of the new | |||
[https://crash-pings.mozilla.org/ crash telemetry dashboard] which allows | |||
visualizing crash information in a way that makes triage and analysis possible. | |||
== minidump-analyzer == | == minidump-analyzer == | ||
Status: | Status: completed<br> | ||
Developer(s):<br> | Developer(s):<br> | ||
Source code: https://github.com/rust-minidump/rust-minidump/<br> | Source code: https://github.com/rust-minidump/rust-minidump/<br> | ||
| Line 161: | Line 298: | ||
* https://hg.mozilla.org/mozilla-central/file/6f0a8dddad51/toolkit/crashreporter/minidump-analyzer | * https://hg.mozilla.org/mozilla-central/file/6f0a8dddad51/toolkit/crashreporter/minidump-analyzer | ||
Bugs:<br> | Bugs:<br> | ||
* {{bug|1743983}} | * {{bug|1743983}} - Rewrite the minidump-analyzer in Rust | ||
=== Description === | === Description === | ||
| Line 208: | Line 345: | ||
Sentry is already working on integrating symbolic with rust-minidump so we're | Sentry is already working on integrating symbolic with rust-minidump so we're | ||
currently waiting it out. This might require very little work in the end. | currently waiting it out. This might require very little work in the end. | ||
=== Results === | |||
The new client-side minidump analyzer supports all our tier 1 and tier 2 | |||
architectures with full support for native deubg information, is significantly | |||
more robust than the old one, extensively covered with tests and provide stacks | |||
that are consistent with our server-side stack walker. Additionally the crates | |||
used by the stack walker machinery are shared with the profiler reducing the | |||
maintaince burden and yielding consistent results across the two projects. | |||
= Server-side tools and components = | = Server-side tools and components = | ||
| Line 292: | Line 438: | ||
* https://hg.mozilla.org/mozilla-central/file/40bc01de5e10/toolkit/crashreporter/breakpad-patches | * https://hg.mozilla.org/mozilla-central/file/40bc01de5e10/toolkit/crashreporter/breakpad-patches | ||
Bugs:<br> | Bugs:<br> | ||
* {{bug|1588538}} | * {{bug|1588538}} - Use the new Windows dump_syms in Firefox local builds | ||
* {{bug|1588534}} | * {{bug|1588534}} - Use the new Windows dump_syms to dump Microsoft libraries | ||
* {{bug|1588739}} | * {{bug|1588739}} - Rewrite the Linux-specific implementation of dump_syms in Rust | ||
* {{bug|1588740}} | * {{bug|1588740}} - Rewrite the macOS-specific implementation of dump_syms in Rust | ||
=== Description === | === Description === | ||
| Line 368: | Line 514: | ||
* https://hg.mozilla.org/mozilla-central/file/55f06c70f4e5/tools/rb/fix_stack_using_bpsyms.py | * https://hg.mozilla.org/mozilla-central/file/55f06c70f4e5/tools/rb/fix_stack_using_bpsyms.py | ||
Bugs:<br> | Bugs:<br> | ||
* {{bug|1596292}} | * {{bug|1596292}} - Replace stack-fixing scripts with a Rust-based one | ||
=== Description === | === Description === | ||
edit