Skip to content

Commit

Permalink
Fix SIGTERM behavior that causes systemd control of etserver to timeo…
Browse files Browse the repository at this point in the history
…ut. (#554)

From various user reports, `systemctl stop et` hangs and times out
requiring a SIGKILL signal. In addition to adding the hook to kill
sentry/telemetry on SIGTERM, we need to exit as well.

Fixes #546

Co-authored-by: James Short <jwshort@fb.com>
  • Loading branch information
jshort and James Short committed Dec 19, 2022
1 parent 10dd371 commit c31f3b9
Show file tree
Hide file tree
Showing 3 changed files with 20 additions and 6 deletions.
1 change: 1 addition & 0 deletions codecov.yml
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@ ignore:
- "src/terminal/PsuedoTerminalConsole.hpp"
- "src/terminal/PsuedoUserTerminal.hpp"
- "src/terminal/*Main.cpp"
- "src/base/Headers.hpp"
- "src/base/TcpSocketHandler*"
- "src/base/SubprocessToString*"
coverage:
Expand Down
11 changes: 9 additions & 2 deletions src/base/Headers.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -409,9 +409,16 @@ inline void HandleTerminate() {
}

inline void InterruptSignalHandler(int signum) {
STERROR << "Got interrupt";
CLOG(INFO, "stdout") << endl
<< "Got interrupt (perhaps ctrl+c?). Exiting." << endl;
<< "Got interrupt (perhaps ctrl+c?): " << signum
<< ". Exiting." << endl;
::exit(signum);
}

inline void TerminateSignalHandler(int signum) {
CLOG(INFO, "stdout") << endl
<< "Got terminate signal: " << signum << ". Exiting."
<< endl;
::exit(signum);
}

Expand Down
14 changes: 10 additions & 4 deletions src/terminal/TelemetryService.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -135,6 +135,7 @@ TelemetryService::TelemetryService(const bool _allow,
}

#ifdef USE_SENTRY
cerr << "Setting up and starting sentry" << endl;
sentry_options_t* options = sentry_options_new();
logHttpClient->set_compress(true);

Expand Down Expand Up @@ -170,9 +171,6 @@ TelemetryService::TelemetryService(const bool _allow,
#ifdef SIGSEGV
SIGSEGV,
#endif
#ifdef SIGTERM
SIGTERM,
#endif
#ifdef SIGKILL
SIGKILL,
#endif
Expand All @@ -181,10 +179,18 @@ TelemetryService::TelemetryService(const bool _allow,
signal(it, sentryShutdownHandler);
}

#ifdef SIGTERM
signal(SIGTERM, [](int i) {
// In addition to exiting like the default SIGTERM handler would do,
// let's shutdown sentry telemetry
shutdownTelemetry();
et::TerminateSignalHandler(i);
});
#endif
#ifdef SIGINT
signal(SIGINT, [](int i) {
shutdownTelemetry();
// Normally this is installed in TerminalServerMain, TerminalClientMain,
// Normally this is configured in TerminalServerMain, TerminalClientMain,
// and TerminalMain, but we need to forward the call since Sentry
// overrides it. This is important to handle SIGINT from Ctrl-C.
et::InterruptSignalHandler(i);
Expand Down

0 comments on commit c31f3b9

Please sign in to comment.