From 630176a4c091c334eb806c73bf0874d430e09f0c Mon Sep 17 00:00:00 2001 From: melekr Date: Mon, 2 Jun 2025 18:06:18 -0400 Subject: [PATCH 1/2] add thread-aware live-report and guard port-leaks add generateLiveReport with thread to CrashReporting protocol defer mach_port_deallocate to avoid Mach-port leaks add helper currentThreadIdentifier() for thread id logging and testing generate reports with faulting thread Add thread forwarding and leak spec tests --- Backtrace.xcodeproj/project.pbxproj | 8 ++ Podfile.lock | 4 +- .../Features/Client/BacktraceReporter.swift | 1 + Sources/Public/BacktraceCrashReporter.swift | 19 ++++ Sources/Public/Internal/CrashReporting.swift | 7 +- Tests/BacktraceReporterThreadSpec.swift | 87 +++++++++++++++++++ 6 files changed, 123 insertions(+), 3 deletions(-) create mode 100644 Tests/BacktraceReporterThreadSpec.swift diff --git a/Backtrace.xcodeproj/project.pbxproj b/Backtrace.xcodeproj/project.pbxproj index e53afc10..829bd5d2 100644 --- a/Backtrace.xcodeproj/project.pbxproj +++ b/Backtrace.xcodeproj/project.pbxproj @@ -25,6 +25,9 @@ 2062D9C92C457B4500E4CE3C /* Crash+CoreDataProperties.swift in Sources */ = {isa = PBXBuildFile; fileRef = 2062D9C52C457B4500E4CE3C /* Crash+CoreDataProperties.swift */; }; 2062D9CA2C457B4500E4CE3C /* Crash+CoreDataClass.swift in Sources */ = {isa = PBXBuildFile; fileRef = 2062D9C42C457B4500E4CE3C /* Crash+CoreDataClass.swift */; }; 2062D9CB2C457B4500E4CE3C /* Crash+CoreDataProperties.swift in Sources */ = {isa = PBXBuildFile; fileRef = 2062D9C52C457B4500E4CE3C /* Crash+CoreDataProperties.swift */; }; + 20B01A542DEE4DCA00FED98F /* BacktraceReporterThreadSpec.swift in Sources */ = {isa = PBXBuildFile; fileRef = 20B01A532DEE4DC600FED98F /* BacktraceReporterThreadSpec.swift */; }; + 20B01A552DEE4DCA00FED98F /* BacktraceReporterThreadSpec.swift in Sources */ = {isa = PBXBuildFile; fileRef = 20B01A532DEE4DC600FED98F /* BacktraceReporterThreadSpec.swift */; }; + 20B01A562DEE4DCA00FED98F /* BacktraceReporterThreadSpec.swift in Sources */ = {isa = PBXBuildFile; fileRef = 20B01A532DEE4DC600FED98F /* BacktraceReporterThreadSpec.swift */; }; 20DE4B342D4830D80076B3F6 /* NSManagedObjectContext+Extensions.swift in Sources */ = {isa = PBXBuildFile; fileRef = 20DE4B332D4830D00076B3F6 /* NSManagedObjectContext+Extensions.swift */; }; 20DE4B352D4830D80076B3F6 /* NSManagedObjectContext+Extensions.swift in Sources */ = {isa = PBXBuildFile; fileRef = 20DE4B332D4830D00076B3F6 /* NSManagedObjectContext+Extensions.swift */; }; 20DE4B362D4830D80076B3F6 /* NSManagedObjectContext+Extensions.swift in Sources */ = {isa = PBXBuildFile; fileRef = 20DE4B332D4830D00076B3F6 /* NSManagedObjectContext+Extensions.swift */; }; @@ -425,6 +428,7 @@ 2050DBBE2C66D98500C6CCA9 /* PrivacyInfo.xcprivacy */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = text.xml; path = PrivacyInfo.xcprivacy; sourceTree = ""; }; 2062D9C42C457B4500E4CE3C /* Crash+CoreDataClass.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = "Crash+CoreDataClass.swift"; sourceTree = ""; }; 2062D9C52C457B4500E4CE3C /* Crash+CoreDataProperties.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = "Crash+CoreDataProperties.swift"; sourceTree = ""; }; + 20B01A532DEE4DC600FED98F /* BacktraceReporterThreadSpec.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = BacktraceReporterThreadSpec.swift; sourceTree = ""; }; 20D4E5F32CB46A41000C92BF /* BacktraceResources-Info.plist */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = text.plist.xml; path = "BacktraceResources-Info.plist"; sourceTree = ""; }; 20DE4B332D4830D00076B3F6 /* NSManagedObjectContext+Extensions.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = "NSManagedObjectContext+Extensions.swift"; sourceTree = ""; }; 20DE4B372D48615C0076B3F6 /* NSManagedObjectContextExtensionTests.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = NSManagedObjectContextExtensionTests.swift; sourceTree = ""; }; @@ -884,6 +888,7 @@ F266B85321C77D5D00D14417 /* Tests */ = { isa = PBXGroup; children = ( + 20B01A532DEE4DC600FED98F /* BacktraceReporterThreadSpec.swift */, 20DE4B372D48615C0076B3F6 /* NSManagedObjectContextExtensionTests.swift */, A24A4B5428B595D8004F5052 /* AttachmentStorageTests.swift */, A24A4B4F28B595D8004F5052 /* AttachmentTests.swift */, @@ -2116,6 +2121,7 @@ A24A4B6228B595D9004F5052 /* BacktraceFileManagerTests.swift in Sources */, AFCCCE252625392300B83A28 /* ReportMetadataStorageMock.swift in Sources */, A24A4B7128B595D9004F5052 /* BacktraceCredentialsTests.swift in Sources */, + 20B01A562DEE4DCA00FED98F /* BacktraceReporterThreadSpec.swift in Sources */, A24A4B5F28B595D9004F5052 /* BacktraceDatabaseTests.swift in Sources */, A24A4B9328B59653004F5052 /* BacktraceNotificationObserverMock.swift in Sources */, A24A4B6B28B595D9004F5052 /* BacktraceRateLimiterTests.swift in Sources */, @@ -2209,6 +2215,7 @@ A24A4B8228B595D9004F5052 /* BacktraceBreadcrumbTests.swift in Sources */, A24A4B7628B595D9004F5052 /* DispatcherTests.swift in Sources */, A24A4B6728B595D9004F5052 /* BacktraceOomWatcherTests.swift in Sources */, + 20B01A542DEE4DCA00FED98F /* BacktraceReporterThreadSpec.swift in Sources */, F2AB636B2244243500939BC9 /* Quick+Throws.swift in Sources */, A24A4B5B28B595D9004F5052 /* BacktraceWatcherTests.swift in Sources */, A24A4B7928B595D9004F5052 /* AttributesTests.swift in Sources */, @@ -2324,6 +2331,7 @@ A24A4B8128B595D9004F5052 /* BacktraceBreadcrumbTests.swift in Sources */, A24A4B7528B595D9004F5052 /* DispatcherTests.swift in Sources */, A24A4B6628B595D9004F5052 /* BacktraceOomWatcherTests.swift in Sources */, + 20B01A552DEE4DCA00FED98F /* BacktraceReporterThreadSpec.swift in Sources */, F2AB636A2244243500939BC9 /* Quick+Throws.swift in Sources */, A24A4B5A28B595D9004F5052 /* BacktraceWatcherTests.swift in Sources */, A24A4B7828B595D9004F5052 /* AttributesTests.swift in Sources */, diff --git a/Podfile.lock b/Podfile.lock index 1b403e1c..2a8ebd17 100644 --- a/Podfile.lock +++ b/Podfile.lock @@ -1,5 +1,5 @@ PODS: - - Backtrace (2.0.6): + - Backtrace (2.0.8): - PLCrashReporter (= 1.11.1) - Nimble (10.0.0) - PLCrashReporter (1.11.1) @@ -22,7 +22,7 @@ EXTERNAL SOURCES: :path: "./Backtrace.podspec" SPEC CHECKSUMS: - Backtrace: 72bea7c55570b42ab18dd7218ce41ceef3edca14 + Backtrace: 5bcc05d7ef650d354f3ec3bdba76462d5edf5530 Nimble: 5316ef81a170ce87baf72dd961f22f89a602ff84 PLCrashReporter: 5d2d3967afe0efad61b3048d617e2199a5d1b787 Quick: 749aa754fd1e7d984f2000fe051e18a3a9809179 diff --git a/Sources/Features/Client/BacktraceReporter.swift b/Sources/Features/Client/BacktraceReporter.swift index 341ea0c7..f12916dd 100644 --- a/Sources/Features/Client/BacktraceReporter.swift +++ b/Sources/Features/Client/BacktraceReporter.swift @@ -118,6 +118,7 @@ extension BacktraceReporter { faultMessage: String? = nil) throws -> BacktraceReport { attributesProvider.set(faultMessage: faultMessage) let resource = try reporter.generateLiveReport(exception: exception, + thread: mach_thread_self(), attributes: attributesProvider.allAttributes, attachmentPaths: attachmentPaths + attributesProvider.attachmentPaths) diff --git a/Sources/Public/BacktraceCrashReporter.swift b/Sources/Public/BacktraceCrashReporter.swift index aa481f35..f55c54c7 100644 --- a/Sources/Public/BacktraceCrashReporter.swift +++ b/Sources/Public/BacktraceCrashReporter.swift @@ -54,6 +54,25 @@ extension BacktraceCrashReporter: CrashReporting { let reportData = try reporter.generateLiveReport(with: exception) return try BacktraceReport(report: reportData, attributes: attributes, attachmentPaths: attachmentPaths) } + + func generateLiveReport(exception: NSException? = nil, + thread: mach_port_t = mach_thread_self(), + attributes: Attributes, + attachmentPaths: [String] = []) throws -> BacktraceReport { + + defer { mach_port_deallocate(mach_task_self_, thread) } + let reportData = try reporter.generateLiveReport(withThread: thread, exception: exception) + + return try BacktraceReport(report: reportData, attributes: attributes, attachmentPaths: attachmentPaths) + } + + // Returns the thread ID + // can be used for logging/testing + static func currentThreadIdentifier() -> thread_t { + let tid = mach_thread_self() + mach_port_deallocate(mach_task_self_, tid) + return tid + } func enableCrashReporting() throws { try reporter.enableAndReturnError() diff --git a/Sources/Public/Internal/CrashReporting.swift b/Sources/Public/Internal/CrashReporting.swift index f314b25e..e3414428 100644 --- a/Sources/Public/Internal/CrashReporting.swift +++ b/Sources/Public/Internal/CrashReporting.swift @@ -1,7 +1,12 @@ import Foundation protocol CrashReporting { - func generateLiveReport(exception: NSException?, attributes: Attributes, + func generateLiveReport(exception: NSException?, + attributes: Attributes, + attachmentPaths: [String]) throws -> BacktraceReport + func generateLiveReport(exception: NSException?, + thread: mach_port_t, + attributes: Attributes, attachmentPaths: [String]) throws -> BacktraceReport func pendingCrashReport() throws -> BacktraceReport func purgePendingCrashReport() throws diff --git a/Tests/BacktraceReporterThreadSpec.swift b/Tests/BacktraceReporterThreadSpec.swift new file mode 100644 index 00000000..f9692c4e --- /dev/null +++ b/Tests/BacktraceReporterThreadSpec.swift @@ -0,0 +1,87 @@ +import Quick +import Nimble +import CrashReporter +@testable import Backtrace + +final class BacktraceReporterThreadSpec: QuickSpec { + + override func spec() { + + describe("Thread-aware live-report capture") { + + var backtraceReporter: BacktraceReporter! + var mockPLCR: PLCrashReporterMock! + var urlSession: URLSessionMock! + var credentials: BacktraceCredentials! + + beforeEach { + credentials = BacktraceCredentials( + endpoint: URL(string: "https://example.backtrace.io")!, + token: "" + ) + urlSession = URLSessionMock() + mockPLCR = PLCrashReporterMock() + + let crashReporter = BacktraceCrashReporter(reporter: mockPLCR) + + let api = BacktraceApi( + credentials: credentials, + session: urlSession, + reportsPerMin: 30 + ) + + backtraceReporter = try! BacktraceReporter( + reporter: crashReporter, + api: api, + dbSettings: BacktraceDatabaseSettings(), + credentials: credentials, + urlSession: urlSession + ) + } + + it("passes the calling thread to PLCrashReporter") { + let callThread = mach_thread_self() + _ = try? backtraceReporter.generate() + expect(mockPLCR.lastThread).to(equal(callThread)) + mach_port_deallocate(mach_task_self_, callThread) + } + + it("does not leak Mach SEND rights (> 2 refs to cover noise from Quick/Nimble or XCTest)") { + let before = sendRefCount() + _ = try? backtraceReporter.generate() + let after = sendRefCount() + expect(after).to(beLessThanOrEqualTo(before + 2)) + } + } + } +} + +/// Mock PLCrashReporter stubs that records the thread parameter +private final class PLCrashReporterMock: PLCrashReporter { + + private(set) var lastThread: thread_t = mach_thread_self() + + override func generateLiveReport( + withThread thread: thread_t, + exception: NSException? + ) throws -> Data { + lastThread = thread + return Data([0xCA, 0xFE]) + } + + override func generateLiveReport(with exception: NSException?) throws -> Data { + return Data([0xBE, 0xEF]) + } +} + +/// Current ref-count for thread +private func sendRefCount() -> mach_port_urefs_t { + var refs: mach_port_urefs_t = 0 + mach_port_get_refs( + mach_task_self_, + mach_thread_self(), + MACH_PORT_RIGHT_SEND, + &refs + ) + return refs +} From 3d5ad2545607f6b53dad20a2d795bab21e3c1b95 Mon Sep 17 00:00:00 2001 From: melekr Date: Mon, 16 Jun 2025 10:44:15 -0400 Subject: [PATCH 2/2] Unify live report generation logic Consolidates the 'generateLiveReport' implementations Remove 'currentThreadIdentifier' helper as it is no longer required by the simplified public interface --- Sources/Features/Client/BacktraceReporter.swift | 1 - Sources/Public/BacktraceCrashReporter.swift | 16 +++++----------- Sources/Public/Internal/CrashReporting.swift | 4 ---- 3 files changed, 5 insertions(+), 16 deletions(-) diff --git a/Sources/Features/Client/BacktraceReporter.swift b/Sources/Features/Client/BacktraceReporter.swift index f12916dd..341ea0c7 100644 --- a/Sources/Features/Client/BacktraceReporter.swift +++ b/Sources/Features/Client/BacktraceReporter.swift @@ -118,7 +118,6 @@ extension BacktraceReporter { faultMessage: String? = nil) throws -> BacktraceReport { attributesProvider.set(faultMessage: faultMessage) let resource = try reporter.generateLiveReport(exception: exception, - thread: mach_thread_self(), attributes: attributesProvider.allAttributes, attachmentPaths: attachmentPaths + attributesProvider.attachmentPaths) diff --git a/Sources/Public/BacktraceCrashReporter.swift b/Sources/Public/BacktraceCrashReporter.swift index f55c54c7..04881e89 100644 --- a/Sources/Public/BacktraceCrashReporter.swift +++ b/Sources/Public/BacktraceCrashReporter.swift @@ -51,12 +51,14 @@ extension BacktraceCrashReporter: CrashReporting { attributes: Attributes, attachmentPaths: [String] = []) throws -> BacktraceReport { - let reportData = try reporter.generateLiveReport(with: exception) - return try BacktraceReport(report: reportData, attributes: attributes, attachmentPaths: attachmentPaths) + return try generateLiveReport(exception: exception, + thread: mach_thread_self(), + attributes: attributes, + attachmentPaths: attachmentPaths) } func generateLiveReport(exception: NSException? = nil, - thread: mach_port_t = mach_thread_self(), + thread: mach_port_t, attributes: Attributes, attachmentPaths: [String] = []) throws -> BacktraceReport { @@ -65,14 +67,6 @@ extension BacktraceCrashReporter: CrashReporting { return try BacktraceReport(report: reportData, attributes: attributes, attachmentPaths: attachmentPaths) } - - // Returns the thread ID - // can be used for logging/testing - static func currentThreadIdentifier() -> thread_t { - let tid = mach_thread_self() - mach_port_deallocate(mach_task_self_, tid) - return tid - } func enableCrashReporting() throws { try reporter.enableAndReturnError() diff --git a/Sources/Public/Internal/CrashReporting.swift b/Sources/Public/Internal/CrashReporting.swift index e3414428..60745ff6 100644 --- a/Sources/Public/Internal/CrashReporting.swift +++ b/Sources/Public/Internal/CrashReporting.swift @@ -4,10 +4,6 @@ protocol CrashReporting { func generateLiveReport(exception: NSException?, attributes: Attributes, attachmentPaths: [String]) throws -> BacktraceReport - func generateLiveReport(exception: NSException?, - thread: mach_port_t, - attributes: Attributes, - attachmentPaths: [String]) throws -> BacktraceReport func pendingCrashReport() throws -> BacktraceReport func purgePendingCrashReport() throws func hasPendingCrashes() -> Bool