1622452 - Unset the remote exception handler when child processes shut down

Status: RESOLVED FIXED

(Toolkit :: Crash Reporting, task, P1)

Description

[Gabriele Svelto [:gsvelto] (PTO)]

While working on bug 1614933 I discovered that while we had functionality to unset breakpad's exception handler in child processes, it was never used. With the modifications in bug 1614933 applied this leads to leaks in tests.

The natural fix here would be to call UnsetRemoteExceptionHandler() in child processes when they shut down and add the relevant annotation-machinery-teardown code to that.

However.

Unsetting the exception handler on Linux requires a call to sigaltstack and the sandbox doesn't allow it. The reason for this is that breakpad installs an alternate stack to handle signals and then switches back to the old one. I'd rather not whitelist sigaltstack just for this purpose but rather skip removing the exception handler in this scenario knowing that this code will be rewritten soon.

Comment 1

[Gabriele Svelto [:gsvelto] (PTO)]

Attachment: Bug 1622452 - Remove the remote exception handler when a content process shuts down r=froydnj

This works on all platforms with the exception of Linux where we remove the
exception handler only if the sandbox is disabled. With the sandbox enabled we
would have to whitelist sigaltstack() which breakpad uses to remove the
alternate signal stack which is not worth the fuss.

Besides this patch refactors the code that sets and unsets the exception
handler, cutting down on the duplication:

• The XRE_UnsetRemoteExceptionHandler() call is removed from XULAppAPI.h since it
  was no longer used
• The duplicate checks for the special strings used to disable the remote exception
  handler have been removed from CrashReporter::UnsetRemoteExceptionHandler()
  leaving them in the calling code
• The SetRemoteExceptionHandler() function was consolidated into only one
  piece of code with only one non-platform-specific shared prototype
• Some additional code was factored out to improve the readability

These changes pave the way both for bug 1614933 and for the oxidation of the
exception handler code.

Comment 2

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/53fdcfc30740
Remove the remote exception handler when a content process shuts down r=froydnj

Comment 3

[Cosmin Sabou [:CosminS]]

https://hg.mozilla.org/mozilla-central/rev/53fdcfc30740