Skip to content

Commit

Permalink
Browse files Browse the repository at this point in the history
Fix installation order of signal handlers
The shutdown signal handlers were installed before the workers were
initialized and weren't removed before the workers were deleted. This
would lead to a debug assertion and an eventual crash when a SIGTERM
signal was received outside of the expected scope.

The proper way to do this is to install the handlers only after the system
is up and running and to disable them as soon as the shutdown process
starts.

This mostly happened with the mxs621_unreadable_cnf test as it seemed to
receive a SIGTERM during the execution of the at-exit handlers.
  • Loading branch information
markus456 committed Jul 2, 2020
1 parent 637bc6f commit c9badcb
Showing 1 changed file with 80 additions and 19 deletions.
99 changes: 80 additions & 19 deletions server/core/gateway.cc
Expand Up @@ -1207,6 +1207,46 @@ bool disable_signals(void)
return true;
}

bool disable_normal_signals(void)
{
sigset_t sigset;

if (sigfillset(&sigset) != 0)
{
int e = errno;
errno = 0;
static const char message[] = "Failed to initialize set the signal set for MaxScale. Exiting.";
print_log_n_stderr(true, true, message, message, e);
return false;
}

if (!delete_signal(&sigset, SIGHUP, "SIGHUP"))
{
return false;
}

if (!delete_signal(&sigset, SIGUSR1, "SIGUSR1"))
{
return false;
}

if (!delete_signal(&sigset, SIGTERM, "SIGTERM"))
{
return false;
}

if (sigprocmask(SIG_SETMASK, &sigset, NULL) != 0)
{
int e = errno;
errno = 0;
static const char message[] = "Failed to set the signal set for MaxScale. Exiting";
print_log_n_stderr(true, true, message, message, e);
return false;
}

return true;
}

/**
* Configures the handling of a particular signal.
*
Expand Down Expand Up @@ -1234,58 +1274,69 @@ static bool configure_signal(int signum, const char* signame, void (* handler)(i
}

/**
* Configures signal handling of MaxScale.
* Configure fatal signal handlers
*
* @return True, if all signals could be configured, false otherwise.
* @return True if signal handlers were installed correctly
*/
bool configure_signals(void)
bool configure_critical_signals(void)
{
if (!configure_signal(SIGHUP, "SIGHUP", sighup_handler))

if (!configure_signal(SIGSEGV, "SIGSEGV", sigfatal_handler))
{
return false;
}

if (!configure_signal(SIGUSR1, "SIGUSR1", sigusr1_handler))
if (!configure_signal(SIGABRT, "SIGABRT", sigfatal_handler))
{
return false;
}

if (!configure_signal(SIGTERM, "SIGTERM", sigterm_handler))
if (!configure_signal(SIGILL, "SIGILL", sigfatal_handler))
{
return false;
}

if (!configure_signal(SIGINT, "SIGINT", sigint_handler))
if (!configure_signal(SIGFPE, "SIGFPE", sigfatal_handler))
{
return false;
}

if (!configure_signal(SIGSEGV, "SIGSEGV", sigfatal_handler))
#ifdef SIGBUS
if (!configure_signal(SIGBUS, "SIGBUS", sigfatal_handler))
{
return false;
}
#endif

if (!configure_signal(SIGABRT, "SIGABRT", sigfatal_handler))
return true;
}

/**
* Configures signal handling of MaxScale.
*
* @return True, if all signals could be configured, false otherwise.
*/
bool configure_normal_signals(void)
{
if (!configure_signal(SIGHUP, "SIGHUP", sighup_handler))
{
return false;
}

if (!configure_signal(SIGILL, "SIGILL", sigfatal_handler))
if (!configure_signal(SIGUSR1, "SIGUSR1", sigusr1_handler))
{
return false;
}

if (!configure_signal(SIGFPE, "SIGFPE", sigfatal_handler))
if (!configure_signal(SIGTERM, "SIGTERM", sigterm_handler))
{
return false;
}

#ifdef SIGBUS
if (!configure_signal(SIGBUS, "SIGBUS", sigfatal_handler))
if (!configure_signal(SIGINT, "SIGINT", sigint_handler))
{
return false;
}
#endif

return true;
}
Expand Down Expand Up @@ -1860,12 +1911,10 @@ int main(int argc, char** argv)
* the pipe. */
close(daemon_pipe[0]);
}
/*<
* Set signal handlers for SIGHUP, SIGTERM, SIGINT and critical signals like SIGSEGV.
*/
if (!configure_signals())

if (!configure_critical_signals())
{
static const char* logerr = "Failed to configure signal handlers. Exiting.";
static const char* logerr = "Failed to configure fatal signal handlers. Exiting.";

print_log_n_stderr(true, true, logerr, logerr, 0);
rc = MAXSCALE_INTERNALERROR;
Expand Down Expand Up @@ -2265,9 +2314,19 @@ int main(int argc, char** argv)
write_child_exit_code(daemon_pipe[1], rc);
}

if (!configure_normal_signals())
{
static const char* logerr = "Failed to configure all signal handlers. Exiting.";

print_log_n_stderr(true, true, logerr, logerr, 0);
rc = MAXSCALE_INTERNALERROR;
goto return_main;
}

/*<
* Run worker 0 in the main thread.
*/

worker = RoutingWorker::get(RoutingWorker::MAIN);
mxb_assert(worker);
worker->run();
Expand All @@ -2278,6 +2337,8 @@ int main(int argc, char** argv)
/*< Destroy all monitors */
monitor_destroy_all();

disable_normal_signals();

/*<
* Wait for the housekeeper to finish.
*/
Expand Down

0 comments on commit c9badcb

Please sign in to comment.