diff --git a/src/config.c b/src/config.c index b26704283..cd2bc1676 100644 --- a/src/config.c +++ b/src/config.c @@ -2528,9 +2528,9 @@ static int updateAofAutoGCEnabled(const char **err) { static int updateSighandlerEnabled(const char **err) { UNUSED(err); if (server.crashlog_enabled) - setupSignalHandlers(); + setupSigSegvHandler(); else - removeSignalHandlers(); + removeSigSegvHandlers(); return 1; } diff --git a/src/debug.c b/src/debug.c index a57b1fde9..809822f96 100644 --- a/src/debug.c +++ b/src/debug.c @@ -66,7 +66,10 @@ typedef ucontext_t sigcontext_t; /* Globals */ static int bug_report_start = 0; /* True if bug report header was already logged. */ static pthread_mutex_t bug_report_start_mutex = PTHREAD_MUTEX_INITIALIZER; - +/* Mutex for a case when two threads crash at the same time. */ +static pthread_mutex_t signal_handler_lock; +static pthread_mutexattr_t signal_handler_lock_attr; +static volatile int signal_handler_lock_initialized = 0; /* Forward declarations */ void bugReportStart(void); void printCrashReport(void); @@ -1063,7 +1066,7 @@ void _serverAssert(const char *estr, const char *file, int line) { } // remove the signal handler so on abort() we will output the crash report. - removeSignalHandlers(); + removeSigSegvHandlers(); bugReportEnd(0, 0); } @@ -1159,7 +1162,7 @@ void _serverPanic(const char *file, int line, const char *msg, ...) { } // remove the signal handler so on abort() we will output the crash report. - removeSignalHandlers(); + removeSigSegvHandlers(); bugReportEnd(0, 0); } @@ -2116,9 +2119,19 @@ void invalidFunctionWasCalled(void) {} typedef void (*invalidFunctionWasCalledType)(void); -void sigsegvHandler(int sig, siginfo_t *info, void *secret) { +static void sigsegvHandler(int sig, siginfo_t *info, void *secret) { UNUSED(secret); UNUSED(info); + /* Check if it is safe to enter the signal handler. second thread crashing at the same time will deadlock. */ + if(pthread_mutex_lock(&signal_handler_lock) == EDEADLK) { + /* If this thread already owns the lock (meaning we crashed during handling a signal) + * log that the crash report can't be generated. */ + serverLog(LL_WARNING, + "Crashed running signal handler. Can't continue to generate the crash report"); + /* gracefully exit */ + bugReportEnd(1, sig); + return; + } bugReportStart(); serverLog(LL_WARNING, @@ -2171,6 +2184,47 @@ void sigsegvHandler(int sig, siginfo_t *info, void *secret) { bugReportEnd(1, sig); } +void setupSigSegvHandler(void) { + /* Initialize the signal handler lock. + Attempting to initialize an already initialized mutex or mutexattr results in undefined behavior. */ + if (!signal_handler_lock_initialized) { + /* Set signal handler with error checking attribute. re-lock within the same thread will error. */ + pthread_mutexattr_init(&signal_handler_lock_attr); + pthread_mutexattr_settype(&signal_handler_lock_attr, PTHREAD_MUTEX_ERRORCHECK); + pthread_mutex_init(&signal_handler_lock, &signal_handler_lock_attr); + signal_handler_lock_initialized = 1; + } + + struct sigaction act; + + sigemptyset(&act.sa_mask); + /* SA_NODEFER to disables adding the signal to the signal mask of the + * calling process on entry to the signal handler unless it is included in the sa_mask field. */ + /* SA_SIGINFO flag is set to raise the function defined in sa_sigaction. + * Otherwise, sa_handler is used. */ + act.sa_flags = SA_NODEFER | SA_SIGINFO; + act.sa_sigaction = sigsegvHandler; + if(server.crashlog_enabled) { + sigaction(SIGSEGV, &act, NULL); + sigaction(SIGBUS, &act, NULL); + sigaction(SIGFPE, &act, NULL); + sigaction(SIGILL, &act, NULL); + sigaction(SIGABRT, &act, NULL); + } +} + +void removeSigSegvHandlers(void) { + struct sigaction act; + sigemptyset(&act.sa_mask); + act.sa_flags = SA_NODEFER | SA_RESETHAND; + act.sa_handler = SIG_DFL; + sigaction(SIGSEGV, &act, NULL); + sigaction(SIGBUS, &act, NULL); + sigaction(SIGFPE, &act, NULL); + sigaction(SIGILL, &act, NULL); + sigaction(SIGABRT, &act, NULL); +} + void printCrashReport(void) { /* Log INFO and CLIENT LIST */ logServerInfo(); @@ -2218,7 +2272,7 @@ void bugReportEnd(int killViaSignal, int sig) { /* Make sure we exit with the right signal at the end. So for instance * the core will be dumped if enabled. */ sigemptyset (&act.sa_mask); - act.sa_flags = SA_NODEFER | SA_ONSTACK | SA_RESETHAND; + act.sa_flags = 0; act.sa_handler = SIG_DFL; sigaction (sig, &act, NULL); kill(getpid(),sig); diff --git a/src/server.c b/src/server.c index 438325f28..72dad9bca 100644 --- a/src/server.c +++ b/src/server.c @@ -6514,37 +6514,13 @@ static void sigShutdownHandler(int sig) { void setupSignalHandlers(void) { struct sigaction act; - /* When the SA_SIGINFO flag is set in sa_flags then sa_sigaction is used. - * Otherwise, sa_handler is used. */ sigemptyset(&act.sa_mask); act.sa_flags = 0; act.sa_handler = sigShutdownHandler; sigaction(SIGTERM, &act, NULL); sigaction(SIGINT, &act, NULL); - sigemptyset(&act.sa_mask); - act.sa_flags = SA_NODEFER | SA_RESETHAND | SA_SIGINFO; - act.sa_sigaction = sigsegvHandler; - if(server.crashlog_enabled) { - sigaction(SIGSEGV, &act, NULL); - sigaction(SIGBUS, &act, NULL); - sigaction(SIGFPE, &act, NULL); - sigaction(SIGILL, &act, NULL); - sigaction(SIGABRT, &act, NULL); - } - return; -} - -void removeSignalHandlers(void) { - struct sigaction act; - sigemptyset(&act.sa_mask); - act.sa_flags = SA_NODEFER | SA_RESETHAND; - act.sa_handler = SIG_DFL; - sigaction(SIGSEGV, &act, NULL); - sigaction(SIGBUS, &act, NULL); - sigaction(SIGFPE, &act, NULL); - sigaction(SIGILL, &act, NULL); - sigaction(SIGABRT, &act, NULL); + setupSigSegvHandler(); } /* This is the signal handler for children process. It is currently useful diff --git a/src/server.h b/src/server.h index cb555031e..4f3005ae9 100644 --- a/src/server.h +++ b/src/server.h @@ -3016,7 +3016,6 @@ int processCommand(client *c); int processPendingCommandAndInputBuffer(client *c); int processCommandAndResetClient(client *c); void setupSignalHandlers(void); -void removeSignalHandlers(void); int createSocketAcceptHandler(connListener *sfd, aeFileProc *accept_handler); connListener *listenerByType(const char *typename); int changeListener(connListener *listener); @@ -3704,7 +3703,8 @@ void _serverPanic(const char *file, int line, const char *msg, ...) void _serverPanic(const char *file, int line, const char *msg, ...); #endif void serverLogObjectDebugInfo(const robj *o); -void sigsegvHandler(int sig, siginfo_t *info, void *secret); +void setupSigSegvHandler(void); +void removeSigSegvHandlers(void); const char *getSafeInfoString(const char *s, size_t len, char **tmp); dict *genInfoSectionDict(robj **argv, int argc, char **defaults, int *out_all, int *out_everything); void releaseInfoSectionDict(dict *sec);