Closed Bug 1622452 Opened 2 years ago Closed 2 years ago

Unset the remote exception handler when child processes shut down

Categories

(Toolkit :: Crash Reporting, task, P1)

Unspecified
All
task

Tracking

()

RESOLVED FIXED
mozilla76
Tracking Status
firefox76 --- fixed

People

(Reporter: gsvelto, Assigned: gsvelto)

References

Details

Attachments

(1 file)

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.

Assignee: nobody → gsvelto
Status: NEW → ASSIGNED
Priority: -- → P1

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.

Pushed by gsvelto@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/53fdcfc30740
Remove the remote exception handler when a content process shuts down r=froydnj
Status: ASSIGNED → RESOLVED
Closed: 2 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla76
You need to log in before you can comment on or make changes to this bug.