Skip to content

Commit

Permalink
Fix uninstalling signal handlers
Browse files Browse the repository at this point in the history
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.
  • Loading branch information
blaztomazicO7 committed Jan 18, 2021
1 parent 75ea085 commit 37ed7ca
Showing 1 changed file with 40 additions and 17 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -50,6 +50,14 @@
*/
static volatile sig_atomic_t bsg_g_installed = 0;

/** Flag noting if we should handle signals.
* When other signal handlers are registered after ours we can't remove our
* signal handlers without removing the others.
* It's not fully thread safe, but it's safer than locking and slightly better
* than nothing.
*/
static volatile sig_atomic_t bsg_g_enabled = 0;

#if !TARGET_OS_TV
/** Our custom signal stack. The signal handler will use this as its stack. */
static stack_t bsg_g_signalStack = {0};
Expand Down Expand Up @@ -82,7 +90,7 @@ static BSG_KSCrash_SentryContext *bsg_g_context;
void bsg_kssighndl_i_handleSignal(int sigNum, siginfo_t *signalInfo,
void *userContext) {
BSG_KSLOG_DEBUG("Trapped signal %d", sigNum);
if (bsg_g_installed) {
if (bsg_g_enabled) {
bool wasHandlingCrash = bsg_g_context->handlingCrash;
bsg_kscrashsentry_beginHandlingCrash(bsg_g_context);

Expand Down Expand Up @@ -116,9 +124,17 @@ void bsg_kssighndl_i_handleSignal(int sigNum, siginfo_t *signalInfo,
bsg_kscrashsentry_resumeThreads();
}

BSG_KSLOG_DEBUG("Re-raising signal for regular handlers to catch.");
// This is technically not allowed, but it works in OSX and iOS.
raise(sigNum);
BSG_KSLOG_DEBUG(
"Re-raising or chaining signal for regular handlers to catch.");
struct sigaction previous = bsg_g_previousSignalHandlers[sigNum];
if (previous.sa_flags & SA_SIGINFO) {
previous.sa_sigaction(sigNum, signalInfo, userContext);
} else if (previous.sa_handler == SIG_DFL) {
// This is technically not allowed, but it works in OSX and iOS.
raise(sigNum);
} else if (previous.sa_handler != SIG_IGN) {
previous.sa_handler(sigNum);
}
}

// ============================================================================
Expand All @@ -129,6 +145,10 @@ bool bsg_kscrashsentry_installSignalHandler(
BSG_KSCrash_SentryContext *context) {
BSG_KSLOG_DEBUG("Installing signal handler.");

if (!bsg_g_enabled) {
bsg_g_enabled = 1;
BSG_KSLOG_DEBUG("Signal handlers enabled.");
}
if (bsg_g_installed) {
BSG_KSLOG_DEBUG("Signal handler already installed.");
return true;
Expand Down Expand Up @@ -194,26 +214,29 @@ bool bsg_kscrashsentry_installSignalHandler(

failed:
BSG_KSLOG_DEBUG("Failed to install signal handlers.");
bsg_g_enabled = 0;
bsg_g_installed = 0;
return false;
}

void bsg_kscrashsentry_uninstallSignalHandler(void) {
BSG_KSLOG_DEBUG("Uninstalling signal handlers.");
if (!bsg_g_installed) {
BSG_KSLOG_DEBUG("Signal handlers were already uninstalled.");
// We only disable signal handling but don't uninstall the signal handlers.
//
// The probblem is that we can safely uninstall signal handlers only when we
// are the last one registered. If we are not we can't know how many
// handlers were registered after us to re-register them. Also other
// handlers could save our handler to chain the signal and our handler will
// be called even when "uninstalled".
//
// Therefore keep the signal handlers installed and just disable the
// handling. The installed signal handlers still chains the signal even when
// not handling.
if (!bsg_g_enabled) {
BSG_KSLOG_DEBUG("Signal handlers were already disabled.");
return;
}

const int *fatalSignals = bsg_kssignal_fatalSignals();
int fatalSignalsCount = bsg_kssignal_numFatalSignals();

for (int i = 0; i < fatalSignalsCount; i++) {
BSG_KSLOG_DEBUG("Restoring original handler for signal %d",
fatalSignals[i]);
sigaction(fatalSignals[i], &bsg_g_previousSignalHandlers[i], NULL);
}

BSG_KSLOG_DEBUG("Signal handlers uninstalled.");
bsg_g_installed = 0;
BSG_KSLOG_DEBUG("Signal handlers disabled.");
bsg_g_enabled = 0;
}

0 comments on commit 37ed7ca

Please sign in to comment.