mirror of
https://github.com/mozilla/gecko-dev.git
synced 2024-10-22 17:55:50 +00:00
06a240a0f3
In bug 1614933 we changed the order in which the crash annotations are sent from a crashed content process to the main process in order to prevent a deadlock that would arise if the child process would block writing into the pipe used to sent the annotations. This unfortunately introduced a race that I had missed. Here's the sequence of event when generating a crash in the child process: 1) The child process enters the exception handler 2) It requests a dump from the main process and wait 3) Once the dump is written, the child process wakes up again and writes out the crash annotations 4) The child process quits One the main process side it looks like this: 1) The crash generation thread receives a request to generate a dump 2) The dump is written out, the crash generation thread notifies the content process that it can resume execution. During this step the finished minidump is recorded in the `OnChildProcessDumpRequested()` callback. 3) The crash generation thread reads the crash annotations sent by the content process and finalizes the crash report 4) The main process grabs the finalized crash report The key issue here is that the main process in step 4 is woken up when the content process dies. Notice how there's no synchronization between the crash generation thread and the main thread in the main process: if the crash generation thread takes too long reading the crash annotations the main thread might see an incomplete or missing crash report; that's because the event that wakes it up - the content process ending execution - can happen in parallel with step 3. This is an issue that was accidentally introduced in Windows by bug 1614933 but was already present in both Linux and macOS. It was just very unlikely to happen. This patch fixes the issue by splitting step 3 in the main process in two different stages: in the main process we grab the generated minidump in `OnChildProcessDumpRequested()`, Breakpad then unblocks the child process and we read the annotations in `OnChildProcessDumpWritten()`. We grab the `dumpMapLock` Mutex in the latter and release it in the former to ensure that the main process will have to wait for the crash report to be finalized when `TakeCrashedChildProcess()` is called. This might appear somewhat confusing and even causes debug builds to spit a harmless warning (we don't want Mutexes to be taken and released from different scopes if we can help it). To implement the above behavior in Breakpad a new callback was introduced in Windows, an existing one was used under macOS and an unused one was used under Linux. This accounts for the different way in which minidumps are generated on the three platforms. Differential Revision: https://phabricator.services.mozilla.com/D74496 |
||
|---|---|---|
| .. | ||
| breakpad-client | ||
| breakpad-patches | ||
| breakpad-windows-libxul | ||
| breakpad-windows-standalone | ||
| client | ||
| content | ||
| docs | ||
| google-breakpad | ||
| injector | ||
| minidump-analyzer | ||
| rust | ||
| test | ||
| tools | ||
| CrashAnnotations.cpp | ||
| CrashAnnotations.h.in | ||
| CrashAnnotations.yaml | ||
| crashreporter.mozbuild | ||
| CrashReports.jsm | ||
| CrashSubmit.jsm | ||
| generate_crash_reporter_sources.py | ||
| InjectCrashReporter.cpp | ||
| InjectCrashReporter.h | ||
| jar.mn | ||
| LoadLibraryRemote.cpp | ||
| LoadLibraryRemote.h | ||
| mac_utils.h | ||
| mac_utils.mm | ||
| moz.build | ||
| nsDummyExceptionHandler.cpp | ||
| nsExceptionHandler.cpp | ||
| nsExceptionHandler.h | ||
| nsExceptionHandlerUtils.cpp | ||
| nsExceptionHandlerUtils.h | ||
| ThreadAnnotation.cpp | ||
| ThreadAnnotation.h | ||
| update-breakpad.sh | ||