Fix uninstalling signal handlers #976
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
xljones
added
backlog
|
Thanks for raising this PR, we like the idea here. We'll review this as priorities allow, and we'll keep you updated on this thread. |
When uninstalling signal handlers disable them instead of uninstalling. Do it this way because: 1. Signal handlers installed after ours can take our signal handling function address and chain to it. In this case the function must always work. 2. If multiple signal handlers for same signal are installed after ours we can't remove ours without removing all but the last installed one. Because we don't uninstall our signal handlers we now directly call previously installed signal handlers.
blaztinn
force-pushed
the
signal-handler-uninstall-fix
branch
from
37ed7ca
to
9956057
Compare
bugsnagbot
added
scheduled
nickdowell
approved these changes
Feb 5, 2021
|
Fixed in v6.6.3 |
yousif-bugsnag
added
released
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Goal
Allow other signal handlers besides bugsnag's.
Design
When trying bugsnag I noticed that my custom signal handlers weren't
triggered. After debugging I found out that bugsnag catches a mach
exception and uninstalls both mach and signal handlers
(
bsg_kscrashsentry_uninstall(BSG_KSCrashTypeAsyncSafe)).The problem is that when uninstalling bugsnag's signal handler it resets to the
previously installed handler, this results in removing all the signal handlers
installed after bugsnag's (which was the case for my signal handler).
To solve this, disable signal handlers instead of uninstalling them. Do it this
way because:
function address and call it. In this case the function must always work and
chain calls forward.
can't remove bugsnag's without removing all but the last installed one.
Because we don't uninstall bugsnag's signal handlers we now always directly
call previously installed signal handlers inside the bugsnag's signal handler.
Changeset
Changed that signal handler is always installed once installed. On uninstall
it gets disabled, where it doesn't handle the crash but only forwards the call
to previous signal handlers.
Testing
After the change both bugsnag and my custom handler handled the crash.
Test-Bugsnag-iOS-2021.01.19_08-19-20-+0100.xcresult.zip