From c31f3b9fd4b6a8800c5d3877abd22ac1032c46f6 Mon Sep 17 00:00:00 2001
From: James Short <james.short@alumni.duke.edu>
Date: Mon, 19 Dec 2022 09:11:16 -0800
Subject: [PATCH] Fix SIGTERM behavior that causes systemd control of etserver
 to timeout. (#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>
---
 codecov.yml                       |  1 +
 src/base/Headers.hpp              | 11 +++++++++--
 src/terminal/TelemetryService.cpp | 14 ++++++++++----
 3 files changed, 20 insertions(+), 6 deletions(-)

diff --git a/codecov.yml b/codecov.yml
index e5d3a74b9..e27cf6203 100644
--- a/codecov.yml
+++ b/codecov.yml
@@ -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:
diff --git a/src/base/Headers.hpp b/src/base/Headers.hpp
index e9b06a860..3e7da057f 100644
--- a/src/base/Headers.hpp
+++ b/src/base/Headers.hpp
@@ -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);
 }
 
diff --git a/src/terminal/TelemetryService.cpp b/src/terminal/TelemetryService.cpp
index 895a4167d..c354b30e7 100644
--- a/src/terminal/TelemetryService.cpp
+++ b/src/terminal/TelemetryService.cpp
@@ -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);
 
@@ -170,9 +171,6 @@ TelemetryService::TelemetryService(const bool _allow,
 #ifdef SIGSEGV
         SIGSEGV,
 #endif
-#ifdef SIGTERM
-        SIGTERM,
-#endif
 #ifdef SIGKILL
         SIGKILL,
 #endif
@@ -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);