Make SIGTERM monitoring optional (#578)

* Add flag for enabling SIGTERM monitoring

* Add integration tests

* Fix format

* Disable signal tests on watchOS

* Add flag to sample

* Fix false-positive OOM reports

Samples/Common/Sources/IntegrationTestsHelper/InstallConfig.swift

@@ -30,6 +30,7 @@ import KSCrashRecording
 public struct InstallConfig: Codable {
     public var installPath: String
     public var isCxaThrowEnabled: Bool?
+    public var isSigTermMonitoringEnabled: Bool?
 
     public init(installPath: String) {
         self.installPath = installPath
@@ -43,6 +44,9 @@ extension InstallConfig {
         if let isCxaThrowEnabled {
             config.enableSwapCxaThrow = isCxaThrowEnabled
         }
+        if let isSigTermMonitoringEnabled {
+            config.enableSigTermMonitoring = isSigTermMonitoringEnabled
+        }
         try KSCrash.shared.install(with: config)
     }
 }

Samples/Common/Sources/IntegrationTestsHelper/IntegrationTestRunner.swift

@@ -79,4 +79,9 @@ public extension IntegrationTestRunner {
         let data = try JSONEncoder().encode(Script(install: install, report: report, delay: delay))
         return data.base64EncodedString()
     }
+
+    static func script(install: InstallConfig? = nil, delay: TimeInterval? = nil) throws -> String {
+        let data = try JSONEncoder().encode(Script(install: install, delay: delay))
+        return data.base64EncodedString()
+    }
 }

Samples/Common/Sources/SampleUI/Screens/InstallView.swift

@@ -76,6 +76,9 @@ struct InstallView: View {
                 Toggle(isOn: bridge.configBinding(for: \.enableSwapCxaThrow)) {
                     Text("Swap __cxa_throw")
                 }
+                Toggle(isOn: bridge.configBinding(for: \.enableSigTermMonitoring)) {
+                    Text("SIGTERM monitoring")
+                }
             }
 
             Button("Only set up reports") {

Samples/Tests/Core/IntegrationTestBase.swift

@@ -158,12 +158,29 @@ class IntegrationTestBase: XCTestCase {
         return report
     }
 
+    func hasCrashReport() throws -> Bool {
+        let reportsDirUrl = installUrl.appendingPathComponent("Reports")
+        let files = try? FileManager.default.contentsOfDirectory(atPath: reportsDirUrl.path)
+        return (files ?? []).isEmpty == false
+    }
+
     func readAppleReport() throws -> String {
         let url = try waitForFile(in: appleReportsUrl, timeout: reportTimeout)
         let appleReport = try String(contentsOf: url)
         return appleReport
     }
 
+    func launchAndInstall(installOverride: ((inout InstallConfig) throws -> Void)? = nil) throws {
+        var installConfig = InstallConfig(installPath: installUrl.path)
+        try installOverride?(&installConfig)
+        app.launchEnvironment[IntegrationTestRunner.envKey] = try IntegrationTestRunner.script(
+            install: installConfig,
+            delay: actionDelay
+        )
+
+        launchAppAndRunScript()
+    }
+
     func launchAndCrash(_ crashId: CrashTriggerId, installOverride: ((inout InstallConfig) throws -> Void)? = nil) throws {
         var installConfig = InstallConfig(installPath: installUrl.path)
         try installOverride?(&installConfig)
@@ -188,4 +205,9 @@ class IntegrationTestBase: XCTestCase {
         let report = try readAppleReport()
         return report
     }
+
+    func terminate() throws {
+        app.terminate()
+        _ = app.wait(for: .notRunning, timeout: self.appTerminateTimeout)
+    }
 }

Samples/Tests/Core/PartialCrashReport.swift

@@ -29,8 +29,17 @@ import Foundation
 struct PartialCrashReport: Decodable {
     struct Crash: Decodable {
         struct Error: Decodable {
+            struct Signal: Decodable {
+                var signal: Int?
+                var name: String?
+                var code: Int?
+                var code_name: String?
+            }
+
             var reason: String?
             var type: String?
+
+            var signal: Signal?
         }
 
         struct Thread: Decodable {

Samples/Tests/IntegrationTests.swift

@@ -70,6 +70,46 @@ final class CppTests: IntegrationTestBase {
     }
 }
 
+#if !os(watchOS)
+
+final class SignalTests: IntegrationTestBase {
+    func testAbort() throws {
+        try launchAndCrash(.signal_abort)
+
+        let rawReport = try readPartialCrashReport()
+        try rawReport.validate()
+        XCTAssertEqual(rawReport.crash?.error?.type, "signal")
+        XCTAssertEqual(rawReport.crash?.error?.signal?.name, "SIGABRT")
+
+        let appleReport = try launchAndReportCrash()
+        XCTAssertTrue(appleReport.contains("SIGABRT"))
+    }
+
+    func testTermination() throws {
+        // Default (termination monitoring disabled)
+        try launchAndInstall()
+        try terminate()
+
+        XCTAssertFalse(try hasCrashReport())
+
+        // With termination monitoring enabled
+        try launchAndInstall { config in
+            config.isSigTermMonitoringEnabled = true
+        }
+        try terminate()
+
+        let rawReport = try readPartialCrashReport()
+        try rawReport.validate()
+        XCTAssertEqual(rawReport.crash?.error?.signal?.name, "SIGTERM")
+
+        let appleReport = try launchAndReportCrash()
+        print(appleReport)
+        XCTAssertTrue(appleReport.contains("SIGTERM"))
+    }
+}
+
+#endif
+
 extension PartialCrashReport {
     func validate() throws {
         let crashedThread = self.crash?.threads?.first(where: { $0.crashed })

Sources/KSCrashRecording/KSCrashC.c

@@ -194,6 +194,7 @@ void handleConfiguration(KSCrashCConfiguration *configuration)
 #endif
     ksccd_setSearchQueueNames(configuration->enableQueueNameSearch);
     kscrashreport_setIntrospectMemory(configuration->enableMemoryIntrospection);
+    kscm_signal_sigterm_setMonitoringEnabled(configuration->enableSigTermMonitoring);
 
     if (configuration->doNotIntrospectClasses.strings != NULL) {
         kscrashreport_setDoNotIntrospectClasses(configuration->doNotIntrospectClasses.strings,

Sources/KSCrashRecording/KSCrashConfiguration.m

@@ -60,6 +60,7 @@ - (instancetype)init
         _addConsoleLogToReport = cConfig.addConsoleLogToReport ? YES : NO;
         _printPreviousLogOnStartup = cConfig.printPreviousLogOnStartup ? YES : NO;
         _enableSwapCxaThrow = cConfig.enableSwapCxaThrow ? YES : NO;
+        _enableSigTermMonitoring = cConfig.enableSigTermMonitoring ? YES : NO;
 
         _reportStoreConfiguration = [KSCrashReportStoreConfiguration new];
         _reportStoreConfiguration.appName = nil;
@@ -102,6 +103,7 @@ - (KSCrashCConfiguration)toCConfiguration
     config.addConsoleLogToReport = self.addConsoleLogToReport;
     config.printPreviousLogOnStartup = self.printPreviousLogOnStartup;
     config.enableSwapCxaThrow = self.enableSwapCxaThrow;
+    config.enableSigTermMonitoring = self.enableSigTermMonitoring;
 
     return config;
 }
@@ -154,6 +156,7 @@ - (nonnull id)copyWithZone:(nullable NSZone *)zone
     copy.addConsoleLogToReport = self.addConsoleLogToReport;
     copy.printPreviousLogOnStartup = self.printPreviousLogOnStartup;
     copy.enableSwapCxaThrow = self.enableSwapCxaThrow;
+    copy.enableSigTermMonitoring = self.enableSigTermMonitoring;
     return copy;
 }
 

Sources/KSCrashRecording/Monitors/KSCrashMonitor_Memory.h

@@ -121,6 +121,12 @@ void ksmemory_set_fatal_reports_enabled(bool enabled);
  */
 bool ksmemory_get_fatal_reports_enabled(void);
 
+/** Notifies memory monitoring logic that there was an unhandled fatal signal.
+ *  E.g. SIGTERM is not considered as a crash by-default but ignoring this signal
+ *  causes false-positive OOM reports.
+ */
+void ksmemory_notifyUnhandledFatalSignal(void);
+
 #ifdef __cplusplus
 }
 #endif

Sources/KSCrashRecording/Monitors/KSCrashMonitor_Memory.m

@@ -569,3 +569,5 @@ bool ksmemory_previous_session_was_terminated_due_to_memory(bool *userPerceptibl
 void ksmemory_set_fatal_reports_enabled(bool enabled) { g_FatalReportsEnabled = enabled; }
 
 bool ksmemory_get_fatal_reports_enabled(void) { return g_FatalReportsEnabled; }
+
+void ksmemory_notifyUnhandledFatalSignal(void) { g_memory->fatal = true; }

Sources/KSCrashRecording/Monitors/KSCrashMonitor_Signal.c

@@ -30,6 +30,7 @@
 #include "KSCrashMonitorContextHelper.h"
 #include "KSCrashMonitorHelper.h"
 #include "KSCrashMonitor_MachException.h"
+#include "KSCrashMonitor_Memory.h"
 #include "KSID.h"
 #include "KSMachineContext.h"
 #include "KSSignalInfo.h"
@@ -52,6 +53,7 @@
 // ============================================================================
 
 static volatile bool g_isEnabled = false;
+static bool g_sigterm_monitoringEnabled = false;
 
 static KSCrash_MonitorContext g_monitorContext;
 static KSStackCursor g_stackCursor;
@@ -66,6 +68,13 @@ static struct sigaction *g_previousSignalHandlers = NULL;
 
 static char g_eventID[37];
 
+// ============================================================================
+#pragma mark - Private -
+// ============================================================================
+
+static void uninstallSignalHandler(void);
+static bool shouldHandleSignal(int sigNum) { return !(sigNum == SIGTERM && !g_sigterm_monitoringEnabled); }
+
 // ============================================================================
 #pragma mark - Callbacks -
 // ============================================================================
@@ -85,7 +94,7 @@ static char g_eventID[37];
 static void handleSignal(int sigNum, siginfo_t *signalInfo, void *userContext)
 {
     KSLOG_DEBUG("Trapped signal %d", sigNum);
-    if (g_isEnabled) {
+    if (g_isEnabled && shouldHandleSignal(sigNum)) {
         thread_act_array_t threads = NULL;
         mach_msg_type_number_t numThreads = 0;
         ksmc_suspendEnvironment(&threads, &numThreads);
@@ -110,6 +119,9 @@ static void handleSignal(int sigNum, siginfo_t *signalInfo, void *userContext)
 
         kscm_handleException(crashContext);
         ksmc_resumeEnvironment(threads, numThreads);
+    } else {
+        uninstallSignalHandler();
+        ksmemory_notifyUnhandledFatalSignal();
     }
 
     KSLOG_DEBUG("Re-raising signal for regular handlers to catch.");
@@ -231,6 +243,13 @@ static void addContextualInfoToEvent(struct KSCrash_MonitorContext *eventContext
 
 #endif /* KSCRASH_HAS_SIGNAL */
 
+void kscm_signal_sigterm_setMonitoringEnabled(bool enabled)
+{
+#if KSCRASH_HAS_SIGNAL
+    g_sigterm_monitoringEnabled = enabled;
+#endif
+}
+
 KSCrashMonitorAPI *kscm_signal_getAPI(void)
 {
 #if KSCRASH_HAS_SIGNAL

Sources/KSCrashRecording/Monitors/KSCrashMonitor_Signal.h

@@ -40,6 +40,13 @@ extern "C" {
  */
 KSCrashMonitorAPI *kscm_signal_getAPI(void);
 
+/** Enables or disables SIGTERM monitoring.
+ *  Default to false.
+ *
+ * @param enabled if true, SIGTERM signals will be monitored and reported.
+ */
+void kscm_signal_sigterm_setMonitoringEnabled(bool enabled);
+
 #ifdef __cplusplus
 }
 #endif

Sources/KSCrashRecording/include/KSCrashCConfiguration.h

@@ -209,6 +209,17 @@ typedef struct {
      * **Default**: true
      */
     bool enableSwapCxaThrow;
+
+    /** If true, enables monitoring for SIGTERM signals.
+     *
+     * A SIGTERM is usually sent to the application by the OS during a graceful shutdown,
+     * but it can also happen on some Watchdog events.
+     * Enabling this can provide more insights into the cause of the SIGTERM, but
+     * it can also generate many false-positive crash reports.
+     *
+     * **Default**: false
+     */
+    bool enableSigTermMonitoring;
 } KSCrashCConfiguration;
 
 static inline KSCrashCConfiguration KSCrashCConfiguration_Default(void)
@@ -226,6 +237,7 @@ static inline KSCrashCConfiguration KSCrashCConfiguration_Default(void)
         .addConsoleLogToReport = false,
         .printPreviousLogOnStartup = false,
         .enableSwapCxaThrow = true,
+        .enableSigTermMonitoring = false,
     };
 }
 

Sources/KSCrashRecording/include/KSCrashConfiguration.h

@@ -152,6 +152,17 @@ NS_ASSUME_NONNULL_BEGIN
  */
 @property(nonatomic, assign) BOOL enableSwapCxaThrow;
 
+/** If true, enables monitoring for SIGTERM signals.
+ *
+ * A SIGTERM is usually sent to the application by the OS during a graceful shutdown,
+ * but it can also happen on some Watchdog events.
+ * Enabling this can provide more insights into the cause of the SIGTERM, but
+ * it can also generate many false-positive crash reports.
+ *
+ * **Default**: false
+ */
+@property(nonatomic, assign) BOOL enableSigTermMonitoring;
+
 @end
 
 NS_SWIFT_NAME(CrashReportStoreConfiguration)