Confirmed users
424
edits
Crash reporting overhaul: Difference between revisions
Jump to navigation
Jump to search
Added information about the exception handlers rewrite
(Added the bug information for the minidump writers rewrite) |
(Added information about the exception handlers rewrite) |
||
| Line 8: | Line 8: | ||
== Exception handlers == | == Exception handlers == | ||
Status: not started<br> | |||
Developer(s): gsvelto<br> | |||
Source code:<br> | |||
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}} | |||
* {{bug|1620990}} | |||
* {{bug|1620991}} | |||
=== 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 rigth places in the hierarchy on a case-by-case basis. | |||
== Minidump writers == | == Minidump writers == | ||
| Line 16: | Line 69: | ||
* 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|1620993}} | * {{bug|1620993}} | ||