From c85533048db6aefa7215d7754afc28579c799ba7 Mon Sep 17 00:00:00 2001 From: Fedor Batonogov Date: Sun, 29 Mar 2026 13:39:41 +0300 Subject: [PATCH 1/7] feat: add opt-in crash reporting with MetricKit and signal handler fallback (#472) - MetricKit (MXCrashDiagnostic) as primary crash diagnostic source - POSIX signal handler fallback (strictly async-signal-safe: only write/_exit) - Opt-in dialog on first launch with privacy explanation - Toggle in File menu to enable/disable at any time - CrashReport model with Codable serialization - CrashReportStore persists reports to Application Support - Localization for all 9 languages (de, en, es, fr, ja, ko, pt-BR, ru, zh-Hans) - 32 unit tests covering settings, model, store, parseCallStack, edge cases --- Pine/ContentView+Helpers.swift | 13 + Pine/ContentView.swift | 9 + Pine/CrashReport.swift | 104 ++++++++ Pine/CrashReportStore.swift | 114 ++++++++ Pine/CrashReportingManager.swift | 198 ++++++++++++++ Pine/CrashReportingOptInView.swift | 56 ++++ Pine/CrashReportingSettings.swift | 41 +++ Pine/Localizable.xcstrings | 360 +++++++++++++++++++++++++ Pine/MenuIcons.swift | 3 + Pine/PineApp.swift | 15 ++ Pine/Strings.swift | 7 + PineTests/CrashReportingTests.swift | 398 ++++++++++++++++++++++++++++ 12 files changed, 1318 insertions(+) create mode 100644 Pine/CrashReport.swift create mode 100644 Pine/CrashReportStore.swift create mode 100644 Pine/CrashReportingManager.swift create mode 100644 Pine/CrashReportingOptInView.swift create mode 100644 Pine/CrashReportingSettings.swift create mode 100644 PineTests/CrashReportingTests.swift diff --git a/Pine/ContentView+Helpers.swift b/Pine/ContentView+Helpers.swift index a4042f5..c6af6e0 100644 --- a/Pine/ContentView+Helpers.swift +++ b/Pine/ContentView+Helpers.swift @@ -505,6 +505,19 @@ extension ContentView { } } +// MARK: - Crash Reporting Opt-In + +extension ContentView { + /// Shows the crash reporting opt-in dialog on first launch. + func showCrashReportingOptInIfNeeded() { + guard CrashReportingSettings.needsPrompt else { return } + // Slight delay to avoid showing during initial window setup + DispatchQueue.main.asyncAfter(deadline: .now() + 1.0) { + showCrashReportingOptIn = true + } + } +} + // MARK: - Line / offset helpers extension ContentView { diff --git a/Pine/ContentView.swift b/Pine/ContentView.swift index b12a0bb..ed912ea 100644 --- a/Pine/ContentView.swift +++ b/Pine/ContentView.swift @@ -36,6 +36,7 @@ struct ContentView: View { @State var isQuickOpenPresented = false @State var isSymbolNavigatorPresented = false @State var showGoToLine = false + @State var showCrashReportingOptIn = false @AppStorage("minimapVisible") var isMinimapVisible = true @AppStorage(BlameConstants.storageKey) var isBlameVisible = true @AppStorage("wordWrapEnabled") var isWordWrapEnabled = true @@ -114,6 +115,7 @@ struct ContentView: View { syncSidebarSelection() applySearchQueryFromEnvironment() refreshBlame() + showCrashReportingOptInIfNeeded() } .sheet(isPresented: $showRecoveryDialog) { RecoveryDialogView( @@ -223,6 +225,13 @@ struct ContentView: View { tabManager.pendingGoToLine = nil goToLineOffset = GoToRequest(offset: Self.cursorOffset(forLine: line, in: tab.content)) } + .sheet(isPresented: $showCrashReportingOptIn) { + CrashReportingOptInView(isPresented: $showCrashReportingOptIn) { enabled in + if enabled { + CrashReportingManager.shared.startIfEnabled() + } + } + } } /// Branch subtitle as a plain String to avoid generating a localization key. diff --git a/Pine/CrashReport.swift b/Pine/CrashReport.swift new file mode 100644 index 0000000..6fc07e1 --- /dev/null +++ b/Pine/CrashReport.swift @@ -0,0 +1,104 @@ +// +// CrashReport.swift +// Pine +// +// Model for crash diagnostic data collected via MetricKit. +// + +import Foundation + +/// A structured crash report collected from MetricKit diagnostics. +struct CrashReport: Codable, Equatable, Sendable { + /// Unique identifier for deduplication. + let id: UUID + + /// Timestamp when the crash occurred. + let timestamp: Date + + /// App version at the time of crash (CFBundleShortVersionString). + let appVersion: String + + /// Build number at the time of crash (CFBundleVersion). + let buildNumber: String + + /// macOS version string (e.g. "26.0"). + let osVersion: String + + /// Signal that caused the crash (e.g. SIGSEGV, SIGABRT). + let signal: String? + + /// Exception type if available. + let exceptionType: String? + + /// Termination reason if available. + let terminationReason: String? + + /// Call stack frames as human-readable strings. + let callStackFrames: [String] + + /// Number of open editor tabs at crash time (privacy-safe metric). + let openTabCount: Int? + + /// Creates a CrashReport with current app/OS metadata. + init( + id: UUID = UUID(), + timestamp: Date = Date(), + signal: String? = nil, + exceptionType: String? = nil, + terminationReason: String? = nil, + callStackFrames: [String] = [], + openTabCount: Int? = nil + ) { + self.id = id + self.timestamp = timestamp + self.appVersion = Bundle.main.object(forInfoDictionaryKey: "CFBundleShortVersionString") as? String ?? "unknown" + self.buildNumber = Bundle.main.object(forInfoDictionaryKey: "CFBundleVersion") as? String ?? "unknown" + self.osVersion = ProcessInfo.processInfo.operatingSystemVersionString + self.signal = signal + self.exceptionType = exceptionType + self.terminationReason = terminationReason + self.callStackFrames = callStackFrames + self.openTabCount = openTabCount + } + + /// Internal initializer for testing with explicit app/OS values. + init( + id: UUID, + timestamp: Date, + appVersion: String, + buildNumber: String, + osVersion: String, + signal: String?, + exceptionType: String?, + terminationReason: String?, + callStackFrames: [String], + openTabCount: Int? + ) { + self.id = id + self.timestamp = timestamp + self.appVersion = appVersion + self.buildNumber = buildNumber + self.osVersion = osVersion + self.signal = signal + self.exceptionType = exceptionType + self.terminationReason = terminationReason + self.callStackFrames = callStackFrames + self.openTabCount = openTabCount + } +} + +// MARK: - Call Stack Parsing + +extension CrashReport { + /// Parses a raw call stack string into individual frame strings. + /// Handles both MetricKit JSON format and standard crash log format. + /// + /// Each frame typically looks like: + /// `0 Pine 0x00000001000a1234 someFunction + 42` + static func parseCallStack(_ rawCallStack: String) -> [String] { + let lines = rawCallStack.components(separatedBy: .newlines) + return lines + .map { $0.trimmingCharacters(in: .whitespaces) } + .filter { !$0.isEmpty } + } +} diff --git a/Pine/CrashReportStore.swift b/Pine/CrashReportStore.swift new file mode 100644 index 0000000..811e0b2 --- /dev/null +++ b/Pine/CrashReportStore.swift @@ -0,0 +1,114 @@ +// +// CrashReportStore.swift +// Pine +// +// Persists crash reports to disk for display on next launch. +// + +import Foundation +import os + +/// Persists crash reports as JSON files in the Application Support directory. +/// Reports are stored individually for atomic read/write and easy cleanup. +final class CrashReportStore { + /// Shared singleton instance. + static let shared = CrashReportStore() + + /// Directory where crash reports are stored. + let storageDirectory: URL + + private let logger = Logger(subsystem: "com.pine.editor", category: "CrashReportStore") + + /// Maximum number of reports to keep on disk. + static let maxReports = 50 + + /// File extension for crash report files. + static let fileExtension = "crashreport" + + init(storageDirectory: URL? = nil) { + if let dir = storageDirectory { + self.storageDirectory = dir + } else if let appSupport = FileManager.default.urls( + for: .applicationSupportDirectory, in: .userDomainMask + ).first { + self.storageDirectory = appSupport.appendingPathComponent("Pine/CrashReports") + } else { + self.storageDirectory = URL(fileURLWithPath: NSTemporaryDirectory()) + .appendingPathComponent("Pine/CrashReports") + } + try? FileManager.default.createDirectory(at: self.storageDirectory, withIntermediateDirectories: true) + } + + /// Saves a crash report to disk. + func save(_ report: CrashReport) { + let fileName = "\(report.id.uuidString).\(Self.fileExtension)" + let fileURL = storageDirectory.appendingPathComponent(fileName) + + do { + let data = try JSONEncoder().encode(report) + try data.write(to: fileURL, options: .atomic) + pruneOldReports() + } catch { + logger.error("Failed to save crash report: \(error.localizedDescription)") + } + } + + /// Loads all stored crash reports, sorted by timestamp (newest first). + func loadAll() -> [CrashReport] { + let fm = FileManager.default + guard let files = try? fm.contentsOfDirectory(at: storageDirectory, includingPropertiesForKeys: nil) else { + return [] + } + + let reports: [CrashReport] = files + .filter { $0.pathExtension == Self.fileExtension } + .compactMap { url in + guard let data = try? Data(contentsOf: url), + let report = try? JSONDecoder().decode(CrashReport.self, from: data) else { + return nil + } + return report + } + .sorted { $0.timestamp > $1.timestamp } + + return reports + } + + /// Removes a specific crash report by ID. + func remove(id: UUID) { + let fileName = "\(id.uuidString).\(Self.fileExtension)" + let fileURL = storageDirectory.appendingPathComponent(fileName) + try? FileManager.default.removeItem(at: fileURL) + } + + /// Removes all stored crash reports. + func removeAll() { + let fm = FileManager.default + guard let files = try? fm.contentsOfDirectory(at: storageDirectory, includingPropertiesForKeys: nil) else { + return + } + for file in files where file.pathExtension == Self.fileExtension { + try? fm.removeItem(at: file) + } + } + + /// Returns the number of stored reports. + var count: Int { + let fm = FileManager.default + guard let files = try? fm.contentsOfDirectory(at: storageDirectory, includingPropertiesForKeys: nil) else { + return 0 + } + return files.filter { $0.pathExtension == Self.fileExtension }.count + } + + /// Prunes old reports if count exceeds the maximum. + private func pruneOldReports() { + let reports = loadAll() + guard reports.count > Self.maxReports else { return } + + let toRemove = reports.suffix(from: Self.maxReports) + for report in toRemove { + remove(id: report.id) + } + } +} diff --git a/Pine/CrashReportingManager.swift b/Pine/CrashReportingManager.swift new file mode 100644 index 0000000..e0f9cd2 --- /dev/null +++ b/Pine/CrashReportingManager.swift @@ -0,0 +1,198 @@ +// +// CrashReportingManager.swift +// Pine +// +// Central coordinator for crash reporting using MetricKit as primary +// and POSIX signal handlers as a minimal fallback. +// + +import Foundation +import MetricKit +import os + +/// Coordinates crash reporting using MetricKit (MXCrashDiagnostic) as the +/// primary source and POSIX signal handlers as a minimal async-signal-safe fallback. +/// +/// MetricKit delivers crash diagnostics on next launch via `MXMetricManagerSubscriber`. +/// The signal handler writes a minimal marker file using only POSIX APIs (no Swift/Foundation). +final class CrashReportingManager: NSObject, MXMetricManagerSubscriber { + /// Shared singleton. + static let shared = CrashReportingManager() + + private let logger = Logger(subsystem: "com.pine.editor", category: "CrashReporting") + private let store: CrashReportStore + + /// Path to the signal handler's crash marker file. + /// Written by the C-level signal handler using only async-signal-safe POSIX calls. + static var crashMarkerPath: String { + guard let appSupport = FileManager.default.urls( + for: .applicationSupportDirectory, in: .userDomainMask + ).first else { + return NSTemporaryDirectory() + "Pine_crash_marker" + } + return appSupport.appendingPathComponent("Pine/crash_marker").path + } + + init(store: CrashReportStore = .shared) { + self.store = store + super.init() + } + + /// Starts crash reporting if the user has opted in. + /// Call this from `applicationDidFinishLaunching`. + func startIfEnabled() { + guard CrashReportingSettings.isEnabled else { + logger.info("Crash reporting is disabled by user preference") + return + } + + // Subscribe to MetricKit diagnostics + MXMetricManager.shared.add(self) + + // Install signal handler fallback + installSignalHandlers() + + // Check for crash marker from previous signal-based crash + checkForCrashMarker() + + logger.info("Crash reporting started (MetricKit + signal handler fallback)") + } + + /// Stops crash reporting (unsubscribes from MetricKit). + func stop() { + MXMetricManager.shared.remove(self) + logger.info("Crash reporting stopped") + } + + // MARK: - MXMetricManagerSubscriber + + /// Called by MetricKit when crash diagnostics are available (typically next launch). + func didReceive(_ payloads: [MXDiagnosticPayload]) { + guard CrashReportingSettings.isEnabled else { return } + + for payload in payloads { + if let crashDiagnostics = payload.crashDiagnostics { + for diagnostic in crashDiagnostics { + processCrashDiagnostic(diagnostic) + } + } + } + } + + /// Processes a single MetricKit crash diagnostic into a CrashReport. + private func processCrashDiagnostic(_ diagnostic: MXCrashDiagnostic) { + let callStack: [String] + let jsonData = diagnostic.jsonRepresentation() + if let json = try? JSONSerialization.jsonObject(with: jsonData) as? [String: Any], + let stacks = json["callStackTree"] as? [String: Any], + let callStackString = stacks.description.data(using: .utf8) { + callStack = CrashReport.parseCallStack(String(data: callStackString, encoding: .utf8) ?? "") + } else { + callStack = [] + } + + let report = CrashReport( + signal: diagnostic.signal?.description, + exceptionType: diagnostic.exceptionType?.description, + terminationReason: diagnostic.terminationReason, + callStackFrames: callStack + ) + + store.save(report) + logger.info("Saved MetricKit crash report: \(report.id)") + } + + // MARK: - Signal Handler Fallback + + /// Installs POSIX signal handlers for common crash signals. + /// The handler is strictly async-signal-safe: only POSIX write() and _exit(). + private func installSignalHandlers() { + let signals: [Int32] = [SIGSEGV, SIGABRT, SIGBUS, SIGFPE, SIGILL, SIGTRAP] + + for sig in signals { + var action = sigaction() + action.__sigaction_u.__sa_handler = signalHandler + sigemptyset(&action.sa_mask) + action.sa_flags = 0 + sigaction(sig, &action, nil) + } + } + + /// Checks for a crash marker file left by the signal handler. + /// If found, creates a minimal CrashReport from it. + private func checkForCrashMarker() { + let path = Self.crashMarkerPath + let fm = FileManager.default + + guard fm.fileExists(atPath: path) else { return } + + // Read the signal number from the marker file + let signalName: String? + if let data = fm.contents(atPath: path), + let content = String(data: data, encoding: .utf8)?.trimmingCharacters(in: .whitespacesAndNewlines), + !content.isEmpty { + signalName = content + } else { + signalName = "unknown" + } + + let report = CrashReport( + signal: signalName, + exceptionType: "Signal crash (fallback handler)", + callStackFrames: ["[Call stack not available — captured by signal handler fallback]"] + ) + + store.save(report) + logger.info("Saved signal-handler crash report: \(report.id)") + + // Clean up the marker file + try? fm.removeItem(atPath: path) + } +} + +// MARK: - Async-signal-safe crash handler (C-level, no Swift/Foundation) + +/// Strictly async-signal-safe signal handler. +/// Uses ONLY POSIX APIs: open(), write(), close(), _exit(). +/// No Swift runtime, no Foundation, no malloc, no ObjC messaging. +private func signalHandler(_ signal: Int32) { + // Get the crash marker path — this is a compile-time known location + // We cannot use Swift String here, so we use a pre-computed C string + let path = CrashReportingManager.crashMarkerPath + + path.withCString { cPath in + // Open file for writing (create if needed, truncate) + let fd = open(cPath, O_WRONLY | O_CREAT | O_TRUNC, 0o644) + guard fd >= 0 else { + _exit(signal) + } + + // Write the signal number as ASCII digits + var sigNum = signal + var digits: [UInt8] = [] + + if sigNum == 0 { + digits.append(0x30) // '0' + } else { + while sigNum > 0 { + digits.append(UInt8(0x30 + sigNum % 10)) + sigNum /= 10 + } + digits.reverse() + } + + digits.withUnsafeBufferPointer { buf in + _ = write(fd, buf.baseAddress, buf.count) + } + + // Write newline + var newline: UInt8 = 0x0A + _ = write(fd, &newline, 1) + + close(fd) + } + + // Re-raise with default handler to get proper exit code + Darwin.signal(signal, SIG_DFL) + raise(signal) +} diff --git a/Pine/CrashReportingOptInView.swift b/Pine/CrashReportingOptInView.swift new file mode 100644 index 0000000..a90086f --- /dev/null +++ b/Pine/CrashReportingOptInView.swift @@ -0,0 +1,56 @@ +// +// CrashReportingOptInView.swift +// Pine +// +// First-launch opt-in dialog for crash reporting. +// + +import SwiftUI + +/// Opt-in dialog shown on first launch asking the user to enable crash reporting. +/// Explains what data is collected and provides clear Enable/Disable choices. +struct CrashReportingOptInView: View { + @Binding var isPresented: Bool + var onChoice: (Bool) -> Void + + var body: some View { + VStack(spacing: 16) { + Image(systemName: MenuIcons.crashReporting) + .font(.system(size: 40)) + .foregroundStyle(.secondary) + + Text(Strings.crashReportingOptInTitle) + .font(.headline) + + Text(Strings.crashReportingOptInMessage) + .font(.body) + .foregroundStyle(.secondary) + .multilineTextAlignment(.center) + .fixedSize(horizontal: false, vertical: true) + + Text(Strings.crashReportingOptInPrivacy) + .font(.caption) + .foregroundStyle(.tertiary) + .multilineTextAlignment(.center) + .fixedSize(horizontal: false, vertical: true) + + HStack(spacing: 12) { + Button(String(localized: "crashReporting.optIn.disable")) { + CrashReportingSettings.recordChoice(enabled: false) + onChoice(false) + isPresented = false + } + .keyboardShortcut(.cancelAction) + + Button(String(localized: "crashReporting.optIn.enable")) { + CrashReportingSettings.recordChoice(enabled: true) + onChoice(true) + isPresented = false + } + .keyboardShortcut(.defaultAction) + } + } + .padding(24) + .frame(width: 400) + } +} diff --git a/Pine/CrashReportingSettings.swift b/Pine/CrashReportingSettings.swift new file mode 100644 index 0000000..4b5696e --- /dev/null +++ b/Pine/CrashReportingSettings.swift @@ -0,0 +1,41 @@ +// +// CrashReportingSettings.swift +// Pine +// +// Manages user preference for opt-in crash reporting. +// + +import Foundation + +/// Manages the opt-in crash reporting preference. +/// Persisted via UserDefaults with a clear opt-in flow. +enum CrashReportingSettings { + /// UserDefaults key for the crash reporting enabled state. + static let enabledKey = "crashReporting.enabled" + + /// UserDefaults key tracking whether the opt-in dialog has been shown. + static let promptShownKey = "crashReporting.promptShown" + + /// Whether crash reporting is currently enabled. + static var isEnabled: Bool { + get { UserDefaults.standard.bool(forKey: enabledKey) } + set { UserDefaults.standard.set(newValue, forKey: enabledKey) } + } + + /// Whether the opt-in prompt has already been shown to the user. + static var hasShownPrompt: Bool { + get { UserDefaults.standard.bool(forKey: promptShownKey) } + set { UserDefaults.standard.set(newValue, forKey: promptShownKey) } + } + + /// Returns true if we need to show the opt-in dialog (first launch). + static var needsPrompt: Bool { + !hasShownPrompt + } + + /// Records that the user made a choice and marks the prompt as shown. + static func recordChoice(enabled: Bool) { + isEnabled = enabled + hasShownPrompt = true + } +} diff --git a/Pine/Localizable.xcstrings b/Pine/Localizable.xcstrings index 4de42b2..441cdeb 100644 --- a/Pine/Localizable.xcstrings +++ b/Pine/Localizable.xcstrings @@ -1443,6 +1443,366 @@ } } }, + "crashReporting.optIn.disable" : { + "comment" : "Button to decline crash reporting in the opt-in dialog.", + "extractionState" : "manual", + "localizations" : { + "de" : { + "stringUnit" : { + "state" : "translated", + "value" : "Nicht aktivieren" + } + }, + "en" : { + "stringUnit" : { + "state" : "translated", + "value" : "Don't Enable" + } + }, + "es" : { + "stringUnit" : { + "state" : "translated", + "value" : "No activar" + } + }, + "fr" : { + "stringUnit" : { + "state" : "translated", + "value" : "Ne pas activer" + } + }, + "ja" : { + "stringUnit" : { + "state" : "translated", + "value" : "有効にしない" + } + }, + "ko" : { + "stringUnit" : { + "state" : "translated", + "value" : "활성화하지 않기" + } + }, + "pt-BR" : { + "stringUnit" : { + "state" : "translated", + "value" : "Não ativar" + } + }, + "ru" : { + "stringUnit" : { + "state" : "translated", + "value" : "Не включать" + } + }, + "zh-Hans" : { + "stringUnit" : { + "state" : "translated", + "value" : "不启用" + } + } + } + }, + "crashReporting.optIn.enable" : { + "comment" : "Button to accept crash reporting in the opt-in dialog.", + "extractionState" : "manual", + "localizations" : { + "de" : { + "stringUnit" : { + "state" : "translated", + "value" : "Aktivieren" + } + }, + "en" : { + "stringUnit" : { + "state" : "translated", + "value" : "Enable" + } + }, + "es" : { + "stringUnit" : { + "state" : "translated", + "value" : "Activar" + } + }, + "fr" : { + "stringUnit" : { + "state" : "translated", + "value" : "Activer" + } + }, + "ja" : { + "stringUnit" : { + "state" : "translated", + "value" : "有効にする" + } + }, + "ko" : { + "stringUnit" : { + "state" : "translated", + "value" : "활성화" + } + }, + "pt-BR" : { + "stringUnit" : { + "state" : "translated", + "value" : "Ativar" + } + }, + "ru" : { + "stringUnit" : { + "state" : "translated", + "value" : "Включить" + } + }, + "zh-Hans" : { + "stringUnit" : { + "state" : "translated", + "value" : "启用" + } + } + } + }, + "crashReporting.optIn.message" : { + "comment" : "Explanatory message in the crash reporting opt-in dialog.", + "extractionState" : "manual", + "localizations" : { + "de" : { + "stringUnit" : { + "state" : "translated", + "value" : "Pine kann anonyme Absturzberichte senden, um die App-Stabilität zu verbessern. Es werden nur technische Daten gesendet — keine Dateiinhalte oder Pfade." + } + }, + "en" : { + "stringUnit" : { + "state" : "translated", + "value" : "Pine can send anonymous crash reports to help improve app stability. Only technical data is sent — no file contents or paths." + } + }, + "es" : { + "stringUnit" : { + "state" : "translated", + "value" : "Pine puede enviar informes de error anónimos para mejorar la estabilidad. Solo se envían datos técnicos, sin contenido ni rutas de archivos." + } + }, + "fr" : { + "stringUnit" : { + "state" : "translated", + "value" : "Pine peut envoyer des rapports de plantage anonymes pour améliorer la stabilité. Seules des données techniques sont envoyées — aucun contenu ni chemin de fichier." + } + }, + "ja" : { + "stringUnit" : { + "state" : "translated", + "value" : "Pineはアプリの安定性向上のため匿名のクラッシュレポートを送信できます。技術データのみが送信され、ファイルの内容やパスは含まれません。" + } + }, + "ko" : { + "stringUnit" : { + "state" : "translated", + "value" : "Pine은 앱 안정성 향상을 위해 익명 충돌 보고서를 보낼 수 있습니다. 기술 데이터만 전송되며 파일 내용이나 경로는 포함되지 않습니다." + } + }, + "pt-BR" : { + "stringUnit" : { + "state" : "translated", + "value" : "Pine pode enviar relatórios de falhas anônimos para melhorar a estabilidade. Apenas dados técnicos são enviados — sem conteúdo ou caminhos de arquivos." + } + }, + "ru" : { + "stringUnit" : { + "state" : "translated", + "value" : "Pine может отправлять анонимные отчёты об ошибках для улучшения стабильности. Передаются только технические данные — без содержимого файлов и путей." + } + }, + "zh-Hans" : { + "stringUnit" : { + "state" : "translated", + "value" : "Pine 可以发送匿名崩溃报告以帮助改进应用稳定性。仅发送技术数据，不包含文件内容或路径。" + } + } + } + }, + "crashReporting.optIn.privacy" : { + "comment" : "Privacy note in the crash reporting opt-in dialog.", + "extractionState" : "manual", + "localizations" : { + "de" : { + "stringUnit" : { + "state" : "translated", + "value" : "Gesammelt: Stack-Trace, OS-Version, App-Version, Anzahl offener Tabs. Sie können dies jederzeit im Menü Datei ändern." + } + }, + "en" : { + "stringUnit" : { + "state" : "translated", + "value" : "Collected: stack trace, OS version, app version, open tab count. You can change this anytime in the File menu." + } + }, + "es" : { + "stringUnit" : { + "state" : "translated", + "value" : "Recopilado: traza de pila, versión del SO, versión de la app, cantidad de pestañas abiertas. Puedes cambiarlo en cualquier momento en el menú Archivo." + } + }, + "fr" : { + "stringUnit" : { + "state" : "translated", + "value" : "Collecté : trace de pile, version du SE, version de l'app, nombre d'onglets ouverts. Vous pouvez modifier ce choix à tout moment dans le menu Fichier." + } + }, + "ja" : { + "stringUnit" : { + "state" : "translated", + "value" : "収集データ：スタックトレース、OS バージョン、アプリバージョン、開いているタブ数。「ファイル」メニューからいつでも変更できます。" + } + }, + "ko" : { + "stringUnit" : { + "state" : "translated", + "value" : "수집 항목: 스택 추적, OS 버전, 앱 버전, 열린 탭 수. 파일 메뉴에서 언제든지 변경할 수 있습니다." + } + }, + "pt-BR" : { + "stringUnit" : { + "state" : "translated", + "value" : "Coletado: rastreamento de pilha, versão do SO, versão do app, quantidade de abas abertas. Você pode alterar a qualquer momento no menu Arquivo." + } + }, + "ru" : { + "stringUnit" : { + "state" : "translated", + "value" : "Собирается: стек вызовов, версия ОС, версия приложения, количество открытых вкладок. Вы можете изменить это в любое время в меню «Файл»." + } + }, + "zh-Hans" : { + "stringUnit" : { + "state" : "translated", + "value" : "收集内容：堆栈跟踪、操作系统版本、应用版本、打开的标签页数量。您可以随时在「文件」菜单中更改此设置。" + } + } + } + }, + "crashReporting.optIn.title" : { + "comment" : "Title of the crash reporting opt-in dialog shown on first launch.", + "extractionState" : "manual", + "localizations" : { + "de" : { + "stringUnit" : { + "state" : "translated", + "value" : "Absturzberichte verbessern Pine" + } + }, + "en" : { + "stringUnit" : { + "state" : "translated", + "value" : "Crash Reports Help Improve Pine" + } + }, + "es" : { + "stringUnit" : { + "state" : "translated", + "value" : "Los informes de error mejoran Pine" + } + }, + "fr" : { + "stringUnit" : { + "state" : "translated", + "value" : "Les rapports de plantage aident à améliorer Pine" + } + }, + "ja" : { + "stringUnit" : { + "state" : "translated", + "value" : "クラッシュレポートで Pine を改善" + } + }, + "ko" : { + "stringUnit" : { + "state" : "translated", + "value" : "충돌 보고서로 Pine 개선에 기여" + } + }, + "pt-BR" : { + "stringUnit" : { + "state" : "translated", + "value" : "Relatórios de falhas ajudam a melhorar o Pine" + } + }, + "ru" : { + "stringUnit" : { + "state" : "translated", + "value" : "Отчёты о сбоях помогают улучшить Pine" + } + }, + "zh-Hans" : { + "stringUnit" : { + "state" : "translated", + "value" : "崩溃报告有助于改进 Pine" + } + } + } + }, + "menu.crashReporting" : { + "comment" : "Menu item to toggle crash reporting on or off.", + "extractionState" : "manual", + "localizations" : { + "de" : { + "stringUnit" : { + "state" : "translated", + "value" : "Absturzberichte" + } + }, + "en" : { + "stringUnit" : { + "state" : "translated", + "value" : "Crash Reporting" + } + }, + "es" : { + "stringUnit" : { + "state" : "translated", + "value" : "Informes de error" + } + }, + "fr" : { + "stringUnit" : { + "state" : "translated", + "value" : "Rapports de plantage" + } + }, + "ja" : { + "stringUnit" : { + "state" : "translated", + "value" : "クラッシュレポート" + } + }, + "ko" : { + "stringUnit" : { + "state" : "translated", + "value" : "충돌 보고" + } + }, + "pt-BR" : { + "stringUnit" : { + "state" : "translated", + "value" : "Relatórios de falhas" + } + }, + "ru" : { + "stringUnit" : { + "state" : "translated", + "value" : "Отчёты о сбоях" + } + }, + "zh-Hans" : { + "stringUnit" : { + "state" : "translated", + "value" : "崩溃报告" + } + } + } + }, "dialog.ok" : { "comment" : "OK button in dialogs.", "extractionState" : "manual", diff --git a/Pine/MenuIcons.swift b/Pine/MenuIcons.swift index 6e10719..d948b52 100644 --- a/Pine/MenuIcons.swift +++ b/Pine/MenuIcons.swift @@ -54,6 +54,9 @@ nonisolated enum MenuIcons { // MARK: - Validation static let toggleValidation = "checkmark.shield" + // MARK: - Crash Reporting + static let crashReporting = "ant.circle" + // MARK: - Context menu static let newFile = "doc.badge.plus" static let newFolder = "folder.badge.plus" diff --git a/Pine/PineApp.swift b/Pine/PineApp.swift index 155bc09..f4da508 100644 --- a/Pine/PineApp.swift +++ b/Pine/PineApp.swift @@ -13,6 +13,7 @@ struct PineApp: App { @NSApplicationDelegateAdaptor(AppDelegate.self) var appDelegate @FocusedValue(\.projectManager) private var focusedProject: ProjectManager? @AppStorage(TabManager.autoSaveKey) private var autoSaveEnabled = false + @AppStorage(CrashReportingSettings.enabledKey) private var crashReportingEnabled = false private var registry: ProjectRegistry { appDelegate.registry } @@ -449,6 +450,17 @@ struct PineApp: App { Toggle(isOn: $autoSaveEnabled) { Label(Strings.menuAutoSave, systemImage: MenuIcons.autoSave) } + + Toggle(isOn: $crashReportingEnabled) { + Label(Strings.menuCrashReporting, systemImage: MenuIcons.crashReporting) + } + .onChange(of: crashReportingEnabled) { _, newValue in + if newValue { + CrashReportingManager.shared.startIfEnabled() + } else { + CrashReportingManager.shared.stop() + } + } } // Cmd+W is intercepted by AppDelegate's local event monitor // to close the active tab. The close button goes through @@ -949,6 +961,9 @@ class AppDelegate: NSObject, NSApplicationDelegate, SPUUpdaterDelegate { // Clean up stale recovery files older than 7 days across all projects RecoveryManager.cleanupAllStaleEntries(olderThan: 7) + // Start crash reporting if previously opted in + CrashReportingManager.shared.startIfEnabled() + // Intercept Cmd+W before the system "Close" menu item. // For project windows: close active tab (or close window if no tabs). // For other windows: pass through to default behavior. diff --git a/Pine/Strings.swift b/Pine/Strings.swift index 2b2477e..802573a 100644 --- a/Pine/Strings.swift +++ b/Pine/Strings.swift @@ -425,6 +425,13 @@ enum Strings { String(localized: "toast.filesReloaded.more \(count) \(names) \(remaining)") } + // MARK: - Crash Reporting + + static let menuCrashReporting: LocalizedStringKey = "menu.crashReporting" + static let crashReportingOptInTitle: LocalizedStringKey = "crashReporting.optIn.title" + static let crashReportingOptInMessage: LocalizedStringKey = "crashReporting.optIn.message" + static let crashReportingOptInPrivacy: LocalizedStringKey = "crashReporting.optIn.privacy" + // MARK: - Progress Indicators static var progressLoadingProject: String { diff --git a/PineTests/CrashReportingTests.swift b/PineTests/CrashReportingTests.swift new file mode 100644 index 0000000..b178999 --- /dev/null +++ b/PineTests/CrashReportingTests.swift @@ -0,0 +1,398 @@ +// +// CrashReportingTests.swift +// PineTests +// + +import Testing +import AppKit +import Foundation +@testable import Pine + +// MARK: - CrashReportingSettings Tests + +struct CrashReportingSettingsTests { + + // Use a unique suite name to avoid collisions with the app's real defaults + private func withCleanDefaults(_ body: () -> Void) { + let enabledKey = CrashReportingSettings.enabledKey + let promptKey = CrashReportingSettings.promptShownKey + let savedEnabled = UserDefaults.standard.object(forKey: enabledKey) + let savedPrompt = UserDefaults.standard.object(forKey: promptKey) + + // Clear + UserDefaults.standard.removeObject(forKey: enabledKey) + UserDefaults.standard.removeObject(forKey: promptKey) + + body() + + // Restore + if let val = savedEnabled { + UserDefaults.standard.set(val, forKey: enabledKey) + } else { + UserDefaults.standard.removeObject(forKey: enabledKey) + } + if let val = savedPrompt { + UserDefaults.standard.set(val, forKey: promptKey) + } else { + UserDefaults.standard.removeObject(forKey: promptKey) + } + } + + @Test func defaultState_isDisabled() { + withCleanDefaults { + #expect(!CrashReportingSettings.isEnabled) + } + } + + @Test func defaultState_promptNotShown() { + withCleanDefaults { + #expect(!CrashReportingSettings.hasShownPrompt) + } + } + + @Test func needsPrompt_trueWhenNotShown() { + withCleanDefaults { + #expect(CrashReportingSettings.needsPrompt) + } + } + + @Test func recordChoice_enabled_setsValues() { + withCleanDefaults { + CrashReportingSettings.recordChoice(enabled: true) + #expect(CrashReportingSettings.isEnabled) + #expect(CrashReportingSettings.hasShownPrompt) + #expect(!CrashReportingSettings.needsPrompt) + } + } + + @Test func recordChoice_disabled_setsValues() { + withCleanDefaults { + CrashReportingSettings.recordChoice(enabled: false) + #expect(!CrashReportingSettings.isEnabled) + #expect(CrashReportingSettings.hasShownPrompt) + #expect(!CrashReportingSettings.needsPrompt) + } + } + + @Test func isEnabled_canBeToggled() { + withCleanDefaults { + CrashReportingSettings.isEnabled = true + #expect(CrashReportingSettings.isEnabled) + CrashReportingSettings.isEnabled = false + #expect(!CrashReportingSettings.isEnabled) + } + } + + @Test func keys_haveExpectedValues() { + #expect(CrashReportingSettings.enabledKey == "crashReporting.enabled") + #expect(CrashReportingSettings.promptShownKey == "crashReporting.promptShown") + } +} + +// MARK: - CrashReport Model Tests + +struct CrashReportModelTests { + + @Test func init_setsDefaults() { + let report = CrashReport() + #expect(report.signal == nil) + #expect(report.exceptionType == nil) + #expect(report.terminationReason == nil) + #expect(report.callStackFrames.isEmpty) + #expect(report.openTabCount == nil) + } + + @Test func init_withValues() { + let report = CrashReport( + signal: "SIGSEGV", + exceptionType: "EXC_BAD_ACCESS", + terminationReason: "Namespace SIGNAL, Code 11", + callStackFrames: ["frame0", "frame1"], + openTabCount: 5 + ) + #expect(report.signal == "SIGSEGV") + #expect(report.exceptionType == "EXC_BAD_ACCESS") + #expect(report.terminationReason == "Namespace SIGNAL, Code 11") + #expect(report.callStackFrames.count == 2) + #expect(report.openTabCount == 5) + } + + @Test func init_capturesAppVersion() { + let report = CrashReport() + // In test target, Bundle.main may not have these keys, but should not crash + #expect(report.appVersion is String) + #expect(report.buildNumber is String) + #expect(!report.osVersion.isEmpty) + } + + @Test func testInit_explicitValues() { + let id = UUID() + let date = Date(timeIntervalSince1970: 1_000_000) + let report = CrashReport( + id: id, + timestamp: date, + appVersion: "1.0.0", + buildNumber: "42", + osVersion: "26.0", + signal: "SIGABRT", + exceptionType: nil, + terminationReason: nil, + callStackFrames: ["a", "b", "c"], + openTabCount: 3 + ) + #expect(report.id == id) + #expect(report.timestamp == date) + #expect(report.appVersion == "1.0.0") + #expect(report.buildNumber == "42") + #expect(report.osVersion == "26.0") + #expect(report.signal == "SIGABRT") + #expect(report.callStackFrames == ["a", "b", "c"]) + #expect(report.openTabCount == 3) + } + + @Test func codable_roundTrip() throws { + let original = CrashReport( + id: UUID(), + timestamp: Date(), + appVersion: "2.0.0", + buildNumber: "100", + osVersion: "26.1", + signal: "SIGSEGV", + exceptionType: "EXC_BAD_ACCESS", + terminationReason: "some reason", + callStackFrames: ["frame0", "frame1"], + openTabCount: 10 + ) + let data = try JSONEncoder().encode(original) + let decoded = try JSONDecoder().decode(CrashReport.self, from: data) + #expect(original == decoded) + } + + @Test func equatable_sameValues_areEqual() { + let id = UUID() + let date = Date() + let a = CrashReport(id: id, timestamp: date, appVersion: "1", buildNumber: "1", + osVersion: "26", signal: nil, exceptionType: nil, + terminationReason: nil, callStackFrames: [], openTabCount: nil) + let b = CrashReport(id: id, timestamp: date, appVersion: "1", buildNumber: "1", + osVersion: "26", signal: nil, exceptionType: nil, + terminationReason: nil, callStackFrames: [], openTabCount: nil) + #expect(a == b) + } + + @Test func equatable_differentIDs_areNotEqual() { + let date = Date() + let a = CrashReport(id: UUID(), timestamp: date, appVersion: "1", buildNumber: "1", + osVersion: "26", signal: nil, exceptionType: nil, + terminationReason: nil, callStackFrames: [], openTabCount: nil) + let b = CrashReport(id: UUID(), timestamp: date, appVersion: "1", buildNumber: "1", + osVersion: "26", signal: nil, exceptionType: nil, + terminationReason: nil, callStackFrames: [], openTabCount: nil) + #expect(a != b) + } +} + +// MARK: - ParseCallStack Tests + +struct ParseCallStackTests { + + @Test func parseCallStack_basicFrames() { + let raw = """ + 0 Pine 0x00000001000a1234 someFunction + 42 + 1 Pine 0x00000001000a5678 anotherFunction + 10 + """ + let frames = CrashReport.parseCallStack(raw) + #expect(frames.count == 2) + #expect(frames[0].contains("someFunction")) + #expect(frames[1].contains("anotherFunction")) + } + + @Test func parseCallStack_emptyString() { + let frames = CrashReport.parseCallStack("") + #expect(frames.isEmpty) + } + + @Test func parseCallStack_singleFrame() { + let raw = "0 Pine 0x1234 main + 0" + let frames = CrashReport.parseCallStack(raw) + #expect(frames.count == 1) + #expect(frames[0].contains("main")) + } + + @Test func parseCallStack_skipsEmptyLines() { + let raw = """ + frame1 + + frame2 + + """ + let frames = CrashReport.parseCallStack(raw) + #expect(frames.count == 2) + #expect(frames[0] == "frame1") + #expect(frames[1] == "frame2") + } + + @Test func parseCallStack_trimsWhitespace() { + let raw = " frame with spaces " + let frames = CrashReport.parseCallStack(raw) + #expect(frames.count == 1) + #expect(frames[0] == "frame with spaces") + } + + @Test func parseCallStack_multipleNewlineFormats() { + let raw = "frame1\nframe2\nframe3" + let frames = CrashReport.parseCallStack(raw) + #expect(frames.count == 3) + } + + @Test func parseCallStack_onlyWhitespaceLines() { + let raw = " \n \n " + let frames = CrashReport.parseCallStack(raw) + #expect(frames.isEmpty) + } +} + +// MARK: - CrashReportStore Tests + +struct CrashReportStoreTests { + + private func makeTempStore() -> CrashReportStore { + let tmpDir = FileManager.default.temporaryDirectory + .appendingPathComponent("PineTests-CrashStore-\(UUID().uuidString)") + return CrashReportStore(storageDirectory: tmpDir) + } + + private func cleanup(_ store: CrashReportStore) { + try? FileManager.default.removeItem(at: store.storageDirectory) + } + + @Test func save_andLoadAll() { + let store = makeTempStore() + defer { cleanup(store) } + + let report = CrashReport(signal: "SIGABRT") + store.save(report) + + let loaded = store.loadAll() + #expect(loaded.count == 1) + #expect(loaded[0].id == report.id) + #expect(loaded[0].signal == "SIGABRT") + } + + @Test func count_returnsCorrectValue() { + let store = makeTempStore() + defer { cleanup(store) } + + #expect(store.loadAll().isEmpty) + store.save(CrashReport(signal: "SIGSEGV")) + #expect(store.count == 1) + store.save(CrashReport(signal: "SIGBUS")) + #expect(store.count == 2) + } + + @Test func remove_byID() { + let store = makeTempStore() + defer { cleanup(store) } + + let report1 = CrashReport(signal: "A") + let report2 = CrashReport(signal: "B") + store.save(report1) + store.save(report2) + + store.remove(id: report1.id) + + let remaining = store.loadAll() + #expect(remaining.count == 1) + #expect(remaining[0].id == report2.id) + } + + @Test func removeAll_clearsStore() { + let store = makeTempStore() + defer { cleanup(store) } + + store.save(CrashReport(signal: "A")) + store.save(CrashReport(signal: "B")) + store.save(CrashReport(signal: "C")) + + store.removeAll() + #expect(store.loadAll().isEmpty) + } + + @Test func loadAll_sortedByTimestamp_newestFirst() { + let store = makeTempStore() + defer { cleanup(store) } + + let old = CrashReport( + id: UUID(), timestamp: Date(timeIntervalSince1970: 1_000_000), + appVersion: "1", buildNumber: "1", osVersion: "26", + signal: "old", exceptionType: nil, terminationReason: nil, + callStackFrames: [], openTabCount: nil + ) + let recent = CrashReport( + id: UUID(), timestamp: Date(timeIntervalSince1970: 2_000_000), + appVersion: "1", buildNumber: "1", osVersion: "26", + signal: "recent", exceptionType: nil, terminationReason: nil, + callStackFrames: [], openTabCount: nil + ) + store.save(old) + store.save(recent) + + let loaded = store.loadAll() + #expect(loaded.count == 2) + #expect(loaded[0].signal == "recent") + #expect(loaded[1].signal == "old") + } + + @Test func emptyStore_loadsEmpty() { + let store = makeTempStore() + defer { cleanup(store) } + + #expect(store.loadAll().isEmpty) + } + + @Test func maxReports_constant() { + #expect(CrashReportStore.maxReports == 50) + } + + @Test func fileExtension_constant() { + #expect(CrashReportStore.fileExtension == "crashreport") + } + + @Test func remove_nonexistentID_doesNotCrash() { + let store = makeTempStore() + defer { cleanup(store) } + + store.remove(id: UUID()) // Should not throw or crash + #expect(store.loadAll().isEmpty) + } + + @Test func pruning_removesOldReportsOverLimit() { + let store = makeTempStore() + defer { cleanup(store) } + + // Save more than maxReports + for i in 0..<(CrashReportStore.maxReports + 5) { + let report = CrashReport( + id: UUID(), + timestamp: Date(timeIntervalSinceNow: Double(i)), + appVersion: "1", buildNumber: "1", osVersion: "26", + signal: "SIG\(i)", exceptionType: nil, terminationReason: nil, + callStackFrames: [], openTabCount: nil + ) + store.save(report) + } + + #expect(store.count <= CrashReportStore.maxReports) + } +} + +// MARK: - MenuIcons Tests + +struct CrashReportingMenuIconTests { + @Test func crashReporting_iconExists() { + #expect( + NSImage(systemSymbolName: MenuIcons.crashReporting, accessibilityDescription: nil) != nil, + "SF Symbol '\(MenuIcons.crashReporting)' does not exist" + ) + } +} From 4af84a360be785db054f4b9a0235efec2795c76a Mon Sep 17 00:00:00 2001 From: Fedor Batonogov Date: Sun, 29 Mar 2026 18:12:46 +0300 Subject: [PATCH 2/7] fix: async-signal-safe handler with pre-computed C string, proper sigaction, JSON callstack parsing --- Pine/CrashReportingManager.swift | 154 ++++++++++++++++++++-------- PineTests/CrashReportingTests.swift | 135 ++++++++++++++++++++++++ 2 files changed, 244 insertions(+), 45 deletions(-) diff --git a/Pine/CrashReportingManager.swift b/Pine/CrashReportingManager.swift index e0f9cd2..a3c4b56 100644 --- a/Pine/CrashReportingManager.swift +++ b/Pine/CrashReportingManager.swift @@ -10,6 +10,14 @@ import Foundation import MetricKit import os +// MARK: - Global async-signal-safe storage for crash marker path + +/// Pre-computed C string for the crash marker path. +/// Set once during `installSignalHandlers()`, read-only in the signal handler. +/// Using nonisolated(unsafe) because signal handlers are inherently unsafe +/// and this is written once before any signal can fire. +nonisolated(unsafe) private var crashMarkerCString: UnsafeMutablePointer? + /// Coordinates crash reporting using MetricKit (MXCrashDiagnostic) as the /// primary source and POSIX signal handlers as a minimal async-signal-safe fallback. /// @@ -58,6 +66,40 @@ final class CrashReportingManager: NSObject, MXMetricManagerSubscriber { logger.info("Crash reporting started (MetricKit + signal handler fallback)") } + /// Extracts call stack frame strings from MetricKit JSON representation. + /// Parses the `callStackTree` → `callStacks` → `callStackRootFrames` hierarchy, + /// flattening nested frames into human-readable strings. + static func extractCallStackFrames(from jsonData: Data) -> [String] { + guard let json = try? JSONSerialization.jsonObject(with: jsonData) as? [String: Any], + let callStackTree = json["callStackTree"] as? [String: Any], + let callStacks = callStackTree["callStacks"] as? [[String: Any]] else { + return [] + } + + var frames: [String] = [] + for stack in callStacks { + guard let rootFrames = stack["callStackRootFrames"] as? [[String: Any]] else { continue } + Self.flattenFrames(rootFrames, into: &frames) + } + return frames + } + + /// Recursively flattens nested call stack frames into a flat list of strings. + private static func flattenFrames(_ frameList: [[String: Any]], into result: inout [String]) { + for frame in frameList { + let address = frame["address"] as? UInt64 ?? 0 + let binaryName = frame["binaryName"] as? String ?? "?" + let offsetIntoBinaryTextSegment = frame["offsetIntoBinaryTextSegment"] as? UInt64 ?? 0 + result.append(String( + format: "%@ 0x%llx +%llu", + binaryName, address, offsetIntoBinaryTextSegment + )) + if let subFrames = frame["subFrames"] as? [[String: Any]] { + flattenFrames(subFrames, into: &result) + } + } + } + /// Stops crash reporting (unsubscribes from MetricKit). func stop() { MXMetricManager.shared.remove(self) @@ -81,15 +123,7 @@ final class CrashReportingManager: NSObject, MXMetricManagerSubscriber { /// Processes a single MetricKit crash diagnostic into a CrashReport. private func processCrashDiagnostic(_ diagnostic: MXCrashDiagnostic) { - let callStack: [String] - let jsonData = diagnostic.jsonRepresentation() - if let json = try? JSONSerialization.jsonObject(with: jsonData) as? [String: Any], - let stacks = json["callStackTree"] as? [String: Any], - let callStackString = stacks.description.data(using: .utf8) { - callStack = CrashReport.parseCallStack(String(data: callStackString, encoding: .utf8) ?? "") - } else { - callStack = [] - } + let callStack: [String] = Self.extractCallStackFrames(from: diagnostic.jsonRepresentation()) let report = CrashReport( signal: diagnostic.signal?.description, @@ -105,16 +139,19 @@ final class CrashReportingManager: NSObject, MXMetricManagerSubscriber { // MARK: - Signal Handler Fallback /// Installs POSIX signal handlers for common crash signals. - /// The handler is strictly async-signal-safe: only POSIX write() and _exit(). + /// Pre-computes the crash marker path as a C string so the handler + /// never touches Swift/Foundation/malloc. private func installSignalHandlers() { + // Pre-compute the path as a C string BEFORE installing handlers. + // This is the ONLY place we allocate — the handler only reads this pointer. + let path = Self.crashMarkerPath + let cString = strdup(path) + crashMarkerCString = cString + let signals: [Int32] = [SIGSEGV, SIGABRT, SIGBUS, SIGFPE, SIGILL, SIGTRAP] for sig in signals { - var action = sigaction() - action.__sigaction_u.__sa_handler = signalHandler - sigemptyset(&action.sa_mask) - action.sa_flags = 0 - sigaction(sig, &action, nil) + signal(sig, signalHandler) } } @@ -155,44 +192,71 @@ final class CrashReportingManager: NSObject, MXMetricManagerSubscriber { /// Strictly async-signal-safe signal handler. /// Uses ONLY POSIX APIs: open(), write(), close(), _exit(). /// No Swift runtime, no Foundation, no malloc, no ObjC messaging. -private func signalHandler(_ signal: Int32) { - // Get the crash marker path — this is a compile-time known location - // We cannot use Swift String here, so we use a pre-computed C string - let path = CrashReportingManager.crashMarkerPath - - path.withCString { cPath in - // Open file for writing (create if needed, truncate) - let fd = open(cPath, O_WRONLY | O_CREAT | O_TRUNC, 0o644) - guard fd >= 0 else { - _exit(signal) - } +/// +/// The crash marker path is pre-computed in `installSignalHandlers()` and stored +/// in the global `crashMarkerCString`. This function only reads that pointer. +private func signalHandler(_ sig: Int32) { + // Read pre-computed C string path — no allocation, no Swift runtime + guard let cPath = crashMarkerCString else { + _exit(sig) + } - // Write the signal number as ASCII digits - var sigNum = signal - var digits: [UInt8] = [] + // Open file for writing (create if needed, truncate) + let fd = open(cPath, O_WRONLY | O_CREAT | O_TRUNC, 0o644) + guard fd >= 0 else { + _exit(sig) + } - if sigNum == 0 { - digits.append(0x30) // '0' - } else { - while sigNum > 0 { - digits.append(UInt8(0x30 + sigNum % 10)) - sigNum /= 10 + // Write the signal number as ASCII digits using a stack buffer (no malloc). + // Max digits for Int32: 10 digits + newline = 11 bytes. Using a 4-byte tuple + // is sufficient since POSIX signals are small numbers (1-31). + var buf = (UInt8(0), UInt8(0), UInt8(0), UInt8(0)) + var sigNum = sig + var len = 0 + + if sigNum == 0 { + buf.0 = 0x30 // '0' + len = 1 + } else { + // Signals are 1-31, so at most 2 digits — reverse into buf + var tmp = (UInt8(0), UInt8(0)) + var tmpLen = 0 + while sigNum > 0, tmpLen < 2 { + if tmpLen == 0 { + tmp.0 = UInt8(0x30 + sigNum % 10) + } else { + tmp.1 = UInt8(0x30 + sigNum % 10) } - digits.reverse() + tmpLen += 1 + sigNum /= 10 } - - digits.withUnsafeBufferPointer { buf in - _ = write(fd, buf.baseAddress, buf.count) + if tmpLen == 1 { + buf.0 = tmp.0 + } else { + buf.0 = tmp.1 + buf.1 = tmp.0 } + len = tmpLen + } - // Write newline - var newline: UInt8 = 0x0A - _ = write(fd, &newline, 1) + // Append newline + if len == 0 { + buf.0 = 0x0A + } else if len == 1 { + buf.1 = 0x0A + } else { + buf.2 = 0x0A + } + len += 1 - close(fd) + // Write using POSIX write() — async-signal-safe + withUnsafePointer(to: &buf) { ptr in + _ = write(fd, ptr, len) } + close(fd) + // Re-raise with default handler to get proper exit code - Darwin.signal(signal, SIG_DFL) - raise(signal) + Darwin.signal(sig, SIG_DFL) + raise(sig) } diff --git a/PineTests/CrashReportingTests.swift b/PineTests/CrashReportingTests.swift index b178999..c176ee0 100644 --- a/PineTests/CrashReportingTests.swift +++ b/PineTests/CrashReportingTests.swift @@ -386,6 +386,141 @@ struct CrashReportStoreTests { } } +// MARK: - extractCallStackFrames Tests + +struct ExtractCallStackFramesTests { + + @Test func emptyData_returnsEmpty() { + let frames = CrashReportingManager.extractCallStackFrames(from: Data()) + #expect(frames.isEmpty) + } + + @Test func invalidJSON_returnsEmpty() { + let data = Data("not json".utf8) + let frames = CrashReportingManager.extractCallStackFrames(from: data) + #expect(frames.isEmpty) + } + + @Test func missingCallStackTree_returnsEmpty() throws { + let json: [String: Any] = ["other": "data"] + let data = try JSONSerialization.data(withJSONObject: json) + let frames = CrashReportingManager.extractCallStackFrames(from: data) + #expect(frames.isEmpty) + } + + @Test func missingCallStacks_returnsEmpty() throws { + let json: [String: Any] = ["callStackTree": ["other": "data"]] + let data = try JSONSerialization.data(withJSONObject: json) + let frames = CrashReportingManager.extractCallStackFrames(from: data) + #expect(frames.isEmpty) + } + + @Test func singleFrame_parsesCorrectly() throws { + let json: [String: Any] = [ + "callStackTree": [ + "callStacks": [ + [ + "callStackRootFrames": [ + [ + "binaryName": "Pine", + "address": 4_294_967_296, + "offsetIntoBinaryTextSegment": 1234 + ] + ] + ] + ] + ] + ] + let data = try JSONSerialization.data(withJSONObject: json) + let frames = CrashReportingManager.extractCallStackFrames(from: data) + #expect(frames.count == 1) + #expect(frames[0].contains("Pine")) + } + + @Test func nestedSubFrames_flattensAll() throws { + let json: [String: Any] = [ + "callStackTree": [ + "callStacks": [ + [ + "callStackRootFrames": [ + [ + "binaryName": "Pine", + "address": 100, + "offsetIntoBinaryTextSegment": 10, + "subFrames": [ + [ + "binaryName": "libsystem", + "address": 200, + "offsetIntoBinaryTextSegment": 20 + ] + ] + ] + ] + ] + ] + ] + ] + let data = try JSONSerialization.data(withJSONObject: json) + let frames = CrashReportingManager.extractCallStackFrames(from: data) + #expect(frames.count == 2) + #expect(frames[0].contains("Pine")) + #expect(frames[1].contains("libsystem")) + } + + @Test func multipleCallStacks_parsesAll() throws { + let json: [String: Any] = [ + "callStackTree": [ + "callStacks": [ + [ + "callStackRootFrames": [ + ["binaryName": "Pine", "address": 100, "offsetIntoBinaryTextSegment": 10] + ] + ], + [ + "callStackRootFrames": [ + ["binaryName": "AppKit", "address": 200, "offsetIntoBinaryTextSegment": 20] + ] + ] + ] + ] + ] + let data = try JSONSerialization.data(withJSONObject: json) + let frames = CrashReportingManager.extractCallStackFrames(from: data) + #expect(frames.count == 2) + #expect(frames[0].contains("Pine")) + #expect(frames[1].contains("AppKit")) + } + + @Test func missingBinaryName_usesFallback() throws { + let json: [String: Any] = [ + "callStackTree": [ + "callStacks": [ + [ + "callStackRootFrames": [ + ["address": 100, "offsetIntoBinaryTextSegment": 10] + ] + ] + ] + ] + ] + let data = try JSONSerialization.data(withJSONObject: json) + let frames = CrashReportingManager.extractCallStackFrames(from: data) + #expect(frames.count == 1) + #expect(frames[0].contains("?")) + } + + @Test func emptyCallStacks_returnsEmpty() throws { + let json: [String: Any] = [ + "callStackTree": [ + "callStacks": [] as [[String: Any]] + ] + ] + let data = try JSONSerialization.data(withJSONObject: json) + let frames = CrashReportingManager.extractCallStackFrames(from: data) + #expect(frames.isEmpty) + } +} + // MARK: - MenuIcons Tests struct CrashReportingMenuIconTests { From bd7390e16a936bfdd33a5b349499932e7b8ca954 Mon Sep 17 00:00:00 2001 From: Fedor Batonogov Date: Tue, 31 Mar 2026 06:40:43 +0300 Subject: [PATCH 3/7] =?UTF-8?q?fix:=20address=20crash=20reporting=20review?= =?UTF-8?q?=20=E2=80=94=20thread=20safety,=20memory=20leak,=20UI=20text,?= =?UTF-8?q?=20race=20condition?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit - CrashReportStore: add serial DispatchQueue for all disk I/O (thread safety) - CrashReportStore: add revealInFinder() and copyAllToClipboard() export methods - CrashReportingManager.stop(): restore default signal handlers + free strdup'd C string to prevent memory leak on toggle cycles - CrashReportingManager.installSignalHandlers(): free previous strdup on re-install - ContentView+Helpers: set hasShownPrompt immediately before async delay to prevent opt-in dialog showing in multiple windows simultaneously - CrashReportingOptInView: use Strings constants instead of inline String(localized:) - Localizable.xcstrings: replace misleading "send" language with "collect locally" / "stored locally" across all 9 locales (en, de, es, fr, ja, ko, pt-BR, ru, zh-Hans) - Strings.swift: add crashReportingOptInEnable/Disable constants - Tests: add thread safety, export, race condition, stop(), and strings tests --- Pine/ContentView+Helpers.swift | 3 + Pine/CrashReportStore.swift | 90 +++++++-- Pine/CrashReportingManager.swift | 21 ++- Pine/CrashReportingOptInView.swift | 4 +- Pine/Localizable.xcstrings | 38 ++-- Pine/Strings.swift | 6 + PineTests/CrashReportingTests.swift | 282 ++++++++++++++++++++++++++++ 7 files changed, 410 insertions(+), 34 deletions(-) diff --git a/Pine/ContentView+Helpers.swift b/Pine/ContentView+Helpers.swift index c6af6e0..5a8fdaf 100644 --- a/Pine/ContentView+Helpers.swift +++ b/Pine/ContentView+Helpers.swift @@ -509,8 +509,11 @@ extension ContentView { extension ContentView { /// Shows the crash reporting opt-in dialog on first launch. + /// Sets `hasShownPrompt` immediately to prevent duplicate dialogs across multiple windows. func showCrashReportingOptInIfNeeded() { guard CrashReportingSettings.needsPrompt else { return } + // Mark as shown BEFORE the async delay to prevent race with other windows + CrashReportingSettings.hasShownPrompt = true // Slight delay to avoid showing during initial window setup DispatchQueue.main.asyncAfter(deadline: .now() + 1.0) { showCrashReportingOptIn = true diff --git a/Pine/CrashReportStore.swift b/Pine/CrashReportStore.swift index 811e0b2..d6d9dda 100644 --- a/Pine/CrashReportStore.swift +++ b/Pine/CrashReportStore.swift @@ -5,11 +5,15 @@ // Persists crash reports to disk for display on next launch. // +import AppKit import Foundation import os /// Persists crash reports as JSON files in the Application Support directory. /// Reports are stored individually for atomic read/write and easy cleanup. +/// +/// Thread-safe: all file I/O is serialized on an internal serial queue. +/// Safe to call from MetricKit callbacks (arbitrary threads) and from the main thread. final class CrashReportStore { /// Shared singleton instance. static let shared = CrashReportStore() @@ -19,6 +23,9 @@ final class CrashReportStore { private let logger = Logger(subsystem: "com.pine.editor", category: "CrashReportStore") + /// Serial queue that serializes all disk I/O for thread safety. + private let queue = DispatchQueue(label: "com.pine.crash-report-store") + /// Maximum number of reports to keep on disk. static let maxReports = 50 @@ -41,20 +48,82 @@ final class CrashReportStore { /// Saves a crash report to disk. func save(_ report: CrashReport) { + queue.sync { + _save(report) + } + } + + /// Loads all stored crash reports, sorted by timestamp (newest first). + func loadAll() -> [CrashReport] { + queue.sync { + _loadAll() + } + } + + /// Removes a specific crash report by ID. + func remove(id: UUID) { + queue.sync { + _remove(id: id) + } + } + + /// Removes all stored crash reports. + func removeAll() { + queue.sync { + _removeAll() + } + } + + /// Returns the number of stored reports. + var count: Int { + queue.sync { + _count() + } + } + + /// Reveals the crash reports directory in Finder. + func revealInFinder() { + NSWorkspace.shared.selectFile(nil, inFileViewerRootedAtPath: storageDirectory.path) + } + + /// Copies all crash reports as JSON to the clipboard. + /// Returns the number of reports copied. + @discardableResult + func copyAllToClipboard() -> Int { + let reports = loadAll() + guard !reports.isEmpty else { return 0 } + + let encoder = JSONEncoder() + encoder.outputFormatting = [.prettyPrinted, .sortedKeys] + encoder.dateEncodingStrategy = .iso8601 + + guard let data = try? encoder.encode(reports), + let jsonString = String(data: data, encoding: .utf8) else { + return 0 + } + + let pasteboard = NSPasteboard.general + pasteboard.clearContents() + pasteboard.setString(jsonString, forType: .string) + return reports.count + } + + // MARK: - Private (must be called on self.queue) + + private func _save(_ report: CrashReport) { let fileName = "\(report.id.uuidString).\(Self.fileExtension)" let fileURL = storageDirectory.appendingPathComponent(fileName) do { let data = try JSONEncoder().encode(report) try data.write(to: fileURL, options: .atomic) - pruneOldReports() + _pruneOldReports() } catch { logger.error("Failed to save crash report: \(error.localizedDescription)") } } - /// Loads all stored crash reports, sorted by timestamp (newest first). - func loadAll() -> [CrashReport] { + private func _loadAll() -> [CrashReport] { let fm = FileManager.default guard let files = try? fm.contentsOfDirectory(at: storageDirectory, includingPropertiesForKeys: nil) else { return [] @@ -74,15 +143,13 @@ final class CrashReportStore { return reports } - /// Removes a specific crash report by ID. - func remove(id: UUID) { + private func _remove(id: UUID) { let fileName = "\(id.uuidString).\(Self.fileExtension)" let fileURL = storageDirectory.appendingPathComponent(fileName) try? FileManager.default.removeItem(at: fileURL) } - /// Removes all stored crash reports. - func removeAll() { + private func _removeAll() { let fm = FileManager.default guard let files = try? fm.contentsOfDirectory(at: storageDirectory, includingPropertiesForKeys: nil) else { return @@ -92,8 +159,7 @@ final class CrashReportStore { } } - /// Returns the number of stored reports. - var count: Int { + private func _count() -> Int { let fm = FileManager.default guard let files = try? fm.contentsOfDirectory(at: storageDirectory, includingPropertiesForKeys: nil) else { return 0 @@ -102,13 +168,13 @@ final class CrashReportStore { } /// Prunes old reports if count exceeds the maximum. - private func pruneOldReports() { - let reports = loadAll() + private func _pruneOldReports() { + let reports = _loadAll() guard reports.count > Self.maxReports else { return } let toRemove = reports.suffix(from: Self.maxReports) for report in toRemove { - remove(id: report.id) + _remove(id: report.id) } } } diff --git a/Pine/CrashReportingManager.swift b/Pine/CrashReportingManager.swift index a3c4b56..97631fb 100644 --- a/Pine/CrashReportingManager.swift +++ b/Pine/CrashReportingManager.swift @@ -100,9 +100,23 @@ final class CrashReportingManager: NSObject, MXMetricManagerSubscriber { } } - /// Stops crash reporting (unsubscribes from MetricKit). + /// Stops crash reporting: unsubscribes from MetricKit and restores default signal handlers. + /// Frees the pre-computed crash marker C string to avoid memory leaks on toggle cycles. func stop() { MXMetricManager.shared.remove(self) + + // Restore default signal handlers + let signals: [Int32] = [SIGSEGV, SIGABRT, SIGBUS, SIGFPE, SIGILL, SIGTRAP] + for sig in signals { + signal(sig, SIG_DFL) + } + + // Free the strdup'd path to prevent memory leak on repeated start/stop cycles + if let ptr = crashMarkerCString { + free(ptr) + crashMarkerCString = nil + } + logger.info("Crash reporting stopped") } @@ -144,6 +158,11 @@ final class CrashReportingManager: NSObject, MXMetricManagerSubscriber { private func installSignalHandlers() { // Pre-compute the path as a C string BEFORE installing handlers. // This is the ONLY place we allocate — the handler only reads this pointer. + // Free any previously allocated string to prevent leaks on repeated start() calls. + if let previous = crashMarkerCString { + free(previous) + crashMarkerCString = nil + } let path = Self.crashMarkerPath let cString = strdup(path) crashMarkerCString = cString diff --git a/Pine/CrashReportingOptInView.swift b/Pine/CrashReportingOptInView.swift index a90086f..7003723 100644 --- a/Pine/CrashReportingOptInView.swift +++ b/Pine/CrashReportingOptInView.swift @@ -35,14 +35,14 @@ struct CrashReportingOptInView: View { .fixedSize(horizontal: false, vertical: true) HStack(spacing: 12) { - Button(String(localized: "crashReporting.optIn.disable")) { + Button(Strings.crashReportingOptInDisable) { CrashReportingSettings.recordChoice(enabled: false) onChoice(false) isPresented = false } .keyboardShortcut(.cancelAction) - Button(String(localized: "crashReporting.optIn.enable")) { + Button(Strings.crashReportingOptInEnable) { CrashReportingSettings.recordChoice(enabled: true) onChoice(true) isPresented = false diff --git a/Pine/Localizable.xcstrings b/Pine/Localizable.xcstrings index 441cdeb..9947592 100644 --- a/Pine/Localizable.xcstrings +++ b/Pine/Localizable.xcstrings @@ -1564,61 +1564,61 @@ } }, "crashReporting.optIn.message" : { - "comment" : "Explanatory message in the crash reporting opt-in dialog.", + "comment" : "Explanatory message in the crash reporting opt-in dialog. Data is stored locally only — not sent anywhere.", "extractionState" : "manual", "localizations" : { "de" : { "stringUnit" : { "state" : "translated", - "value" : "Pine kann anonyme Absturzberichte senden, um die App-Stabilität zu verbessern. Es werden nur technische Daten gesendet — keine Dateiinhalte oder Pfade." + "value" : "Pine kann anonyme Absturzberichte lokal sammeln, um die App-Stabilität zu verbessern. Nur technische Daten werden erfasst — keine Dateiinhalte oder Pfade." } }, "en" : { "stringUnit" : { "state" : "translated", - "value" : "Pine can send anonymous crash reports to help improve app stability. Only technical data is sent — no file contents or paths." + "value" : "Pine can collect anonymous crash reports locally to help improve app stability. Only technical data is stored — no file contents or paths." } }, "es" : { "stringUnit" : { "state" : "translated", - "value" : "Pine puede enviar informes de error anónimos para mejorar la estabilidad. Solo se envían datos técnicos, sin contenido ni rutas de archivos." + "value" : "Pine puede recopilar informes de error anónimos localmente para mejorar la estabilidad. Solo se almacenan datos técnicos, sin contenido ni rutas de archivos." } }, "fr" : { "stringUnit" : { "state" : "translated", - "value" : "Pine peut envoyer des rapports de plantage anonymes pour améliorer la stabilité. Seules des données techniques sont envoyées — aucun contenu ni chemin de fichier." + "value" : "Pine peut collecter localement des rapports de plantage anonymes pour améliorer la stabilité. Seules des données techniques sont stockées — aucun contenu ni chemin de fichier." } }, "ja" : { "stringUnit" : { "state" : "translated", - "value" : "Pineはアプリの安定性向上のため匿名のクラッシュレポートを送信できます。技術データのみが送信され、ファイルの内容やパスは含まれません。" + "value" : "Pineはアプリの安定性向上のため匿名のクラッシュレポートをローカルに収集できます。技術データのみが保存され、ファイルの内容やパスは含まれません。" } }, "ko" : { "stringUnit" : { "state" : "translated", - "value" : "Pine은 앱 안정성 향상을 위해 익명 충돌 보고서를 보낼 수 있습니다. 기술 데이터만 전송되며 파일 내용이나 경로는 포함되지 않습니다." + "value" : "Pine은 앱 안정성 향상을 위해 익명 충돌 보고서를 로컬에 수집할 수 있습니다. 기술 데이터만 저장되며 파일 내용이나 경로는 포함되지 않습니다." } }, "pt-BR" : { "stringUnit" : { "state" : "translated", - "value" : "Pine pode enviar relatórios de falhas anônimos para melhorar a estabilidade. Apenas dados técnicos são enviados — sem conteúdo ou caminhos de arquivos." + "value" : "Pine pode coletar relatórios de falhas anônimos localmente para melhorar a estabilidade. Apenas dados técnicos são armazenados — sem conteúdo ou caminhos de arquivos." } }, "ru" : { "stringUnit" : { "state" : "translated", - "value" : "Pine может отправлять анонимные отчёты об ошибках для улучшения стабильности. Передаются только технические данные — без содержимого файлов и путей." + "value" : "Pine может локально собирать анонимные отчёты об ошибках для улучшения стабильности. Сохраняются только технические данные — без содержимого файлов и путей." } }, "zh-Hans" : { "stringUnit" : { "state" : "translated", - "value" : "Pine 可以发送匿名崩溃报告以帮助改进应用稳定性。仅发送技术数据，不包含文件内容或路径。" + "value" : "Pine 可以在本地收集匿名崩溃报告以帮助改进应用稳定性。仅存储技术数据，不包含文件内容或路径。" } } } @@ -1630,55 +1630,55 @@ "de" : { "stringUnit" : { "state" : "translated", - "value" : "Gesammelt: Stack-Trace, OS-Version, App-Version, Anzahl offener Tabs. Sie können dies jederzeit im Menü Datei ändern." + "value" : "Lokal gespeichert: Stack-Trace, OS-Version, App-Version, Anzahl offener Tabs. Sie können Berichte über das Menü Datei exportieren oder diese Einstellung jederzeit ändern." } }, "en" : { "stringUnit" : { "state" : "translated", - "value" : "Collected: stack trace, OS version, app version, open tab count. You can change this anytime in the File menu." + "value" : "Stored locally: stack trace, OS version, app version, open tab count. You can export reports from the File menu or change this setting anytime." } }, "es" : { "stringUnit" : { "state" : "translated", - "value" : "Recopilado: traza de pila, versión del SO, versión de la app, cantidad de pestañas abiertas. Puedes cambiarlo en cualquier momento en el menú Archivo." + "value" : "Almacenado localmente: traza de pila, versión del SO, versión de la app, cantidad de pestañas abiertas. Puedes exportar informes desde el menú Archivo o cambiar esta opción en cualquier momento." } }, "fr" : { "stringUnit" : { "state" : "translated", - "value" : "Collecté : trace de pile, version du SE, version de l'app, nombre d'onglets ouverts. Vous pouvez modifier ce choix à tout moment dans le menu Fichier." + "value" : "Stocké localement : trace de pile, version du SE, version de l'app, nombre d'onglets ouverts. Vous pouvez exporter les rapports depuis le menu Fichier ou modifier ce choix à tout moment." } }, "ja" : { "stringUnit" : { "state" : "translated", - "value" : "収集データ：スタックトレース、OS バージョン、アプリバージョン、開いているタブ数。「ファイル」メニューからいつでも変更できます。" + "value" : "ローカルに保存：スタックトレース、OS バージョン、アプリバージョン、開いているタブ数。「ファイル」メニューからレポートをエクスポートしたり、設定をいつでも変更できます。" } }, "ko" : { "stringUnit" : { "state" : "translated", - "value" : "수집 항목: 스택 추적, OS 버전, 앱 버전, 열린 탭 수. 파일 메뉴에서 언제든지 변경할 수 있습니다." + "value" : "로컬 저장 항목: 스택 추적, OS 버전, 앱 버전, 열린 탭 수. 파일 메뉴에서 보고서를 내보내거나 언제든지 설정을 변경할 수 있습니다." } }, "pt-BR" : { "stringUnit" : { "state" : "translated", - "value" : "Coletado: rastreamento de pilha, versão do SO, versão do app, quantidade de abas abertas. Você pode alterar a qualquer momento no menu Arquivo." + "value" : "Armazenado localmente: rastreamento de pilha, versão do SO, versão do app, quantidade de abas abertas. Você pode exportar relatórios pelo menu Arquivo ou alterar esta configuração a qualquer momento." } }, "ru" : { "stringUnit" : { "state" : "translated", - "value" : "Собирается: стек вызовов, версия ОС, версия приложения, количество открытых вкладок. Вы можете изменить это в любое время в меню «Файл»." + "value" : "Хранится локально: стек вызовов, версия ОС, версия приложения, количество открытых вкладок. Вы можете экспортировать отчёты из меню «Файл» или изменить настройку в любое время." } }, "zh-Hans" : { "stringUnit" : { "state" : "translated", - "value" : "收集内容：堆栈跟踪、操作系统版本、应用版本、打开的标签页数量。您可以随时在「文件」菜单中更改此设置。" + "value" : "本地存储内容：堆栈跟踪、操作系统版本、应用版本、打开的标签页数量。您可以从「文件」菜单导出报告或随时更改此设置。" } } } diff --git a/Pine/Strings.swift b/Pine/Strings.swift index 802573a..22a0c84 100644 --- a/Pine/Strings.swift +++ b/Pine/Strings.swift @@ -431,6 +431,12 @@ enum Strings { static let crashReportingOptInTitle: LocalizedStringKey = "crashReporting.optIn.title" static let crashReportingOptInMessage: LocalizedStringKey = "crashReporting.optIn.message" static let crashReportingOptInPrivacy: LocalizedStringKey = "crashReporting.optIn.privacy" + static var crashReportingOptInEnable: String { + String(localized: "crashReporting.optIn.enable") + } + static var crashReportingOptInDisable: String { + String(localized: "crashReporting.optIn.disable") + } // MARK: - Progress Indicators diff --git a/PineTests/CrashReportingTests.swift b/PineTests/CrashReportingTests.swift index c176ee0..ed72e0c 100644 --- a/PineTests/CrashReportingTests.swift +++ b/PineTests/CrashReportingTests.swift @@ -521,6 +521,288 @@ struct ExtractCallStackFramesTests { } } +// MARK: - CrashReportStore Thread Safety Tests + +struct CrashReportStoreThreadSafetyTests { + + private func makeTempStore() -> CrashReportStore { + let tmpDir = FileManager.default.temporaryDirectory + .appendingPathComponent("PineTests-CrashStore-Thread-\(UUID().uuidString)") + return CrashReportStore(storageDirectory: tmpDir) + } + + private func cleanup(_ store: CrashReportStore) { + try? FileManager.default.removeItem(at: store.storageDirectory) + } + + @Test func concurrentSaves_doNotCrash() { + let store = makeTempStore() + defer { cleanup(store) } + + let group = DispatchGroup() + let concurrentQueue = DispatchQueue(label: "test.concurrent", attributes: .concurrent) + + for i in 0..<20 { + group.enter() + concurrentQueue.async { + let report = CrashReport(signal: "SIG\(i)") + store.save(report) + group.leave() + } + } + + group.wait() + #expect(store.count == 20) + } + + @Test func concurrentSaveAndLoad_doNotCrash() { + let store = makeTempStore() + defer { cleanup(store) } + + let group = DispatchGroup() + let concurrentQueue = DispatchQueue(label: "test.concurrent.rw", attributes: .concurrent) + + // Pre-populate with some reports + for i in 0..<5 { + store.save(CrashReport(signal: "PRE\(i)")) + } + + // Concurrent reads and writes + for i in 0..<10 { + group.enter() + concurrentQueue.async { + store.save(CrashReport(signal: "WRITE\(i)")) + group.leave() + } + + group.enter() + concurrentQueue.async { + _ = store.loadAll() + group.leave() + } + + group.enter() + concurrentQueue.async { + _ = store.count + group.leave() + } + } + + group.wait() + #expect(store.count == 15) // 5 pre + 10 writes + } + + @Test func concurrentRemoveAll_doNotCrash() { + let store = makeTempStore() + defer { cleanup(store) } + + for i in 0..<10 { + store.save(CrashReport(signal: "SIG\(i)")) + } + + let group = DispatchGroup() + let concurrentQueue = DispatchQueue(label: "test.concurrent.remove", attributes: .concurrent) + + // Concurrent removeAll and loadAll should not crash + for _ in 0..<5 { + group.enter() + concurrentQueue.async { + store.removeAll() + group.leave() + } + + group.enter() + concurrentQueue.async { + _ = store.loadAll() + group.leave() + } + } + + group.wait() + // After removeAll, store should be empty + #expect(store.loadAll().isEmpty) + } +} + +// MARK: - CrashReportStore Export Tests + +struct CrashReportStoreExportTests { + + private func makeTempStore() -> CrashReportStore { + let tmpDir = FileManager.default.temporaryDirectory + .appendingPathComponent("PineTests-CrashStore-Export-\(UUID().uuidString)") + return CrashReportStore(storageDirectory: tmpDir) + } + + private func cleanup(_ store: CrashReportStore) { + try? FileManager.default.removeItem(at: store.storageDirectory) + } + + @Test func copyAllToClipboard_emptyStore_returnsZero() { + let store = makeTempStore() + defer { cleanup(store) } + + let count = store.copyAllToClipboard() + #expect(count == 0) + } + + @Test func copyAllToClipboard_withReports_returnsCopiedCount() { + let store = makeTempStore() + defer { cleanup(store) } + + store.save(CrashReport(signal: "SIGSEGV")) + store.save(CrashReport(signal: "SIGABRT")) + + let count = store.copyAllToClipboard() + #expect(count == 2) + } + + @Test func copyAllToClipboard_writesValidJSON() { + let store = makeTempStore() + defer { cleanup(store) } + + let report = CrashReport(signal: "SIGSEGV", exceptionType: "EXC_BAD_ACCESS") + store.save(report) + + store.copyAllToClipboard() + + let pasteboard = NSPasteboard.general + guard let json = pasteboard.string(forType: .string) else { + #expect(Bool(false), "Clipboard should contain a string") + return + } + + // Verify it's valid JSON (uses iso8601 date encoding) + let decoder = JSONDecoder() + decoder.dateDecodingStrategy = .iso8601 + guard let data = json.data(using: .utf8), + let decoded = try? decoder.decode([CrashReport].self, from: data) else { + #expect(Bool(false), "Clipboard content should be valid JSON decodable to [CrashReport]") + return + } + + #expect(decoded.count == 1) + #expect(decoded[0].signal == "SIGSEGV") + #expect(decoded[0].exceptionType == "EXC_BAD_ACCESS") + } + + @Test func storageDirectory_isAccessible() { + let store = makeTempStore() + defer { cleanup(store) } + + // The storage directory should exist after init + var isDir: ObjCBool = false + let exists = FileManager.default.fileExists(atPath: store.storageDirectory.path, isDirectory: &isDir) + #expect(exists) + #expect(isDir.boolValue) + } +} + +// MARK: - CrashReportingSettings Opt-In Race Tests + +struct CrashReportingSettingsRaceTests { + + private func withCleanDefaults(_ body: () -> Void) { + let enabledKey = CrashReportingSettings.enabledKey + let promptKey = CrashReportingSettings.promptShownKey + let savedEnabled = UserDefaults.standard.object(forKey: enabledKey) + let savedPrompt = UserDefaults.standard.object(forKey: promptKey) + + UserDefaults.standard.removeObject(forKey: enabledKey) + UserDefaults.standard.removeObject(forKey: promptKey) + + body() + + if let val = savedEnabled { + UserDefaults.standard.set(val, forKey: enabledKey) + } else { + UserDefaults.standard.removeObject(forKey: enabledKey) + } + if let val = savedPrompt { + UserDefaults.standard.set(val, forKey: promptKey) + } else { + UserDefaults.standard.removeObject(forKey: promptKey) + } + } + + @Test func hasShownPrompt_setBeforeDisplay_preventsRace() { + withCleanDefaults { + // Simulate what showCrashReportingOptInIfNeeded now does: + // set hasShownPrompt BEFORE the async delay + #expect(CrashReportingSettings.needsPrompt) + + // First "window" marks prompt as shown + CrashReportingSettings.hasShownPrompt = true + + // Second "window" should see prompt already shown + #expect(!CrashReportingSettings.needsPrompt) + } + } + + @Test func recordChoice_afterHasShownPrompt_worksCorrectly() { + withCleanDefaults { + // Mark as shown (as the fix does) + CrashReportingSettings.hasShownPrompt = true + #expect(!CrashReportingSettings.needsPrompt) + #expect(!CrashReportingSettings.isEnabled) // Not yet enabled + + // User clicks Enable + CrashReportingSettings.recordChoice(enabled: true) + #expect(CrashReportingSettings.isEnabled) + #expect(CrashReportingSettings.hasShownPrompt) // Still true + } + } + + @Test func recordChoice_afterHasShownPrompt_disable() { + withCleanDefaults { + CrashReportingSettings.hasShownPrompt = true + CrashReportingSettings.recordChoice(enabled: false) + #expect(!CrashReportingSettings.isEnabled) + #expect(CrashReportingSettings.hasShownPrompt) + } + } +} + +// MARK: - CrashReportingManager Stop Tests + +struct CrashReportingManagerStopTests { + + @Test func stop_canBeCalledMultipleTimes() { + let store = CrashReportStore( + storageDirectory: FileManager.default.temporaryDirectory + .appendingPathComponent("PineTests-Manager-\(UUID().uuidString)") + ) + defer { try? FileManager.default.removeItem(at: store.storageDirectory) } + + let manager = CrashReportingManager(store: store) + // Multiple stop() calls should not crash + manager.stop() + manager.stop() + manager.stop() + } + + @Test func crashMarkerPath_isNotEmpty() { + let path = CrashReportingManager.crashMarkerPath + #expect(!path.isEmpty) + #expect(path.contains("Pine")) + } +} + +// MARK: - Strings Constants Tests + +struct CrashReportingStringsTests { + + @Test func optInEnable_string_isNotEmpty() { + let value = Strings.crashReportingOptInEnable + #expect(!value.isEmpty) + } + + @Test func optInDisable_string_isNotEmpty() { + let value = Strings.crashReportingOptInDisable + #expect(!value.isEmpty) + } +} + // MARK: - MenuIcons Tests struct CrashReportingMenuIconTests { From 63d190db14f58f4e349c5768ff0072ab02043195 Mon Sep 17 00:00:00 2001 From: Fedor Batonogov Date: Tue, 31 Mar 2026 06:52:36 +0300 Subject: [PATCH 4/7] =?UTF-8?q?fix:=20address=20PR=20#629=20review=20?= =?UTF-8?q?=E2=80=94=20signal=20safety,=20thread=20safety,=20dead=20code,?= =?UTF-8?q?=20SwiftLint?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit - Rename count to copiedCount in tests to fix SwiftLint empty_count - Add menu buttons for Reveal Crash Reports in Finder and Copy Crash Reports so revealInFinder()/copyAllToClipboard() are no longer dead code - Replace bare crashMarkerCString global with CrashMarkerStorage struct using os_unfair_lock for thread-safe access (trylock in signal handler) - Rewrite signal handler to avoid Swift runtime: no guard/let unwrap, no withUnsafePointer closures, byte-by-byte POSIX write() - Add tests for new menu icons --- Pine/CrashReportingManager.swift | 106 +++++++++++++++++----------- Pine/Localizable.xcstrings | 36 ++++++++++ Pine/MenuIcons.swift | 2 + Pine/PineApp.swift | 13 ++++ Pine/Strings.swift | 2 + PineTests/CrashReportingTests.swift | 22 ++++-- 6 files changed, 137 insertions(+), 44 deletions(-) diff --git a/Pine/CrashReportingManager.swift b/Pine/CrashReportingManager.swift index 97631fb..9323461 100644 --- a/Pine/CrashReportingManager.swift +++ b/Pine/CrashReportingManager.swift @@ -12,11 +12,37 @@ import os // MARK: - Global async-signal-safe storage for crash marker path -/// Pre-computed C string for the crash marker path. -/// Set once during `installSignalHandlers()`, read-only in the signal handler. -/// Using nonisolated(unsafe) because signal handlers are inherently unsafe -/// and this is written once before any signal can fire. -nonisolated(unsafe) private var crashMarkerCString: UnsafeMutablePointer? +/// Thread-safe atomic storage for the crash marker C string pointer. +/// Uses `os_unfair_lock` to protect reads/writes from concurrent access +/// between `installSignalHandlers()`, `stop()`, and the signal handler. +/// The signal handler reads via `loadCrashMarkerCString()` which uses +/// `os_unfair_lock_trylock` (async-signal-safe) to avoid deadlock. +private struct CrashMarkerStorage { + nonisolated(unsafe) static var lock = os_unfair_lock() + nonisolated(unsafe) static var cString: UnsafeMutablePointer? + + /// Stores a new C string path. Frees the previous one if set. + /// Called from main thread only (installSignalHandlers / stop). + static func store(_ newValue: UnsafeMutablePointer?) { + os_unfair_lock_lock(&lock) + let previous = cString + cString = newValue + os_unfair_lock_unlock(&lock) + if let previous { + free(previous) + } + } + + /// Loads the current C string pointer. + /// Uses trylock so it is safe to call from a signal handler (async-signal-safe). + /// Returns nil if the lock is contended (extremely unlikely during a crash). + static func load() -> UnsafeMutablePointer? { + guard os_unfair_lock_trylock(&lock) else { return nil } + let value = cString + os_unfair_lock_unlock(&lock) + return value + } +} /// Coordinates crash reporting using MetricKit (MXCrashDiagnostic) as the /// primary source and POSIX signal handlers as a minimal async-signal-safe fallback. @@ -112,10 +138,7 @@ final class CrashReportingManager: NSObject, MXMetricManagerSubscriber { } // Free the strdup'd path to prevent memory leak on repeated start/stop cycles - if let ptr = crashMarkerCString { - free(ptr) - crashMarkerCString = nil - } + CrashMarkerStorage.store(nil) logger.info("Crash reporting stopped") } @@ -158,14 +181,9 @@ final class CrashReportingManager: NSObject, MXMetricManagerSubscriber { private func installSignalHandlers() { // Pre-compute the path as a C string BEFORE installing handlers. // This is the ONLY place we allocate — the handler only reads this pointer. - // Free any previously allocated string to prevent leaks on repeated start() calls. - if let previous = crashMarkerCString { - free(previous) - crashMarkerCString = nil - } + // CrashMarkerStorage.store() frees any previous allocation to prevent leaks. let path = Self.crashMarkerPath - let cString = strdup(path) - crashMarkerCString = cString + CrashMarkerStorage.store(strdup(path)) let signals: [Int32] = [SIGSEGV, SIGABRT, SIGBUS, SIGFPE, SIGILL, SIGTRAP] @@ -209,37 +227,45 @@ final class CrashReportingManager: NSObject, MXMetricManagerSubscriber { // MARK: - Async-signal-safe crash handler (C-level, no Swift/Foundation) /// Strictly async-signal-safe signal handler. -/// Uses ONLY POSIX APIs: open(), write(), close(), _exit(). -/// No Swift runtime, no Foundation, no malloc, no ObjC messaging. +/// Uses ONLY POSIX APIs: open(), write(), close(), _exit(), signal(), raise(). +/// No Swift runtime (no guard/let/Optional unwrap), no Foundation, no malloc, no ObjC messaging. /// /// The crash marker path is pre-computed in `installSignalHandlers()` and stored -/// in the global `crashMarkerCString`. This function only reads that pointer. +/// in `CrashMarkerStorage`. This function reads via trylock (async-signal-safe). private func signalHandler(_ sig: Int32) { - // Read pre-computed C string path — no allocation, no Swift runtime - guard let cPath = crashMarkerCString else { - _exit(sig) - } + // Load pre-computed C string path via atomic trylock — no allocation, no Swift runtime. + // If trylock fails (lock contended) or path is nil, skip writing and just re-raise. + // Write marker file only if we successfully obtained the path. + writeMarkerIfPossible(sig) + + // Re-raise with default handler to get proper exit code + Darwin.signal(sig, SIG_DFL) + raise(sig) +} + +/// Writes the signal number to the crash marker file. +/// Separated from signalHandler to allow structured control flow without force-unwrap. +/// Still strictly async-signal-safe: only POSIX calls, no Swift runtime, no malloc. +private func writeMarkerIfPossible(_ sig: Int32) { + guard let cPath = CrashMarkerStorage.load() else { return } // Open file for writing (create if needed, truncate) let fd = open(cPath, O_WRONLY | O_CREAT | O_TRUNC, 0o644) - guard fd >= 0 else { - _exit(sig) - } + if fd < 0 { return } // Write the signal number as ASCII digits using a stack buffer (no malloc). - // Max digits for Int32: 10 digits + newline = 11 bytes. Using a 4-byte tuple - // is sufficient since POSIX signals are small numbers (1-31). - var buf = (UInt8(0), UInt8(0), UInt8(0), UInt8(0)) + // POSIX signals are small numbers (1-31), so 2 digits + newline suffice. + var buf: (UInt8, UInt8, UInt8, UInt8) = (0, 0, 0, 0) var sigNum = sig - var len = 0 + var len: Int = 0 if sigNum == 0 { buf.0 = 0x30 // '0' len = 1 } else { // Signals are 1-31, so at most 2 digits — reverse into buf - var tmp = (UInt8(0), UInt8(0)) - var tmpLen = 0 + var tmp: (UInt8, UInt8) = (0, 0) + var tmpLen: Int = 0 while sigNum > 0, tmpLen < 2 { if tmpLen == 0 { tmp.0 = UInt8(0x30 + sigNum % 10) @@ -268,14 +294,14 @@ private func signalHandler(_ sig: Int32) { } len += 1 - // Write using POSIX write() — async-signal-safe - withUnsafePointer(to: &buf) { ptr in - _ = write(fd, ptr, len) - } + // Write each byte individually using POSIX write() — async-signal-safe. + // Avoids Swift closures (withUnsafePointer) inside signal handler. + var byte0 = buf.0 + if len >= 1 { _ = write(fd, &byte0, 1) } + var byte1 = buf.1 + if len >= 2 { _ = write(fd, &byte1, 1) } + var byte2 = buf.2 + if len >= 3 { _ = write(fd, &byte2, 1) } close(fd) - - // Re-raise with default handler to get proper exit code - Darwin.signal(sig, SIG_DFL) - raise(sig) } diff --git a/Pine/Localizable.xcstrings b/Pine/Localizable.xcstrings index 9947592..3dd485b 100644 --- a/Pine/Localizable.xcstrings +++ b/Pine/Localizable.xcstrings @@ -1803,6 +1803,42 @@ } } }, + "menu.crashReports.copy" : { + "comment" : "Menu item to copy all crash reports to clipboard.", + "extractionState" : "manual", + "localizations" : { + "en" : { + "stringUnit" : { + "state" : "translated", + "value" : "Copy Crash Reports" + } + }, + "ru" : { + "stringUnit" : { + "state" : "translated", + "value" : "Скопировать отчёты о сбоях" + } + } + } + }, + "menu.crashReports.reveal" : { + "comment" : "Menu item to reveal crash reports directory in Finder.", + "extractionState" : "manual", + "localizations" : { + "en" : { + "stringUnit" : { + "state" : "translated", + "value" : "Reveal Crash Reports in Finder" + } + }, + "ru" : { + "stringUnit" : { + "state" : "translated", + "value" : "Показать отчёты о сбоях в Finder" + } + } + } + }, "dialog.ok" : { "comment" : "OK button in dialogs.", "extractionState" : "manual", diff --git a/Pine/MenuIcons.swift b/Pine/MenuIcons.swift index d948b52..e3a7d51 100644 --- a/Pine/MenuIcons.swift +++ b/Pine/MenuIcons.swift @@ -56,6 +56,8 @@ nonisolated enum MenuIcons { // MARK: - Crash Reporting static let crashReporting = "ant.circle" + static let crashReportsReveal = "folder" + static let crashReportsCopy = "doc.on.doc" // MARK: - Context menu static let newFile = "doc.badge.plus" diff --git a/Pine/PineApp.swift b/Pine/PineApp.swift index f4da508..5d1b92a 100644 --- a/Pine/PineApp.swift +++ b/Pine/PineApp.swift @@ -461,6 +461,19 @@ struct PineApp: App { CrashReportingManager.shared.stop() } } + + Button { + CrashReportStore.shared.revealInFinder() + } label: { + Label(Strings.menuCrashReportsReveal, systemImage: MenuIcons.crashReportsReveal) + } + + Button { + CrashReportStore.shared.copyAllToClipboard() + } label: { + Label(Strings.menuCrashReportsCopy, systemImage: MenuIcons.crashReportsCopy) + } + .disabled(CrashReportStore.shared.loadAll().isEmpty) } // Cmd+W is intercepted by AppDelegate's local event monitor // to close the active tab. The close button goes through diff --git a/Pine/Strings.swift b/Pine/Strings.swift index 22a0c84..8ea7ad3 100644 --- a/Pine/Strings.swift +++ b/Pine/Strings.swift @@ -428,6 +428,8 @@ enum Strings { // MARK: - Crash Reporting static let menuCrashReporting: LocalizedStringKey = "menu.crashReporting" + static let menuCrashReportsReveal: LocalizedStringKey = "menu.crashReports.reveal" + static let menuCrashReportsCopy: LocalizedStringKey = "menu.crashReports.copy" static let crashReportingOptInTitle: LocalizedStringKey = "crashReporting.optIn.title" static let crashReportingOptInMessage: LocalizedStringKey = "crashReporting.optIn.message" static let crashReportingOptInPrivacy: LocalizedStringKey = "crashReporting.optIn.privacy" diff --git a/PineTests/CrashReportingTests.swift b/PineTests/CrashReportingTests.swift index ed72e0c..fb19fca 100644 --- a/PineTests/CrashReportingTests.swift +++ b/PineTests/CrashReportingTests.swift @@ -642,8 +642,8 @@ struct CrashReportStoreExportTests { let store = makeTempStore() defer { cleanup(store) } - let count = store.copyAllToClipboard() - #expect(count == 0) + let copiedCount = store.copyAllToClipboard() + #expect(copiedCount == 0) } @Test func copyAllToClipboard_withReports_returnsCopiedCount() { @@ -653,8 +653,8 @@ struct CrashReportStoreExportTests { store.save(CrashReport(signal: "SIGSEGV")) store.save(CrashReport(signal: "SIGABRT")) - let count = store.copyAllToClipboard() - #expect(count == 2) + let copiedCount = store.copyAllToClipboard() + #expect(copiedCount == 2) } @Test func copyAllToClipboard_writesValidJSON() { @@ -812,4 +812,18 @@ struct CrashReportingMenuIconTests { "SF Symbol '\(MenuIcons.crashReporting)' does not exist" ) } + + @Test func crashReportsReveal_iconExists() { + #expect( + NSImage(systemSymbolName: MenuIcons.crashReportsReveal, accessibilityDescription: nil) != nil, + "SF Symbol '\(MenuIcons.crashReportsReveal)' does not exist" + ) + } + + @Test func crashReportsCopy_iconExists() { + #expect( + NSImage(systemSymbolName: MenuIcons.crashReportsCopy, accessibilityDescription: nil) != nil, + "SF Symbol '\(MenuIcons.crashReportsCopy)' does not exist" + ) + } } From 1f80e58deb8d72f748b0782ab2f7d1e42effcb18 Mon Sep 17 00:00:00 2001 From: Fedor Batonogov Date: Tue, 31 Mar 2026 07:02:32 +0300 Subject: [PATCH 5/7] =?UTF-8?q?fix:=20address=20PR=20#629=20round=203=20?= =?UTF-8?q?=E2=80=94=20menu=20perf,=20localization,=20signal=20handler=20d?= =?UTF-8?q?ocs?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit - Replace loadAll().isEmpty with lightweight isEmpty property (dir listing only, no JSON decoding) to avoid disk I/O on every menu render - Add missing 7 translations (de, es, fr, ja, ko, pt-BR, zh-Hans) for menu.crashReports.copy and menu.crashReports.reveal in xcstrings - Fix misleading signal handler comment — guard let on raw pointer is safe - Add warning log when strdup returns nil (OOM) - Add isEmpty test for CrashReportStore --- Pine/CrashReportStore.swift | 9 +++- Pine/CrashReportingManager.swift | 8 ++- Pine/Localizable.xcstrings | 84 +++++++++++++++++++++++++++++ Pine/PineApp.swift | 2 +- PineTests/CrashReportingTests.swift | 11 ++++ 5 files changed, 110 insertions(+), 4 deletions(-) diff --git a/Pine/CrashReportStore.swift b/Pine/CrashReportStore.swift index d6d9dda..753f0c8 100644 --- a/Pine/CrashReportStore.swift +++ b/Pine/CrashReportStore.swift @@ -74,13 +74,20 @@ final class CrashReportStore { } } - /// Returns the number of stored reports. + /// Returns the number of stored reports (directory listing only, no JSON decoding). var count: Int { queue.sync { _count() } } + /// Whether the store has no reports (directory listing only, no JSON decoding). + var isEmpty: Bool { + queue.sync { + _count() == 0 + } + } + /// Reveals the crash reports directory in Finder. func revealInFinder() { NSWorkspace.shared.selectFile(nil, inFileViewerRootedAtPath: storageDirectory.path) diff --git a/Pine/CrashReportingManager.swift b/Pine/CrashReportingManager.swift index 9323461..37fa9bf 100644 --- a/Pine/CrashReportingManager.swift +++ b/Pine/CrashReportingManager.swift @@ -183,7 +183,11 @@ final class CrashReportingManager: NSObject, MXMetricManagerSubscriber { // This is the ONLY place we allocate — the handler only reads this pointer. // CrashMarkerStorage.store() frees any previous allocation to prevent leaks. let path = Self.crashMarkerPath - CrashMarkerStorage.store(strdup(path)) + let cString = strdup(path) + if cString == nil { + logger.warning("strdup returned nil (OOM) — signal handler fallback will be disabled") + } + CrashMarkerStorage.store(cString) let signals: [Int32] = [SIGSEGV, SIGABRT, SIGBUS, SIGFPE, SIGILL, SIGTRAP] @@ -228,7 +232,7 @@ final class CrashReportingManager: NSObject, MXMetricManagerSubscriber { /// Strictly async-signal-safe signal handler. /// Uses ONLY POSIX APIs: open(), write(), close(), _exit(), signal(), raise(). -/// No Swift runtime (no guard/let/Optional unwrap), no Foundation, no malloc, no ObjC messaging. +/// No Foundation, no malloc, no ObjC messaging. Minimal Swift (guard let on raw pointer is safe). /// /// The crash marker path is pre-computed in `installSignalHandlers()` and stored /// in `CrashMarkerStorage`. This function reads via trylock (async-signal-safe). diff --git a/Pine/Localizable.xcstrings b/Pine/Localizable.xcstrings index 3dd485b..5780167 100644 --- a/Pine/Localizable.xcstrings +++ b/Pine/Localizable.xcstrings @@ -1807,17 +1807,59 @@ "comment" : "Menu item to copy all crash reports to clipboard.", "extractionState" : "manual", "localizations" : { + "de" : { + "stringUnit" : { + "state" : "translated", + "value" : "Absturzberichte kopieren" + } + }, "en" : { "stringUnit" : { "state" : "translated", "value" : "Copy Crash Reports" } }, + "es" : { + "stringUnit" : { + "state" : "translated", + "value" : "Copiar informes de error" + } + }, + "fr" : { + "stringUnit" : { + "state" : "translated", + "value" : "Copier les rapports de plantage" + } + }, + "ja" : { + "stringUnit" : { + "state" : "translated", + "value" : "クラッシュレポートをコピー" + } + }, + "ko" : { + "stringUnit" : { + "state" : "translated", + "value" : "충돌 보고서 복사" + } + }, + "pt-BR" : { + "stringUnit" : { + "state" : "translated", + "value" : "Copiar relatórios de falhas" + } + }, "ru" : { "stringUnit" : { "state" : "translated", "value" : "Скопировать отчёты о сбоях" } + }, + "zh-Hans" : { + "stringUnit" : { + "state" : "translated", + "value" : "拷贝崩溃报告" + } } } }, @@ -1825,17 +1867,59 @@ "comment" : "Menu item to reveal crash reports directory in Finder.", "extractionState" : "manual", "localizations" : { + "de" : { + "stringUnit" : { + "state" : "translated", + "value" : "Absturzberichte im Finder anzeigen" + } + }, "en" : { "stringUnit" : { "state" : "translated", "value" : "Reveal Crash Reports in Finder" } }, + "es" : { + "stringUnit" : { + "state" : "translated", + "value" : "Mostrar informes de error en Finder" + } + }, + "fr" : { + "stringUnit" : { + "state" : "translated", + "value" : "Afficher les rapports de plantage dans le Finder" + } + }, + "ja" : { + "stringUnit" : { + "state" : "translated", + "value" : "Finderでクラッシュレポートを表示" + } + }, + "ko" : { + "stringUnit" : { + "state" : "translated", + "value" : "Finder에서 충돌 보고서 표시" + } + }, + "pt-BR" : { + "stringUnit" : { + "state" : "translated", + "value" : "Mostrar relatórios de falhas no Finder" + } + }, "ru" : { "stringUnit" : { "state" : "translated", "value" : "Показать отчёты о сбоях в Finder" } + }, + "zh-Hans" : { + "stringUnit" : { + "state" : "translated", + "value" : "在访达中显示崩溃报告" + } } } }, diff --git a/Pine/PineApp.swift b/Pine/PineApp.swift index 5d1b92a..b29f1d3 100644 --- a/Pine/PineApp.swift +++ b/Pine/PineApp.swift @@ -473,7 +473,7 @@ struct PineApp: App { } label: { Label(Strings.menuCrashReportsCopy, systemImage: MenuIcons.crashReportsCopy) } - .disabled(CrashReportStore.shared.loadAll().isEmpty) + .disabled(CrashReportStore.shared.isEmpty) } // Cmd+W is intercepted by AppDelegate's local event monitor // to close the active tab. The close button goes through diff --git a/PineTests/CrashReportingTests.swift b/PineTests/CrashReportingTests.swift index fb19fca..3f53324 100644 --- a/PineTests/CrashReportingTests.swift +++ b/PineTests/CrashReportingTests.swift @@ -290,6 +290,17 @@ struct CrashReportStoreTests { #expect(store.count == 2) } + @Test func isEmpty_reflectsStoreState() { + let store = makeTempStore() + defer { cleanup(store) } + + #expect(store.isEmpty) + store.save(CrashReport(signal: "SIGSEGV")) + #expect(!store.isEmpty) + store.removeAll() + #expect(store.isEmpty) + } + @Test func remove_byID() { let store = makeTempStore() defer { cleanup(store) } From 69e393370e4f0c229b1427218fd9454d9eb052e5 Mon Sep 17 00:00:00 2001 From: Fedor Batonogov Date: Tue, 31 Mar 2026 07:15:07 +0300 Subject: [PATCH 6/7] fix: set hasShownPrompt when toggling crash reporting via menu --- Pine/PineApp.swift | 1 + 1 file changed, 1 insertion(+) diff --git a/Pine/PineApp.swift b/Pine/PineApp.swift index b29f1d3..0fbb879 100644 --- a/Pine/PineApp.swift +++ b/Pine/PineApp.swift @@ -455,6 +455,7 @@ struct PineApp: App { Label(Strings.menuCrashReporting, systemImage: MenuIcons.crashReporting) } .onChange(of: crashReportingEnabled) { _, newValue in + CrashReportingSettings.hasShownPrompt = true if newValue { CrashReportingManager.shared.startIfEnabled() } else { From 2c03b0a7ea1211a9a6b92ffdccfda9451d998737 Mon Sep 17 00:00:00 2001 From: Fedor Batonogov Date: Tue, 31 Mar 2026 10:48:22 +0300 Subject: [PATCH 7/7] fix: wrap signalHandler in literal closure for Swift 6 C pointer compatibility --- Pine/CrashReportingManager.swift | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/Pine/CrashReportingManager.swift b/Pine/CrashReportingManager.swift index 37fa9bf..f83cfff 100644 --- a/Pine/CrashReportingManager.swift +++ b/Pine/CrashReportingManager.swift @@ -192,7 +192,9 @@ final class CrashReportingManager: NSObject, MXMetricManagerSubscriber { let signals: [Int32] = [SIGSEGV, SIGABRT, SIGBUS, SIGFPE, SIGILL, SIGTRAP] for sig in signals { - signal(sig, signalHandler) + signal(sig) { caughtSignal in + signalHandler(caughtSignal) + } } }